From 2d9521b2bd723141745f8cdbd8fd05ebaccb2e3d Mon Sep 17 00:00:00 2001 From: Hiromu Asahina Date: Tue, 17 Aug 2021 11:50:39 +0900 Subject: [PATCH] Fix scaling during the heat cooldown When scaling occurs during the heat cooldown, VNF is not actually scaled but the scale_level value in the VNF information will change. This bug happens because Heat does not return any error responses when it cannot perform scaling action due to the cooldown, and thus Tacker cannot detect a scaling failure. To fix this bug, this patch changes the ``scale`` method of OpenStack infra-driver to wait for scaling until cooldown ends. Closes-Bug: #1925119 Change-Id: Ieca345d7e46e03756d34f7d00b37ebc9c25d8d8b Signed-off-by: Hiromu Asahina --- tacker/common/exceptions.py | 4 + .../tests/unit/vnflcm/test_vnflcm_driver.py | 68 ++++++++++++++++ .../openstack/test_openstack_driver.py | 78 ++++++++++++++++++- tacker/vnflcm/vnflcm_driver.py | 63 ++++++--------- .../infra_drivers/openstack/heat_client.py | 10 +++ .../vnfm/infra_drivers/openstack/openstack.py | 16 +++- 6 files changed, 196 insertions(+), 43 deletions(-) diff --git a/tacker/common/exceptions.py b/tacker/common/exceptions.py index faad56ebd..19ad46f6e 100644 --- a/tacker/common/exceptions.py +++ b/tacker/common/exceptions.py @@ -301,6 +301,10 @@ class VnfPreInstantiationFailed(TackerException): "%(error)s") +class VnfScaleFailed(TackerException): + message = _("Scale Vnf failed for vnf %(id)s, error: %(error)s") + + class VnfHealFailed(TackerException): message = _("Heal Vnf failed for vnf %(id)s, error: %(error)s") diff --git a/tacker/tests/unit/vnflcm/test_vnflcm_driver.py b/tacker/tests/unit/vnflcm/test_vnflcm_driver.py index 18f15e540..d019465f1 100644 --- a/tacker/tests/unit/vnflcm/test_vnflcm_driver.py +++ b/tacker/tests/unit/vnflcm/test_vnflcm_driver.py @@ -1971,6 +1971,74 @@ class TestVnflcmDriver(db_base.SqlTestCase): driver.scale_vnf(self.context, vnf_info, vnf_instance, scale_vnf_request) + @mock.patch.object(TackerManager, 'get_service_plugins', + return_value={'VNFM': FakeVNFMPlugin()}) + @mock.patch.object(VnfLcmDriver, + '_init_mgmt_driver_hash') + @mock.patch.object(driver_manager.DriverManager, "invoke") + def test_scale_scale_type_unknown(self, mock_invoke, mock_init_hash, + mock_get_service_plugins): + mock_init_hash.return_value = { + "vnflcm_noop": "ffea638bfdbde3fb01f191bbe75b031859" + "b18d663b127100eb72b19eecd7ed51" + } + attributes = {'scale_group': '{\"scaleGroupDict\": ' + + '{ \"SP1\": { \"vdu\": [\"VDU1\"], \"num\": ' + + '1, \"maxLevel\": 3, \"initialNum\": 0, ' + + '\"initialLevel\": 0, \"default\": 0 }}}'} + vnf_info = fakes._get_vnf(attributes=attributes) + scale_vnf_request = fakes.scale_request("UNKNOWN", "SP1", 1, "True") + vim_connection_info = vim_connection.VimConnectionInfo( + vim_type="openstack") + 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) + + @mock.patch.object(TackerManager, 'get_service_plugins', + return_value={'VNFM': FakeVNFMPlugin()}) + @mock.patch.object(VnfLcmDriver, + '_init_mgmt_driver_hash') + @mock.patch.object(driver_manager.DriverManager, "invoke") + def test_scale_vim_type_unknown(self, mock_invoke, mock_init_hash, + mock_get_service_plugins): + mock_init_hash.return_value = { + "vnflcm_noop": "ffea638bfdbde3fb01f191bbe75b031859" + "b18d663b127100eb72b19eecd7ed51" + } + attributes = {'scale_group': '{\"scaleGroupDict\": ' + + '{ \"SP1\": { \"vdu\": [\"VDU1\"], \"num\": ' + + '1, \"maxLevel\": 3, \"initialNum\": 0, ' + + '\"initialLevel\": 0, \"default\": 0 }}}'} + vnf_info = fakes._get_vnf(attributes=attributes) + scale_vnf_request = fakes.scale_request("SCALE_OUT", "SP1", 1, "True") + vim_connection_info = vim_connection.VimConnectionInfo( + vim_type="unknown") + 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) + @mock.patch.object(TackerManager, 'get_service_plugins', return_value={'VNFM': FakeVNFMPlugin()}) @mock.patch.object(VnfLcmDriver, diff --git a/tacker/tests/unit/vnfm/infra_drivers/openstack/test_openstack_driver.py b/tacker/tests/unit/vnfm/infra_drivers/openstack/test_openstack_driver.py index c0e12cbe3..91e506163 100644 --- a/tacker/tests/unit/vnfm/infra_drivers/openstack/test_openstack_driver.py +++ b/tacker/tests/unit/vnfm/infra_drivers/openstack/test_openstack_driver.py @@ -25,6 +25,7 @@ import yaml from heatclient.v1 import resources from oslo_serialization import jsonutils +from oslo_utils import timeutils from tacker.common import exceptions from tacker.common import utils as cutils from tacker import context @@ -1500,7 +1501,18 @@ class TestOpenStack(base.FixturedTestCase): self.requests_mock.register_uri('GET', url, json=json, headers=self.json_headers) - def test_scale(self): + @mock.patch('time.sleep') + @mock.patch('oslo_utils.timeutils.utcnow') + @mock.patch.object(hc.HeatClient, 'resource_metadata') + def test_scale(self, + mock_resource_metadata, + mock_utcnow, + mock_sleep): + + mock_resource_metadata.return_value = {} + mock_utcnow.return_value = timeutils.parse_strtime( + '2000-01-01T00:00:00.000000') + dummy_event = fd_utils.get_dummy_event() self._responses_in_resource_event_list(dummy_event) # response for heat_client's resource_signal() @@ -1512,6 +1524,70 @@ class TestOpenStack(base.FixturedTestCase): auth_attr=None, policy=fd_utils.get_dummy_policy_dict(), region_name=None) + + mock_sleep.assert_not_called() + self.assertEqual(dummy_event['id'], event_id) + + @mock.patch('time.sleep') + @mock.patch('oslo_utils.timeutils.utcnow') + @mock.patch.object(hc.HeatClient, 'resource_metadata') + def test_scale_cooldown(self, + mock_resource_metadata, + mock_utcnow, + mock_sleep): + """A case where cooldown hasn't ended""" + + mock_resource_metadata.return_value = {'cooldown_end': + {'2000-01-01T00:00:01.000000': + 'cooldown_reason'}, + 'scaling_in_progress': 'false'} + mock_utcnow.return_value = timeutils.parse_strtime( + '2000-01-01T00:00:00.000000') + + dummy_event = fd_utils.get_dummy_event() + self._responses_in_resource_event_list(dummy_event) + # response for heat_client's resource_signal() + url = self.heat_url + '/stacks/' + self.instance_uuid + ( + '/myStack/60f83b5e/resources/SP1_scale_out/signal') + self.requests_mock.register_uri('POST', url, json={}, + headers=self.json_headers) + event_id = self.openstack.scale(plugin=self, context=self.context, + auth_attr=None, + policy=fd_utils.get_dummy_policy_dict(), + region_name=None) + + mock_sleep.assert_called_once_with(1) + self.assertEqual(dummy_event['id'], event_id) + + @mock.patch('time.sleep') + @mock.patch('oslo_utils.timeutils.utcnow') + @mock.patch.object(hc.HeatClient, 'resource_metadata') + def test_scale_cooldown_ended(self, + mock_resource_metadata, + mock_utcnow, + mock_sleep): + """A case where cooldown has already ended""" + + mock_resource_metadata.return_value = {'cooldown_end': + {'2000-01-01T00:00:00.000000': + 'cooldown_reason'}, + 'scaling_in_progress': 'false'} + mock_utcnow.return_value = timeutils.parse_strtime( + '2000-01-01T00:00:01.000000') + + dummy_event = fd_utils.get_dummy_event() + self._responses_in_resource_event_list(dummy_event) + # response for heat_client's resource_signal() + url = self.heat_url + '/stacks/' + self.instance_uuid + ( + '/myStack/60f83b5e/resources/SP1_scale_out/signal') + self.requests_mock.register_uri('POST', url, json={}, + headers=self.json_headers) + event_id = self.openstack.scale(plugin=self, context=self.context, + auth_attr=None, + policy=fd_utils.get_dummy_policy_dict(), + region_name=None) + + mock_sleep.assert_not_called() self.assertEqual(dummy_event['id'], event_id) def _response_in_resource_get_list(self, stack_id=None, diff --git a/tacker/vnflcm/vnflcm_driver.py b/tacker/vnflcm/vnflcm_driver.py index a987e204d..f1bc382aa 100644 --- a/tacker/vnflcm/vnflcm_driver.py +++ b/tacker/vnflcm/vnflcm_driver.py @@ -20,9 +20,7 @@ import hashlib import inspect import os import re -import time import traceback -import yaml from oslo_config import cfg from oslo_log import log as logging @@ -1250,14 +1248,21 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver): self._vnf_manager = driver_manager.DriverManager( 'tacker.tacker.vnfm.drivers', cfg.CONF.tacker.infra_driver) - policy = {} - policy['instance_id'] = vnf_info['instance_id'] - policy['name'] = scale_vnf_request.aspect_id - policy['vnf'] = vnf_info + if scale_vnf_request.type == 'SCALE_IN': - policy['action'] = 'in' + action = 'in' + elif scale_vnf_request.type == 'SCALE_OUT': + action = 'out' else: - policy['action'] = 'out' + msg = 'Unknown scale type: %s' % scale_vnf_request.type + raise exceptions.VnfScaleFailed(id=vnf_info['instance_id'], + error=msg) + + policy = {'instance_id': vnf_info['instance_id'], + 'name': scale_vnf_request.aspect_id, + 'vnf': vnf_info, + 'action': action} + LOG.debug( "is_reverse: %s", scale_vnf_request.additional_params.get('is_reverse')) @@ -1280,14 +1285,19 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver): policy['delta_num'] = vnflcm_utils.get_scale_delta_num( extract_policy_infos=extract_policy_infos, aspect_id=scale_vnf_request.aspect_id) - else: + elif vim_connection_info.vim_type == 'openstack': # NOTE(ueha): The logic of Scale for OpenStack VIM is widely hard # coded with `vnf_info`. This dependency is to be refactored in # future. scale_json = vnf_info['attributes']['scale_group'] - scaleGroupDict = jsonutils.loads(scale_json) + scale_group_dict = jsonutils.loads(scale_json) key_aspect = scale_vnf_request.aspect_id - default = scaleGroupDict['scaleGroupDict'][key_aspect]['default'] + 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) + if (scale_vnf_request.type == 'SCALE_IN' and scale_vnf_request.additional_params['is_reverse'] == 'True'): self._vnf_manager.invoke( @@ -1334,33 +1344,7 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver): region_name=vim_connection_info.access_info.get('region_name') ) else: - cooldown = None - if vim_connection_info.vim_type != 'kubernetes': - # NOTE(ueha): The logic of Scale for OpenStack VIM is widely - # hard coded with `vnf_info`. This dependency is to be - # refactored in future. - heat_template = vnf_info['attributes']['heat_template'] - policy_in_name = scale_vnf_request.aspect_id + '_scale_in' - policy_out_name = scale_vnf_request.aspect_id + '_scale_out' - - heat_resource = yaml.safe_load(heat_template) - if scale_vnf_request.type == 'SCALE_IN': - policy['action'] = 'in' - policy_temp = heat_resource['resources'][policy_in_name] - policy_prop = policy_temp['properties'] - cooldown = policy_prop.get('cooldown') - policy_name = policy_in_name - else: - policy['action'] = 'out' - policy_temp = heat_resource['resources'][policy_out_name] - policy_prop = policy_temp['properties'] - cooldown = policy_prop.get('cooldown') - policy_name = policy_out_name - - policy_temp = heat_resource['resources'][policy_name] - policy_prop = policy_temp['properties'] - - for i in range(scale_vnf_request.number_of_steps): + for _ in range(scale_vnf_request.number_of_steps): last_event_id = self._vnf_manager.invoke( vim_connection_info.vim_type, 'scale', @@ -1382,9 +1366,6 @@ class VnfLcmDriver(abstract_driver.VnfInstanceAbstractDriver): region_name=vim_connection_info.access_info.get('\ region_name'), last_event_id=last_event_id) - if i != scale_vnf_request.number_of_steps - 1: - if cooldown: - time.sleep(cooldown) def _term_resource_update(self, context, vnf_info, vnf_instance, error=False): diff --git a/tacker/vnfm/infra_drivers/openstack/heat_client.py b/tacker/vnfm/infra_drivers/openstack/heat_client.py index f01b0c040..6f012fbb9 100644 --- a/tacker/vnfm/infra_drivers/openstack/heat_client.py +++ b/tacker/vnfm/infra_drivers/openstack/heat_client.py @@ -14,6 +14,7 @@ import sys from heatclient import exc as heatException from oslo_log import log as logging +from oslo_utils import timeutils from tacker.common import clients from tacker.extensions import vnfm @@ -45,6 +46,15 @@ class HeatClient(object): stack = sub_stack return stack + def get_cooldown(self, stack_id, rsc_name): + metadata = self.resource_metadata(stack_id, rsc_name) + if 'cooldown_end' in metadata: + now = timeutils.utcnow() + cooldown_end = timeutils.parse_strtime( + next(iter(metadata['cooldown_end'].keys()))) + return max(0, timeutils.delta_seconds(now, cooldown_end)) + return None + def create(self, fields): fields = fields.copy() fields['disable_rollback'] = True diff --git a/tacker/vnfm/infra_drivers/openstack/openstack.py b/tacker/vnfm/infra_drivers/openstack/openstack.py index 0aea411ef..33148fd2d 100644 --- a/tacker/vnfm/infra_drivers/openstack/openstack.py +++ b/tacker/vnfm/infra_drivers/openstack/openstack.py @@ -826,8 +826,21 @@ class OpenStack(abstract_driver.VnfAbstractDriver, policy_rsc, limit=1, sort_dir='desc', sort_keys='event_time') + stack_id = policy['instance_id'] + policy_name = policy['name'] + # Guard for differentiate call from legacy or ETSI code + # TODO(h-asahina) : Find more sophisticated ways to detect legacy + if 'before_error_point' not in policy['vnf']: + policy_name += '_group' - heatclient.resource_signal(policy['instance_id'], policy_rsc) + cooldown = heatclient.get_cooldown(stack_id, policy_name) + if cooldown: + LOG.info('Wait %(cooldown)s seconds for VNF scaling until the ' + 'cooldown of stack %(stack)s ends', + {'cooldown': cooldown, 'stack': policy['instance_id']}) + time.sleep(cooldown) + + heatclient.resource_signal(stack_id, policy_rsc) return events[0].id @log.log @@ -838,6 +851,7 @@ class OpenStack(abstract_driver.VnfAbstractDriver, stack_id = policy['instance_id'] policy_name = policy['name'] # Guard for differentiate call from legacy or ETSI code + # TODO(h-asahina) : Find more sophisticated ways to detect legacy if 'before_error_point' not in policy['vnf']: policy_name += '_group' grp = heatclient.resource_get(stack_id, policy_name)