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 <hiromu.asahina.az@hco.ntt.co.jp>

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")
 

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,

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,

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
+            for _ in range(scale_vnf_request.number_of_steps):
-            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):
                 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):

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

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)