openstack/tacker
Code Issues Proposed changes

Refactor constants in conductor

Browse Source 
In the current implementation, some groups of constant values are
defined in conductor_server as a tuple that is a dependency on
`_change_vnf_status`. There are two problems: (i) the place where the
constants declared is inappropriate; (ii) it depends on a specific
function.

This patch solves these problem by moving constants declaration to
constants.py and modify the `_change_vnf_status`.

Change-Id: I7255e18f95ac3fd8199d25181b7b3c4f9de01b51
This commit is contained in:
Hiromu Asahina 2022-02-04 15:48:22 +09:00
parent 4d6031883c
commit 7b45bd1987
3 changed files with 43 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
tacker/conductor/conductor_server.py
View File

@ -107,17 +107,6 @@ cfg.CONF.register_opts(OPTS, 'keystone_authtoken')
LOG = logging.getLogger(__name__) LOG = logging.getLogger(__name__)
_INACTIVE_STATUS = ('INACTIVE',)
_ACTIVE_STATUS = ('ACTIVE',)
_PENDING_STATUS = ('PENDING_CREATE',
'PENDING_TERMINATE',
'PENDING_DELETE',
'PENDING_HEAL',
'PENDING_CHANGE_EXT_CONN')
_ERROR_STATUS = ('ERROR',)
_ALL_STATUSES = _ACTIVE_STATUS + _INACTIVE_STATUS + _PENDING_STATUS + \
_ERROR_STATUS
def _delete_csar(context, vnf_package): def _delete_csar(context, vnf_package):
# Delete from glance store # Delete from glance store
@ -630,25 +619,27 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
vnf_package.save() vnf_package.save()
@log.log @log.log
def _change_vnf_status(self, context, vnf_id, def _change_vnf_status(self, context, vnf_id, state_conditions,
current_statuses, new_status, error_reason=None): new_status):
'''Change vnf status and add error reason if error''' if isinstance(state_conditions, str):
LOG.debug("Change status of vnf %s from %s to %s", vnf_id, state_conditions = {state_conditions}
current_statuses, new_status)
'''Change vnf status'''
with context.session.begin(subtransactions=True): with context.session.begin(subtransactions=True):
updated_values = { updated_values = {'status': new_status,
'status': new_status, 'updated_at': timeutils.utcnow()} 'updated_at': timeutils.utcnow()}
vnf_model = (context.session.query( vnf_model = (context.session
vnfm_db.VNF).filter_by(id=vnf_id).first()) .query(vnfm_db.VNF).
filter_by(id=vnf_id).first())
LOG.debug("Change status of vnf %s from %s to %s", vnf_id,
vnf_model.status, new_status)
if not vnf_model: if not vnf_model:
raise exceptions.VnfInstanceNotFound( raise exceptions.VnfInstanceNotFound(
message="VNF {} not found".format(vnf_id)) message="VNF {} not found".format(vnf_id))
if vnf_model.status not in current_statuses: if vnf_model.status not in state_conditions:
raise exceptions.VnfConflictState( raise exceptions.VnfConflictState(
message='Cannot change status to {} \ message=('Cannot change status to %s while in %s'
while in {}'.format( % (updated_values['status'], vnf_model.status)))
updated_values['status'], vnf_model.status))
vnf_model.update(updated_values) vnf_model.update(updated_values)
def _update_vnf_attributes(self, context, vnf_instance, vnf_dict, def _update_vnf_attributes(self, context, vnf_instance, vnf_dict,
@ -1977,7 +1968,7 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
if vnf_dict['status'] == 'INACTIVE': if vnf_dict['status'] == 'INACTIVE':
vnf_dict['status'] = 'PENDING_CREATE' vnf_dict['status'] = 'PENDING_CREATE'
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_INACTIVE_STATUS, 'PENDING_CREATE') constants.INACTIVE, 'PENDING_CREATE')
if vnf_dict['before_error_point'] <= \ if vnf_dict['before_error_point'] <= \
fields.ErrorPoint.VNF_CONFIG_END: fields.ErrorPoint.VNF_CONFIG_END:
@ -1989,7 +1980,8 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
instantiate_vnf_req=instantiate_vnf) instantiate_vnf_req=instantiate_vnf)
self._update_vnf_attributes(context, vnf_instance, vnf_dict, self._update_vnf_attributes(context, vnf_instance, vnf_dict,
_PENDING_STATUS, _ACTIVE_STATUS) constants.PENDING_STATUSES,
constants.ACTIVE)
vnf_dict['current_error_point'] = \ vnf_dict['current_error_point'] = \
fields.ErrorPoint.NOTIFY_COMPLETED fields.ErrorPoint.NOTIFY_COMPLETED
@ -2018,7 +2010,7 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
vnf_dict['current_error_point'] = \ vnf_dict['current_error_point'] = \
fields.ErrorPoint.POST_VIM_CONTROL fields.ErrorPoint.POST_VIM_CONTROL
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_ALL_STATUSES, 'ERROR') constants.ALL_STATUSES, 'ERROR')
self._build_instantiated_vnf_info(context, vnf_instance, self._build_instantiated_vnf_info(context, vnf_instance,
instantiate_vnf) instantiate_vnf)
@ -2065,7 +2057,7 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
if vnf_dict['before_error_point'] <= \ if vnf_dict['before_error_point'] <= \
fields.ErrorPoint.NOTIFY_PROCESSING: fields.ErrorPoint.NOTIFY_PROCESSING:
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_ACTIVE_STATUS, 'PENDING_TERMINATE') constants.ACTIVE, 'PENDING_TERMINATE')
if vnf_dict['before_error_point'] <= \ if vnf_dict['before_error_point'] <= \
fields.ErrorPoint.VNF_CONFIG_END: fields.ErrorPoint.VNF_CONFIG_END:
@ -2075,7 +2067,7 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
self._delete_placement(context, vnf_instance.id) self._delete_placement(context, vnf_instance.id)
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_PENDING_STATUS, 'INACTIVE') constants.PENDING_STATUSES, 'INACTIVE')
vnf_dict['current_error_point'] = \ vnf_dict['current_error_point'] = \
fields.ErrorPoint.NOTIFY_COMPLETED fields.ErrorPoint.NOTIFY_COMPLETED
@ -2099,7 +2091,7 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
except Exception as exc: except Exception as exc:
# set vnf_status to error # set vnf_status to error
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_ALL_STATUSES, 'ERROR') constants.ALL_STATUSES, 'ERROR')
# Update vnf_lcm_op_occs table and send notification "FAILED_TEMP" # Update vnf_lcm_op_occs table and send notification "FAILED_TEMP"
self._send_lcm_op_occ_notification( self._send_lcm_op_occ_notification(
@ -2164,7 +2156,8 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
fields.ErrorPoint.NOTIFY_PROCESSING: fields.ErrorPoint.NOTIFY_PROCESSING:
# update vnf status to PENDING_HEAL # update vnf status to PENDING_HEAL
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_ACTIVE_STATUS, constants.PENDING_HEAL) constants.ACTIVE,
constants.PENDING_HEAL)
if vnf_dict['before_error_point'] <= \ if vnf_dict['before_error_point'] <= \
fields.ErrorPoint.VNF_CONFIG_END: fields.ErrorPoint.VNF_CONFIG_END:
@ -2184,7 +2177,8 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
# update vnf status to ACTIVE # update vnf status to ACTIVE
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_PENDING_STATUS, constants.ACTIVE) constants.PENDING_STATUSES,
constants.ACTIVE)
vnf_dict['current_error_point'] = \ vnf_dict['current_error_point'] = \
fields.ErrorPoint.NOTIFY_COMPLETED fields.ErrorPoint.NOTIFY_COMPLETED
@ -2208,7 +2202,7 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
except Exception as ex: except Exception as ex:
# update vnf_status to 'ERROR' and create event with 'ERROR' status # update vnf_status to 'ERROR' and create event with 'ERROR' status
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_ALL_STATUSES, constants.ERROR, str(ex)) constants.ALL_STATUSES, constants.ERROR)
# call _update_instantiated_vnf_info for notification # call _update_instantiated_vnf_info for notification
self._update_instantiated_vnf_info(context, vnf_instance, self._update_instantiated_vnf_info(context, vnf_instance,
@ -2432,7 +2426,8 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
if vnf_dict['before_error_point'] <= EP.NOTIFY_PROCESSING: if vnf_dict['before_error_point'] <= EP.NOTIFY_PROCESSING:
# update vnf status to PENDING_CHANGE_EXT_CONN # update vnf status to PENDING_CHANGE_EXT_CONN
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_ACTIVE_STATUS, 'PENDING_CHANGE_EXT_CONN') constants.ACTIVE,
'PENDING_CHANGE_EXT_CONN')
self.vnflcm_driver.change_ext_conn_vnf( self.vnflcm_driver.change_ext_conn_vnf(
context, context,
@ -2445,7 +2440,8 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
context, vnf_instance, change_ext_conn_req) context, vnf_instance, change_ext_conn_req)
# update vnf status to ACTIVE # update vnf status to ACTIVE
self._update_vnf_attributes(context, vnf_instance, vnf_dict, self._update_vnf_attributes(context, vnf_instance, vnf_dict,
_PENDING_STATUS, _ACTIVE_STATUS) constants.PENDING_STATUSES,
constants.ACTIVE)
# Update vnf_lcm_op_occs table and send notification "COMPLETED" # Update vnf_lcm_op_occs table and send notification "COMPLETED"
self._send_lcm_op_occ_notification( self._send_lcm_op_occ_notification(
context=context, context=context,
@ -2459,7 +2455,7 @@ class Conductor(manager.Manager, v2_hook.ConductorV2Hook):
except Exception as e: except Exception as e:
# update vnf_status to 'ERROR' and create event with 'ERROR' status # update vnf_status to 'ERROR' and create event with 'ERROR' status
self._change_vnf_status(context, vnf_instance.id, self._change_vnf_status(context, vnf_instance.id,
_ALL_STATUSES, constants.ERROR, str(e)) constants.ALL_STATUSES, constants.ERROR)
LOG.error('Failed to execute operation. error={}'.format(e)) LOG.error('Failed to execute operation. error={}'.format(e))
if vnf_dict['current_error_point'] in [EP.INTERNAL_PROCESSING, if vnf_dict['current_error_point'] in [EP.INTERNAL_PROCESSING,

6
tacker/plugins/common/constants.py
View File

@ -63,12 +63,6 @@ ALL_STATUSES = (
*PENDING_STATUSES, *PENDING_STATUSES,
) )
ACTIVE_PENDING_STATUSES = (
ACTIVE,
PENDING_CREATE,
PENDING_UPDATE
)
POLICY_SCALING = 'tosca.policies.tacker.Scaling' POLICY_SCALING = 'tosca.policies.tacker.Scaling'
POLICY_SCALING_ACTIONS = (ACTION_SCALE_OUT, POLICY_SCALING_ACTIONS = (ACTION_SCALE_OUT,
ACTION_SCALE_IN) = ('out', 'in') ACTION_SCALE_IN) = ('out', 'in')

26
tacker/tests/unit/conductor/test_conductor_server.py
View File

@ -2116,7 +2116,7 @@ class TestConductor(SqlTestCase, unit_base.FixturedTestCase):
self.conductor.heal(self.context, vnf_instance, vnf_dict, self.conductor.heal(self.context, vnf_instance, vnf_dict,
heal_vnf_req, vnf_lcm_op_occs_id) heal_vnf_req, vnf_lcm_op_occs_id)
mock_change_vnf_status.assert_called_with(self.context, mock_change_vnf_status.assert_called_with(self.context,
vnf_instance.id, mock.ANY, constants.ERROR, "") vnf_instance.id, mock.ANY, constants.ERROR)
mock_update_insta_vnf_info.assert_called_with(self.context, mock_update_insta_vnf_info.assert_called_with(self.context,
vnf_instance, heal_vnf_req) vnf_instance, heal_vnf_req)
mock_update_vnf_attributes_stack_param.assert_called_once_with( mock_update_vnf_attributes_stack_param.assert_called_once_with(
@ -3283,12 +3283,10 @@ class TestConductor(SqlTestCase, unit_base.FixturedTestCase):
vnf_dict, vnf_dict,
change_ext_conn_req, change_ext_conn_req,
vnf_lcm_op_occs_id) vnf_lcm_op_occs_id)
mock_change_vnf_status.assert_called_with( mock_change_vnf_status.assert_called_with(self.context, mock.ANY,
self.context, mock.ANY, (constants.ACTIVE,), constants.ACTIVE, constants.PENDING_CHANGE_EXT_CONN)
constants.PENDING_CHANGE_EXT_CONN) mock_update_vnf_attributes.assert_called_with(self.context, mock.ANY,
mock_update_vnf_attributes.assert_called_with( mock.ANY, mock.ANY, constants.ACTIVE)
self.context, mock.ANY, mock.ANY, mock.ANY,
(constants.ACTIVE,))
self.assertEqual(2, mock_send_notification.call_count) self.assertEqual(2, mock_send_notification.call_count)
self.assertEqual('PROCESSING', op_states[0]) self.assertEqual('PROCESSING', op_states[0])
@ -3380,10 +3378,9 @@ class TestConductor(SqlTestCase, unit_base.FixturedTestCase):
change_ext_conn_req, change_ext_conn_req,
vnf_lcm_op_occs_id) vnf_lcm_op_occs_id)
mock_change_vnf_status.assert_called_with(self.context, mock_change_vnf_status.assert_called_with(self.context,
mock.ANY, (constants.ACTIVE,), mock.ANY, constants.ACTIVE, constants.PENDING_CHANGE_EXT_CONN)
constants.PENDING_CHANGE_EXT_CONN) mock_update_vnf_attributes.assert_called_with(self.context, mock.ANY,
mock_update_vnf_attributes.assert_called_with(self.context, mock.ANY, mock.ANY, constants.ACTIVE)
mock.ANY, mock.ANY, mock.ANY, (constants.ACTIVE,))
self.assertEqual(2, mock_send_notification.call_count) self.assertEqual(2, mock_send_notification.call_count)
self.assertEqual('PROCESSING', op_states[0]) self.assertEqual('PROCESSING', op_states[0])
@ -3564,7 +3561,7 @@ class TestConductor(SqlTestCase, unit_base.FixturedTestCase):
change_ext_conn_req, change_ext_conn_req,
vnf_lcm_op_occs_id) vnf_lcm_op_occs_id)
mock_change_vnf_status.assert_called_with(self.context, mock_change_vnf_status.assert_called_with(self.context,
mock.ANY, mock.ANY, constants.ERROR, mock.ANY) mock.ANY, mock.ANY, constants.ERROR)
self.vnflcm_driver.change_ext_conn_vnf.assert_called_once_with( self.vnflcm_driver.change_ext_conn_vnf.assert_called_once_with(
self.context, vnf_instance, vnf_dict, change_ext_conn_req) self.context, vnf_instance, vnf_dict, change_ext_conn_req)
mock_update_vnf_info_change_ext_conn.assert_called_once() mock_update_vnf_info_change_ext_conn.assert_called_once()
@ -3618,9 +3615,8 @@ class TestConductor(SqlTestCase, unit_base.FixturedTestCase):
vnf_lcm_op_occs_id) vnf_lcm_op_occs_id)
self.vnflcm_driver.change_ext_conn_vnf.assert_called_once_with( self.vnflcm_driver.change_ext_conn_vnf.assert_called_once_with(
self.context, vnf_instance, vnf_dict, change_ext_conn_req) self.context, vnf_instance, vnf_dict, change_ext_conn_req)
mock_change_vnf_status.assert_called_with(self.context, mock_change_vnf_status.assert_called_with(self.context, mock.ANY,
mock.ANY, (constants.ACTIVE,), constants.ACTIVE, constants.PENDING_CHANGE_EXT_CONN)
constants.PENDING_CHANGE_EXT_CONN)
self.assertEqual(mock_change_ext_conn_grant.call_count, 0) self.assertEqual(mock_change_ext_conn_grant.call_count, 0)
mock_update_vnf_attributes.assert_called_once() mock_update_vnf_attributes.assert_called_once()