openstack/tacker
Code Issues Proposed changes

Refactor v1 vnflcm_driver

Browse Source 
The following points in the vnflcm_driver reduce the readability and
might be potential bugs.

(i) There are some unused variables, e.g., ``scale_id_list`` in
``_scale_vnf_pre``.

(ii) ``else`` in ``_scale_vnf_pre`` looks inappropriate because all
unknown request types are considered as 'SCALE_OUT'.

(iii) ``_scale_vnf`` is redundant. It can be merged into `scale`.

This patch solves these problems as follows.

(i) Removed unused variables.

(ii) Wrote ``elif scale_vnf_request.type == 'SCALE_OUT':`` explicitly.

(iii) Merged ``_scale_vnf`` and ``scale`` and renamed ``scale`` to
``_scale_vnf`` as most LCM operation methods consist of a pair of a
protected method and public method, e.g., `heal_vnf and _heal_vnf`.

Change-Id: I49457aa35458a68f2294c229b440ba1c68cbfe91
This commit is contained in:
Hiromu Asahina
2022-02-04 20:20:59 +09:00
parent 4d6031883c
commit 45ac7ec7b9
2 changed files with 87 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

92
tacker/tests/unit/vnflcm/test_vnflcm_driver.py
View File

@@ -1797,6 +1797,7 @@ class TestVnflcmDriver(db_base.SqlTestCase):
@mock.patch.object(driver_manager.DriverManager, "invoke")
def test_scale_true(self, mock_invoke, mock_init_hash,
mock_get_service_plugins):
op_occ = mock.MagicMock()
mock_init_hash.return_value = {
"vnflcm_noop": "ffea638bfdbde3fb01f191bbe75b031859"
"b18d663b127100eb72b19eecd7ed51"
@@ -1810,11 +1811,18 @@ class TestVnflcmDriver(db_base.SqlTestCase):
vnf_info['vnf_lcm_op_occ'] = mock.ANY
vim_connection_info = vim_connection.VimConnectionInfo(
vim_type="openstack")
update = {'vim_connection_info': [vim_connection_info]}
scale_status = objects.ScaleInfo(aspect_id='vdu1_aspect',
scale_level=1)
vnf_instance = fakes.return_vnf_instance(
fields.VnfInstanceState.INSTANTIATED, scale_status=scale_status,
**update)
scale_name_list = ["fake"]
grp_id = "fake_id"
driver = vnflcm_driver.VnfLcmDriver()
driver.scale(self.context, vnf_info, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id)
driver._scale_vnf(self.context, vnf_info, vnf_instance,
scale_vnf_request, vim_connection_info,
scale_name_list, grp_id, op_occ)
@mock.patch.object(TackerManager, 'get_service_plugins',
return_value={'VNFM': FakeVNFMPlugin()})
@@ -1825,6 +1833,7 @@ class TestVnflcmDriver(db_base.SqlTestCase):
def test_scale_false_in(self, mock_invoke, mock_safe_load,
mock_init_hash,
mock_get_service_plugins):
op_occ = mock.MagicMock()
mock_init_hash.return_value = {
"vnflcm_noop": "ffea638bfdbde3fb01f191bbe75b031859"
"b18d663b127100eb72b19eecd7ed51"
@@ -1838,14 +1847,22 @@ class TestVnflcmDriver(db_base.SqlTestCase):
vnf_info['vnf_lcm_op_occ'] = mock.ANY
vim_connection_info = vim_connection.VimConnectionInfo(
vim_type="openstack")
update = {'vim_connection_info': [vim_connection_info]}
scale_status = objects.ScaleInfo(aspect_id='vdu1_aspect',
scale_level=1)
vnf_instance = fakes.return_vnf_instance(
fields.VnfInstanceState.INSTANTIATED, scale_status=scale_status,
**update)
scale_name_list = ["fake"]
grp_id = "fake_id"
with open(vnf_info["attributes"]["heat_template"], "r") as f:
mock_safe_load.return_value = yaml.safe_load(f)
print(mock_safe_load.return_value)
driver = vnflcm_driver.VnfLcmDriver()
driver.scale(self.context, vnf_info, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id)
driver._scale_vnf(self.context, vnf_info, vnf_instance,
scale_vnf_request, vim_connection_info,
scale_name_list, grp_id, op_occ)
@mock.patch.object(TackerManager, 'get_service_plugins',
return_value={'VNFM': FakeVNFMPlugin()})
@@ -1856,6 +1873,7 @@ class TestVnflcmDriver(db_base.SqlTestCase):
def test_scale_false_out_initial(self, mock_invoke, mock_safe_load,
mock_init_hash,
mock_get_service_plugins):
op_occ = mock.MagicMock()
mock_init_hash.return_value = {
"vnflcm_noop": "ffea638bfdbde3fb01f191bbe75b031859"
"b18d663b127100eb72b19eecd7ed51"
@@ -1869,14 +1887,22 @@ class TestVnflcmDriver(db_base.SqlTestCase):
vnf_info['vnf_lcm_op_occ'] = mock.ANY
vim_connection_info = vim_connection.VimConnectionInfo(
vim_type="openstack")
update = {'vim_connection_info': [vim_connection_info]}
scale_status = objects.ScaleInfo(aspect_id='vdu1_aspect',
scale_level=1)
vnf_instance = fakes.return_vnf_instance(
fields.VnfInstanceState.INSTANTIATED, scale_status=scale_status,
**update)
scale_name_list = ["fake"]
grp_id = "fake_id"
with open(vnf_info["attributes"]["heat_template"], "r") as f:
mock_safe_load.return_value = yaml.safe_load(f)
print(mock_safe_load.return_value)
driver = vnflcm_driver.VnfLcmDriver()
driver.scale(self.context, vnf_info, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id)
driver._scale_vnf(self.context, vnf_info, vnf_instance,
scale_vnf_request, vim_connection_info,
scale_name_list, grp_id, op_occ)
@mock.patch.object(TackerManager, 'get_service_plugins',
return_value={'VNFM': FakeVNFMPlugin()})
@@ -1887,6 +1913,7 @@ class TestVnflcmDriver(db_base.SqlTestCase):
def test_scale_false_out_level_up(self, mock_invoke, mock_safe_load,
mock_init_hash,
mock_get_service_plugins):
op_occ = mock.MagicMock()
mock_init_hash.return_value = {
"vnflcm_noop": "ffea638bfdbde3fb01f191bbe75b031859"
"b18d663b127100eb72b19eecd7ed51"
@@ -1900,14 +1927,21 @@ class TestVnflcmDriver(db_base.SqlTestCase):
vnf_info['vnf_lcm_op_occ'] = mock.ANY
vim_connection_info = vim_connection.VimConnectionInfo(
vim_type="openstack")
update = {'vim_connection_info': [vim_connection_info]}
scale_status = objects.ScaleInfo(
aspect_id='vdu1_aspect', scale_level=1)
vnf_instance = fakes.return_vnf_instance(
fields.VnfInstanceState.INSTANTIATED, scale_status=scale_status,
**update)
scale_name_list = ["fake"]
grp_id = "fake_id"
with open(vnf_info["attributes"]["heat_template"], "r") as f:
mock_safe_load.return_value = yaml.safe_load(f)
print(mock_safe_load.return_value)
driver = vnflcm_driver.VnfLcmDriver()
driver.scale(self.context, vnf_info, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id)
driver._scale_vnf(self.context, vnf_info, vnf_instance,
scale_vnf_request, vim_connection_info,
scale_name_list, grp_id, op_occ)
@mock.patch.object(TackerManager, 'get_service_plugins',
return_value={'VNFM': FakeVNFMPlugin()})
@@ -2009,6 +2043,7 @@ class TestVnflcmDriver(db_base.SqlTestCase):
@mock.patch.object(driver_manager.DriverManager, "invoke")
def test_scale_scale_type_unknown(self, mock_invoke, mock_init_hash,
mock_get_service_plugins):
op_occ = mock.MagicMock()
mock_init_hash.return_value = {
"vnflcm_noop": "ffea638bfdbde3fb01f191bbe75b031859"
"b18d663b127100eb72b19eecd7ed51"
@@ -2021,20 +2056,22 @@ class TestVnflcmDriver(db_base.SqlTestCase):
scale_vnf_request = fakes.scale_request("UNKNOWN", "SP1", 1, "True")
vim_connection_info = vim_connection.VimConnectionInfo(
vim_type="openstack")
update = {'vim_connection_info': [vim_connection_info]}
scale_status = objects.ScaleInfo(
aspect_id='vdu1_aspect', scale_level=1)
vnf_instance = fakes.return_vnf_instance(
fields.VnfInstanceState.INSTANTIATED, scale_status=scale_status,
**update)
scale_name_list = ["fake"]
grp_id = "fake_id"
driver = vnflcm_driver.VnfLcmDriver()
msg = 'Unknown scale type'
self.assertRaisesRegex(exceptions.VnfScaleFailed,
msg,
driver.scale,
self.context,
vnf_info,
scale_vnf_request,
vim_connection_info,
scale_name_list,
grp_id)
self.assertRaisesRegex(exceptions.VnfScaleFailed, msg,
driver._scale_vnf, self.context, vnf_info,
vnf_instance, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id,
op_occ)
@mock.patch.object(TackerManager, 'get_service_plugins',
return_value={'VNFM': FakeVNFMPlugin()})
@@ -2043,6 +2080,7 @@ class TestVnflcmDriver(db_base.SqlTestCase):
@mock.patch.object(driver_manager.DriverManager, "invoke")
def test_scale_vim_type_unknown(self, mock_invoke, mock_init_hash,
mock_get_service_plugins):
op_occ = mock.MagicMock()
mock_init_hash.return_value = {
"vnflcm_noop": "ffea638bfdbde3fb01f191bbe75b031859"
"b18d663b127100eb72b19eecd7ed51"
@@ -2055,20 +2093,22 @@ class TestVnflcmDriver(db_base.SqlTestCase):
scale_vnf_request = fakes.scale_request("SCALE_OUT", "SP1", 1, "True")
vim_connection_info = vim_connection.VimConnectionInfo(
vim_type="unknown")
update = {'vim_connection_info': [vim_connection_info]}
scale_status = objects.ScaleInfo(
aspect_id='vdu1_aspect', scale_level=1)
vnf_instance = fakes.return_vnf_instance(
fields.VnfInstanceState.INSTANTIATED, scale_status=scale_status,
**update)
scale_name_list = ["fake"]
grp_id = "fake_id"
driver = vnflcm_driver.VnfLcmDriver()
msg = 'Unknown vim type'
self.assertRaisesRegex(exceptions.VnfScaleFailed,
msg,
driver.scale,
self.context,
vnf_info,
scale_vnf_request,
vim_connection_info,
scale_name_list,
grp_id)
self.assertRaisesRegex(exceptions.VnfScaleFailed, msg,
driver._scale_vnf, self.context, vnf_info,
vnf_instance, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id,
op_occ)
@mock.patch.object(TackerManager, 'get_service_plugins',
return_value={'VNFM': FakeVNFMPlugin()})

54
tacker/vnflcm/vnflcm_driver.py
View File

@@ -879,7 +879,6 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
+ scale_vnf_request.type)
vnf_info['current_error_point'] = EP.VNF_CONFIG_START
scale_id_list = []
scale_name_list = []
grp_id = None
vnf_info['policy_name'] = scale_vnf_request.aspect_id
@@ -922,7 +921,7 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
scale_vnf_request=scale_vnf_request,
grant=vnf_info.get('grant'), grant_request=None,
**kwargs)
else:
elif scale_vnf_request.type == 'SCALE_OUT':
vnf_info['action'] = 'out'
scale_id_list = self._vnf_manager.invoke(
vim_connection_info.vim_type,
@@ -933,6 +932,10 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
auth_attr=vim_connection_info.access_info,
region_name=vim_connection_info.access_info.get('region_name')
)
else:
msg = 'Unknown vim type: %s' % vim_connection_info.vim_type
raise exceptions.VnfScaleFailed(id=vnf_info['instance_id'],
error=msg)
vnf_info['current_error_point'] = EP.PRE_VIM_CONTROL
return scale_id_list, scale_name_list, grp_id
@@ -950,7 +953,6 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
scale_vnf_request, vim_connection_info,
scale_id_list,
resource_changes):
vnf_lcm_op_occ = vnf_info['vnf_lcm_op_occ']
vnf_info['current_error_point'] = EP.VNF_CONFIG_END
if scale_vnf_request.type == 'SCALE_OUT':
vnfd_dict = vnflcm_utils._get_vnfd_dict(
@@ -966,7 +968,6 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
auth_attr=vim_connection_info.access_info,
region_name=vim_connection_info.access_info.get('region_name')
)
id_list = []
id_list = list(set(scale_id_after) - set(scale_id_list))
vnf_info['res_num'] = len(scale_id_after)
@@ -1185,15 +1186,6 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
vnf_info['resource_changes'] = resource_changes
return resource_changes
def _scale_vnf(self, context, vnf_info, vnf_instance,
scale_vnf_request, vim_connection_info,
scale_name_list, grp_id):
# action_driver
LOG.debug("vnf_info['vnfd']['attributes'] %s",
vnf_info['vnfd']['attributes'])
self.scale(context, vnf_info, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id)
@log.log
@revert_to_error_scale
def scale_vnf(self, context, vnf_info, vnf_instance, scale_vnf_request):
@@ -1225,11 +1217,9 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
vim_connection_info)
if vnf_info['before_error_point'] <= EP.POST_VIM_CONTROL:
self._scale_vnf(context, vnf_info,
vnf_instance,
scale_vnf_request,
vim_connection_info,
scale_name_list, grp_id)
self._scale_vnf(context, vnf_info, vnf_instance, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id,
vnf_lcm_op_occ)
resource_changes = self._scale_resource_update(context, vnf_info,
vnf_instance,
@@ -1248,14 +1238,13 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
LOG.info("Request received for scale vnf '%s' is completed "
"successfully", vnf_instance.id)
def scale(
self,
context,
vnf_info,
scale_vnf_request,
vim_connection_info,
scale_name_list,
grp_id):
def _scale_vnf(self, context, vnf_info, vnf_instance, scale_vnf_request,
vim_connection_info, scale_name_list, grp_id,
vnf_lcm_op_occ):
# action_driver
LOG.debug("vnf_info['vnfd']['attributes'] %s", (vnf_info
.get('vnfd', {})
.get('attributes')))
self._vnf_manager = driver_manager.DriverManager(
'tacker.tacker.vnfm.drivers',
cfg.CONF.tacker.infra_driver)
@@ -1266,10 +1255,11 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
action = 'out'
else:
msg = 'Unknown scale type: %s' % scale_vnf_request.type
raise exceptions.VnfScaleFailed(id=vnf_info['instance_id'],
error=msg)
raise exceptions.VnfScaleFailed(id=vnf_instance.id, error=msg)
policy = {'instance_id': vnf_info['instance_id'],
stack_id = vnf_instance.instantiated_vnf_info.instance_id
# TODO(h-asahina): change the key name `instance_id` attr to `stack_id`
policy = {'instance_id': stack_id,
'name': scale_vnf_request.aspect_id,
'vnf': vnf_info,
'action': action}
@@ -1279,8 +1269,7 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
scale_vnf_request.additional_params.get('is_reverse'))
default = None
if vim_connection_info.vim_type == 'kubernetes':
policy['vnf_instance_id'] = \
vnf_info['vnf_lcm_op_occ'].get('vnf_instance_id')
policy['vnf_instance_id'] = vnf_lcm_op_occ.get('vnf_instance_id')
vnf_instance = objects.VnfInstance.get_by_id(context,
policy['vnf_instance_id'])
vnfd_dict = vnflcm_utils._get_vnfd_dict(context,
@@ -1306,8 +1295,7 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver):
default = scale_group_dict['scaleGroupDict'][key_aspect]['default']
else:
msg = 'Unknown vim type: %s' % vim_connection_info.vim_type
raise exceptions.VnfScaleFailed(id=vnf_info['instance_id'],
error=msg)
raise exceptions.VnfScaleFailed(id=vnf_instance.id, error=msg)
if (scale_vnf_request.type == 'SCALE_IN' and
scale_vnf_request.additional_params['is_reverse'] == 'True'):