Merge "Validate parameters before updating"

tripleo_common/actions/parameters.py

@@ -13,6 +13,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import copy
 import json
 import logging
 import uuid
@@ -121,7 +122,7 @@ class ResetParametersAction(base.TripleOAction):
         return env
 
 
-class UpdateParametersAction(base.TripleOAction):
+class UpdateParametersAction(templates.ProcessTemplatesAction):
     """Updates plan environment with parameters."""
 
     def __init__(self, parameters,
@@ -134,6 +135,7 @@ class UpdateParametersAction(base.TripleOAction):
 
     def run(self, context):
         swift = self.get_object_client(context)
+        heat = self.get_orchestration_client(context)
 
         try:
             env = plan_utils.get_env(swift, self.container)
@@ -143,19 +145,71 @@ class UpdateParametersAction(base.TripleOAction):
             LOG.exception(err_msg)
             return actions.Result(error=err_msg)
 
+        saved_env = copy.deepcopy(env)
         try:
+
             plan_utils.update_in_env(swift, env, self.key,
                                      self.parameters)
+
         except swiftexceptions.ClientException as err:
             err_msg = ("Error updating environment for plan %s: %s" % (
                 self.container, err))
             LOG.exception(err_msg)
             return actions.Result(error=err_msg)
 
-        self.cache_delete(context,
+        processed_data = super(UpdateParametersAction, self).run(context)
+
+        # If we receive a 'Result' instance it is because the parent action
+        # had an error.
+        if isinstance(processed_data, actions.Result):
+            return processed_data
+
+        processed_data['show_nested'] = True
+        env = plan_utils.get_env(swift, self.container)
+
+        params = env.get('parameter_defaults')
+        fields = {
+            'template': processed_data['template'],
+            'files': processed_data['files'],
+            'environment': processed_data['environment'],
+            'show_nested': True
+        }
+
+        try:
+            result = {
+                'heat_resource_tree': heat.stacks.validate(**fields),
+                'environment_parameters': params,
+            }
+
+            # Validation passes so the old cache gets replaced.
+            self.cache_set(context,
                            self.container,
-                          "tripleo.parameters.get")
+                           "tripleo.parameters.get",
-        return env
+                           result)
+
+            if result['heat_resource_tree']:
+                flattened = {'resources': {}, 'parameters': {}}
+                _flat_it(flattened, 'Root',
+                         result['heat_resource_tree'])
+                result['heat_resource_tree'] = flattened
+
+        except heat_exc.HTTPException as err:
+            LOG.debug("Validation failed rebuilding saved env")
+
+            # There has been an error validating we must reprocess the
+            # templates with the saved working env
+            plan_utils.put_env(swift, saved_env)
+            env = saved_env
+            processed_data = super(UpdateParametersAction, self).run(context)
+
+            err_msg = ("Error validating environment for plan %s: %s" % (
+                self.container, err))
+            LOG.exception(err_msg)
+            return actions.Result(error=err_msg)
+
+        LOG.debug("Validation worked new env is saved")
+
+        return result
 
 
 class UpdateRoleParametersAction(UpdateParametersAction):
@@ -380,47 +434,6 @@ class GetFlattenedParametersAction(GetParametersAction):
     def __init__(self, container=constants.DEFAULT_CONTAINER_NAME):
         super(GetFlattenedParametersAction, self).__init__(container)
 
-    def _process_params(self, flattened, params):
-        for item in params:
-            if item not in flattened['parameters']:
-                param_obj = {}
-                for key, value in params.get(item).items():
-                    camel_case_key = key[0].lower() + key[1:]
-                    param_obj[camel_case_key] = value
-                param_obj['name'] = item
-                flattened['parameters'][item] = param_obj
-        return list(params)
-
-    def _process(self, flattened, name, data):
-        key = str(uuid.uuid4())
-        value = {}
-        value.update({
-            'name': name,
-            'id': key
-        })
-        if 'Type' in data:
-            value['type'] = data['Type']
-        if 'Description' in data:
-            value['description'] = data['Description']
-        if 'Parameters' in data:
-            value['parameters'] = self._process_params(flattened,
-                                                       data['Parameters'])
-        if 'ParameterGroups' in data:
-            value['parameter_groups'] = data['ParameterGroups']
-        if 'NestedParameters' in data:
-            nested = data['NestedParameters']
-            nested_ids = []
-            for nested_key in nested.keys():
-                nested_data = self._process(flattened, nested_key,
-                                            nested.get(nested_key))
-                # nested_data will always have one key (and only one)
-                nested_ids.append(list(nested_data)[0])
-
-            value['resources'] = nested_ids
-
-        flattened['resources'][key] = value
-        return {key: value}
-
     def run(self, context):
         # process all plan files and create or update a stack
         processed_data = super(GetFlattenedParametersAction, self).run(context)
@@ -432,13 +445,56 @@ class GetFlattenedParametersAction(GetParametersAction):
 
         if processed_data['heat_resource_tree']:
             flattened = {'resources': {}, 'parameters': {}}
-            self._process(flattened, 'Root',
+            _flat_it(flattened, 'Root',
                      processed_data['heat_resource_tree'])
             processed_data['heat_resource_tree'] = flattened
 
         return processed_data
 
 
+def _process_params(flattened, params):
+    for item in params:
+        if item not in flattened['parameters']:
+            param_obj = {}
+            for key, value in params.get(item).items():
+                camel_case_key = key[0].lower() + key[1:]
+                param_obj[camel_case_key] = value
+            param_obj['name'] = item
+            flattened['parameters'][item] = param_obj
+    return list(params)
+
+
+def _flat_it(flattened, name, data):
+    key = str(uuid.uuid4())
+    value = {}
+    value.update({
+        'name': name,
+        'id': key
+    })
+    if 'Type' in data:
+        value['type'] = data['Type']
+    if 'Description' in data:
+        value['description'] = data['Description']
+    if 'Parameters' in data:
+        value['parameters'] = _process_params(flattened,
+                                              data['Parameters'])
+    if 'ParameterGroups' in data:
+        value['parameter_groups'] = data['ParameterGroups']
+    if 'NestedParameters' in data:
+        nested = data['NestedParameters']
+        nested_ids = []
+        for nested_key in nested.keys():
+            nested_data = _flat_it(flattened, nested_key,
+                                   nested.get(nested_key))
+            # nested_data will always have one key (and only one)
+            nested_ids.append(list(nested_data)[0])
+
+        value['resources'] = nested_ids
+
+    flattened['resources'][key] = value
+    return {key: value}
+
+
 class GetProfileOfFlavorAction(base.TripleOAction):
     """Gets the profile name for a given flavor name.
 

tripleo_common/actions/templates.py

@@ -348,8 +348,8 @@ class ProcessTemplatesAction(base.TripleOAction):
             LOG.exception("Error occurred while processing custom roles.")
             return actions.Result(error=six.text_type(err))
 
-        template_name = plan_env.get('template')
+        template_name = plan_env.get('template', "")
-        environments = plan_env.get('environments')
+        environments = plan_env.get('environments', [])
         env_paths = []
         temp_files = []
 

tripleo_common/tests/actions/test_parameters.py

@@ -260,27 +260,100 @@ class ResetParametersActionTest(base.TestCase):
 
 class UpdateParametersActionTest(base.TestCase):
 
+    @mock.patch('tripleo_common.actions.parameters.uuid')
+    @mock.patch('heatclient.common.template_utils.'
+                'process_multiple_environments_and_files')
+    @mock.patch('heatclient.common.template_utils.'
+                'get_template_contents')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
-                'cache_delete')
+                'cache_set')
-    @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
-    def test_run(self, mock_get_object_client, mock_cache):
+                'get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
+                'get_orchestration_client')
+    def test_run(self, mock_get_orchestration_client_client,
+                 mock_get_object_client, mock_cache,
+                 mock_get_template_contents, mock_env_files,
+                 mock_uuid):
+
+        mock_env_files.return_value = ({}, {})
 
         mock_ctx = mock.MagicMock()
 
         swift = mock.MagicMock(url="http://test.com")
+
         mock_env = yaml.safe_dump({
             'name': constants.DEFAULT_CONTAINER_NAME,
             'temp_environment': 'temp_environment',
             'template': 'template',
             'environments': [{u'path': u'environments/test.yaml'}],
         }, default_flow_style=False)
-        swift.get_object.return_value = ({}, mock_env)
+
+        mock_roles = yaml.safe_dump([{"name": "foo"}])
+        mock_network = yaml.safe_dump([{'enabled': False}])
+        mock_exclude = yaml.safe_dump({"name": "foo"})
+
+        swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            swiftexceptions.ClientException('atest2')
+        )
+
+        def return_container_files(*args):
+            return ('headers', [{'name': 'foo.role.j2.yaml'}])
+
+        swift.get_container = mock.MagicMock(
+            side_effect=return_container_files)
+
         mock_get_object_client.return_value = swift
 
+        mock_heat = mock.MagicMock()
+        mock_get_orchestration_client_client.return_value = mock_heat
+
+        mock_heat.stacks.validate.return_value = {
+            "Type": "Foo",
+            "Description": "Le foo bar",
+            "Parameters": {"bar": {"foo": "bar barz"}},
+            "NestedParameters": {"Type": "foobar"}
+        }
+
+        mock_uuid.uuid4.return_value = "cheese"
+
+        expected_value = {
+            'environment_parameters': None,
+            'heat_resource_tree': {
+                'parameters': {'bar': {'foo': 'bar barz',
+                                       'name': 'bar'}},
+                'resources': {'cheese': {
+                    'id': 'cheese',
+                    'name': 'Root',
+                    'description': 'Le foo bar',
+                    'parameters': ['bar'],
+                    'resources': ['cheese'],
+                    'type': 'Foo'}
+                }
+            }
+        }
+
+        mock_get_template_contents.return_value = ({}, {
+            'heat_template_version': '2016-04-30'
+        })
+
         # Test
         test_parameters = {'SomeTestParameter': 42}
         action = parameters.UpdateParametersAction(test_parameters)
-        action.run(mock_ctx)
+        return_value = action.run(mock_ctx)
 
         mock_env_updated = yaml.safe_dump({
             'name': constants.DEFAULT_CONTAINER_NAME,
@@ -290,21 +363,42 @@ class UpdateParametersActionTest(base.TestCase):
             'environments': [{u'path': u'environments/test.yaml'}]
         }, default_flow_style=False)
 
-        swift.put_object.assert_called_once_with(
+        swift.put_object.assert_any_call(
             constants.DEFAULT_CONTAINER_NAME,
             constants.PLAN_ENVIRONMENT,
             mock_env_updated
         )
+
+        mock_heat.stacks.validate.assert_called_once_with(
+            environment={},
+            files={},
+            show_nested=True,
+            template={'heat_template_version': '2016-04-30'},
+        )
+
         mock_cache.assert_called_once_with(
             mock_ctx,
             "overcloud",
-            "tripleo.parameters.get"
+            "tripleo.parameters.get",
+            expected_value
         )
+        self.assertEqual(return_value, expected_value)
 
+    @mock.patch('heatclient.common.template_utils.'
+                'process_multiple_environments_and_files')
+    @mock.patch('heatclient.common.template_utils.'
+                'get_template_contents')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
-                'cache_delete')
+                'cache_set')
-    @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
-    def test_run_new_key(self, mock_get_object_client, mock_cache):
+                'get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
+                'get_orchestration_client')
+    def test_run_new_key(self, mock_get_orchestration_client_client,
+                         mock_get_object_client, mock_cache,
+                         mock_get_template_contents, mock_env_files):
+
+        mock_env_files.return_value = ({}, {})
 
         mock_ctx = mock.MagicMock()
 
@@ -315,9 +409,44 @@ class UpdateParametersActionTest(base.TestCase):
             'template': 'template',
             'environments': [{u'path': u'environments/test.yaml'}],
         }, default_flow_style=False)
-        swift.get_object.return_value = ({}, mock_env)
+
+        mock_roles = yaml.safe_dump([{"name": "foo"}])
+        mock_network = yaml.safe_dump([{'enabled': False}])
+        mock_exclude = yaml.safe_dump({"name": "foo"})
+
+        swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            swiftexceptions.ClientException('atest2')
+        )
+
+        def return_container_files(*args):
+            return ('headers', [{'name': 'foo.role.j2.yaml'}])
+
+        swift.get_container = mock.MagicMock(
+            side_effect=return_container_files)
+
         mock_get_object_client.return_value = swift
 
+        heat = mock.MagicMock()
+        heat.stacks.validate.return_value = {}
+        mock_get_orchestration_client_client.return_value = heat
+
+        mock_get_template_contents.return_value = ({}, {
+            'heat_template_version': '2016-04-30'
+        })
+
         # Test
         test_parameters = {'SomeTestParameter': 42}
         action = parameters.UpdateParametersAction(test_parameters,
@@ -332,40 +461,95 @@ class UpdateParametersActionTest(base.TestCase):
             'environments': [{u'path': u'environments/test.yaml'}]
         }, default_flow_style=False)
 
-        swift.put_object.assert_called_once_with(
+        swift.put_object.assert_any_call(
             constants.DEFAULT_CONTAINER_NAME,
             constants.PLAN_ENVIRONMENT,
             mock_env_updated
         )
+
+        heat.stacks.validate.assert_called_once_with(
+            environment={},
+            files={},
+            show_nested=True,
+            template={'heat_template_version': '2016-04-30'},
+        )
+
         mock_cache.assert_called_once_with(
             mock_ctx,
             "overcloud",
-            "tripleo.parameters.get"
+            "tripleo.parameters.get",
+            {'environment_parameters': None, 'heat_resource_tree': {}}
         )
 
 
 class UpdateRoleParametersActionTest(base.TestCase):
 
+    @mock.patch('heatclient.common.template_utils.'
+                'process_multiple_environments_and_files')
+    @mock.patch('heatclient.common.template_utils.'
+                'get_template_contents')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
-                'cache_delete')
+                'cache_set')
     @mock.patch('tripleo_common.utils.parameters.set_count_and_flavor_params')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
                 'get_baremetal_client')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
                 'get_compute_client')
-    @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
-    def test_run(self, mock_get_object_client, mock_get_compute_client,
+                'get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
+                'get_orchestration_client')
+    def test_run(self, mock_get_orchestration_client_client,
+                 mock_get_object_client, mock_get_compute_client,
                  mock_get_baremetal_client, mock_set_count_and_flavor,
-                 mock_cache):
+                 mock_cache, mock_get_template_contents, mock_env_files):
+
+        mock_env_files.return_value = ({}, {})
 
         mock_ctx = mock.MagicMock()
 
         swift = mock.MagicMock(url="http://test.com")
-        mock_env = yaml.safe_dump({'name': 'overcast'},
+        mock_env = yaml.safe_dump({
-                                  default_flow_style=False)
+            'name': 'overcast'
-        swift.get_object.return_value = ({}, mock_env)
+        }, default_flow_style=False)
+
+        mock_roles = yaml.safe_dump([{"name": "foo"}])
+        mock_network = yaml.safe_dump([{'enabled': False}])
+        mock_exclude = yaml.safe_dump({"name": "foo"})
+
+        swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            swiftexceptions.ClientException('atest2')
+        )
+
+        def return_container_files(*args):
+            return ('headers', [{'name': 'foo.yaml'}])
+
+        swift.get_container = mock.MagicMock(
+            side_effect=return_container_files)
+
         mock_get_object_client.return_value = swift
 
+        heat = mock.MagicMock()
+        heat.stacks.validate.return_value = {}
+        mock_get_orchestration_client_client.return_value = heat
+
+        mock_get_template_contents.return_value = ({}, {
+            'heat_template_version': '2016-04-30'
+        })
+
         params = {'CephStorageCount': 1,
                   'OvercloudCephStorageFlavor': 'ceph-storage'}
         mock_set_count_and_flavor.return_value = params
@@ -376,18 +560,27 @@ class UpdateRoleParametersActionTest(base.TestCase):
 
         mock_env_updated = yaml.safe_dump({
             'name': 'overcast',
-            'parameter_defaults': params,
+            'parameter_defaults': params
         }, default_flow_style=False)
 
-        swift.put_object.assert_called_once_with(
+        swift.put_object.assert_any_call(
             'overcast',
             constants.PLAN_ENVIRONMENT,
             mock_env_updated
         )
+
+        heat.stacks.validate.assert_called_once_with(
+            environment={},
+            files={},
+            show_nested=True,
+            template={'heat_template_version': '2016-04-30'},
+        )
+
         mock_cache.assert_called_once_with(
             mock_ctx,
             "overcast",
-            "tripleo.parameters.get"
+            "tripleo.parameters.get",
+            {'environment_parameters': None, 'heat_resource_tree': {}}
         )
 
 

tripleo_common/tests/actions/test_scale.py

@@ -79,6 +79,7 @@ class ScaleDownActionTest(base.TestCase):
             )
         ]
         heatclient.stacks.get.return_value = mock_stack()
+        heatclient.stacks.validate.return_value = {}
         mock_get_heat_client.return_value = heatclient
 
         mock_ctx = mock.MagicMock()
@@ -89,11 +90,31 @@ class ScaleDownActionTest(base.TestCase):
             'template': 'template',
             'environments': [{u'path': u'environments/test.yaml'}]
         }, default_flow_style=False)
+        mock_roles = yaml.safe_dump([{"name": "foo"}])
+        mock_network = yaml.safe_dump([{'enabled': False}])
+        mock_exclude = yaml.safe_dump({"name": "foo"})
         swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
             ({}, mock_env),
             ({}, mock_env),
             swiftexceptions.ClientException('atest2')
         )
+
+        def return_container_files(*args):
+            return ('headers', [{'name': 'foo.role.j2.yaml'}])
+
+        swift.get_container = mock.MagicMock(
+            side_effect=return_container_files)
         mock_get_object_client.return_value = swift
 
         env = {
@@ -111,6 +132,13 @@ class ScaleDownActionTest(base.TestCase):
             constants.STACK_TIMEOUT_DEFAULT, ['resource_id'], 'stack')
         action.run(mock_ctx)
 
+        heatclient.stacks.validate.assert_called_once_with(
+            environment=env,
+            files={},
+            show_nested=True,
+            template={'heat_template_version': '2016-04-30'}
+        )
+
         heatclient.stacks.update.assert_called_once_with(
             'stack',
             stack_name='stack',
@@ -120,7 +148,7 @@ class ScaleDownActionTest(base.TestCase):
             files={},
             timeout_mins=240)
 
-        mock_cache.assert_called_once_with(
+        mock_cache.assert_called_with(
             mock_ctx,
             "stack",
             "tripleo.parameters.get"