Merge "Modify roles to remove unused services" into stable/train

tripleo_common/actions/deployment.py

@@ -139,6 +139,7 @@ class DeployStackAction(base.TripleOAction):
         self.container = container
         self.timeout_mins = timeout
         self.skip_deploy_identifier = skip_deploy_identifier
+        self.role_data = None
 
     def run(self, context):
         # check to see if the stack exists
@@ -198,12 +199,31 @@ class DeployStackAction(base.TripleOAction):
             container=self.container
         )
         processed_data = process_templates_action.run(context)
+        self.role_data = process_templates_action.role_data
 
         # If we receive a 'Result' instance it is because the parent action
         # had an error.
         if isinstance(processed_data, actions.Result):
             return processed_data
 
+        # prune roles of unused services after the templates have been
+        # processed
+        environment = processed_data.get('environment', {})
+        resource_reg = environment.get('resource_registry', {})
+        roles_changed = self._prune_unused_services(resource_reg, swift)
+
+        if roles_changed:
+            # reprocess the data with the new role information
+            process_templates_action = templates.ProcessTemplatesAction(
+                container=self.container
+            )
+            processed_data = process_templates_action.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
+
         stack_args = processed_data.copy()
         stack_args['timeout_mins'] = self.timeout_mins
 
@@ -267,6 +287,44 @@ class DeployStackAction(base.TripleOAction):
         orig_camap.update(ca_map_entry)
         return orig_camap
 
+    def _prune_unused_services(self, resource_registry, swift):
+        """Remove unused services from role data
+
+        Finds the unused services in the resource registry and removes them
+        from the role data in the plan so we do not create empty service
+        chain stacks that are not needed.
+
+        :param resource_registry: tripleo resource registry dict
+        :param swift: swift client
+        :returns: true if we updated the roles file. else false
+        """
+        to_remove = set()
+        for key, value in resource_registry.items():
+            if (key.startswith('OS::TripleO::Services::') and
+                    value.startswith('OS::Heat::None')):
+                to_remove.add(key)
+
+        if not to_remove or not self.role_data:
+            LOG.info('No unused services to prune or no role data')
+            return False
+
+        LOG.info('Removing unused services from role data')
+        for role in self.role_data:
+            role_name = role.get('name')
+            for service in to_remove:
+                try:
+                    role.get('ServicesDefault', []).remove(service)
+                    LOG.debug('Removing {} from {} role'.format(
+                        service, role_name))
+                except ValueError:
+                    pass
+        LOG.debug('Saving updated role data to swift')
+        swift.put_object(self.container,
+                         constants.OVERCLOUD_J2_ROLES_NAME,
+                         yaml.safe_dump(self.role_data,
+                                        default_flow_style=False))
+        return True
+
 
 class OvercloudRcAction(base.TripleOAction):
     """Generate the overcloudrc for a plan

tripleo_common/actions/templates.py

@@ -39,16 +39,10 @@ class J2SwiftLoader(jinja2.BaseLoader):
     only the absolute path relative to the container root is searched.
     """
 
-    def __init__(self, swift, container, searchpath=None):
+    def __init__(self, swift, container, searchpath):
         self.swift = swift
         self.container = container
-        if searchpath is not None:
+        self.searchpath = [searchpath]
-            if isinstance(searchpath, six.string_types):
-                self.searchpath = [searchpath]
-            else:
-                self.searchpath = list(searchpath)
-        else:
-            self.searchpath = []
         # Always search the absolute path from the root of the swift container
         if '' not in self.searchpath:
             self.searchpath.append('')
@@ -99,15 +93,9 @@ class ProcessTemplatesAction(base.TripleOAction):
     def __init__(self, container=constants.DEFAULT_CONTAINER_NAME):
         super(ProcessTemplatesAction, self).__init__()
         self.container = container
+        self.role_data = None
 
-    def _j2_render_and_put(self,
+    def _j2_render_and_put(self, j2_template, j2_data, yaml_f, swift):
-                           j2_template,
-                           j2_data,
-                           outfile_name=None,
-                           context=None):
-        swift = self.get_object_client(context)
-        yaml_f = outfile_name or j2_template.replace('.j2.yaml', '.yaml')
-
         # Search for templates relative to the current template path first
         template_base = os.path.dirname(yaml_f)
         j2_loader = J2SwiftLoader(swift, self.container, template_base)
@@ -287,7 +275,7 @@ class ProcessTemplatesAction(base.TripleOAction):
                             self._j2_render_and_put(j2_template,
                                                     j2_data,
                                                     out_f_path,
-                                                    context=context)
+                                                    swift)
                         else:
                             # Backwards compatibility with templates
                             # that specify {{role}} vs {{role.name}}
@@ -296,12 +284,12 @@ class ProcessTemplatesAction(base.TripleOAction):
                             self._j2_render_and_put(j2_template,
                                                     j2_data,
                                                     out_f_path,
-                                                    context=context)
+                                                    swift)
                     else:
                         LOG.info("Skipping rendering of %s, defined in %s" %
                                  (out_f_path, j2_excl_data))
 
-            elif (f.endswith('.network.j2.yaml')):
+            elif f.endswith('.network.j2.yaml'):
                 LOG.info("jinja2 rendering network template %s" % f)
                 j2_template = swiftutils.get_object_string(swift,
                                                            self.container,
@@ -323,7 +311,7 @@ class ProcessTemplatesAction(base.TripleOAction):
                         self._j2_render_and_put(j2_template,
                                                 j2_data,
                                                 out_f_path,
-                                                context=context)
+                                                swift)
                     else:
                         LOG.info("Skipping rendering of %s, defined in %s" %
                                  (out_f_path, j2_excl_data))
@@ -338,7 +326,8 @@ class ProcessTemplatesAction(base.TripleOAction):
                 self._j2_render_and_put(j2_template,
                                         j2_data,
                                         out_f,
-                                        context=context)
+                                        swift)
+        return role_data
 
     def run(self, context):
         error_text = None
@@ -360,7 +349,7 @@ class ProcessTemplatesAction(base.TripleOAction):
             # not found in swift, but if they are found and an exception
             # occurs during processing, that exception will cause the
             # ProcessTemplatesAction to return an error result.
-            self._process_custom_roles(context)
+            self.role_data = self._process_custom_roles(context)
         except Exception as err:
             LOG.exception("Error occurred while processing custom roles.")
             return actions.Result(error=six.text_type(err))

tripleo_common/tests/actions/test_deployment.py

@@ -206,6 +206,8 @@ class OrchestrationDeployActionTest(base.TestCase):
 
 class DeployStackActionTest(base.TestCase):
 
+    @mock.patch('tripleo_common.actions.deployment.DeployStackAction.'
+                '_prune_unused_services', return_value=False)
     @mock.patch('tripleo_common.actions.deployment.time')
     @mock.patch('heatclient.common.template_utils.'
                 'process_multiple_environments_and_files')
@@ -216,7 +218,7 @@ class DeployStackActionTest(base.TestCase):
     def test_run(self, get_orchestration_client_mock,
                  mock_get_object_client, mock_get_template_contents,
                  mock_process_multiple_environments_and_files,
-                 mock_time):
+                 mock_time, mock_prune):
 
         mock_ctx = mock.MagicMock()
         # setup swift
@@ -283,6 +285,71 @@ class DeployStackActionTest(base.TestCase):
             "overcloud-swift-rings", "swift-rings.tar.gz",
             "overcloud-swift-rings/swift-rings.tar.gz-%d" % 1473366264)
 
+    @mock.patch('tripleo_common.utils.plan.update_in_env')
+    @mock.patch('tripleo_common.utils.plan.get_env')
+    @mock.patch('tripleo_common.actions.templates.ProcessTemplatesAction.run')
+    @mock.patch('tripleo_common.actions.deployment.DeployStackAction.'
+                '_prune_unused_services', return_value=True)
+    @mock.patch('tripleo_common.actions.deployment.time')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
+    @mock.patch(
+        'tripleo_common.actions.base.TripleOAction.get_orchestration_client')
+    def test_run_role_changes(self, get_orchestration_client_mock,
+                              mock_get_object_client,
+                              mock_time, mock_prune, mock_template_action,
+                              mock_get_env, mock_update_in_env):
+
+        mock_ctx = mock.MagicMock()
+        # setup swift
+        swift = mock.MagicMock(url="http://test.com")
+        mock_env = yaml.safe_dump({
+            'name': 'overcloud',
+            'temp_environment': 'temp_environment',
+            'template': 'template',
+            'environments': [{u'path': u'environments/test.yaml'}],
+            'parameter_defaults': {'random_existing_data': 'a_value'},
+        }, default_flow_style=False)
+        swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+        )
+        mock_get_object_client.return_value = swift
+
+        heat = mock.MagicMock()
+        heat.stacks.get.return_value = None
+        get_orchestration_client_mock.return_value = heat
+
+        # freeze time at datetime.datetime(2016, 9, 8, 16, 24, 24)
+        mock_time.time.return_value = 1473366264
+
+        mock_template_action.return_value = {
+            'stack_name': 'overcloud',
+            'template': {'heat_template_version': '2016-04-30'},
+            'environment': {},
+            'files': {}
+        }
+
+        action = deployment.DeployStackAction(1, 'overcloud')
+        action.run(mock_ctx)
+
+        mock_prune.assert_called_once()
+        self.assertEqual(mock_template_action.call_count, 2)
+
+        heat.stacks.create.assert_called_once_with(
+            environment={},
+            files={},
+            stack_name='overcloud',
+            template={'heat_template_version': '2016-04-30'},
+            timeout_mins=1,
+        )
+        swift.delete_object.assert_called_once_with(
+            "overcloud-swift-rings", "swift-rings.tar.gz")
+        swift.copy_object.assert_called_once_with(
+            "overcloud-swift-rings", "swift-rings.tar.gz",
+            "overcloud-swift-rings/swift-rings.tar.gz-%d" % 1473366264)
+
+    @mock.patch('tripleo_common.actions.deployment.DeployStackAction.'
+                '_prune_unused_services', return_value=False)
     @mock.patch('tripleo_common.actions.deployment.time')
     @mock.patch('heatclient.common.template_utils.'
                 'process_multiple_environments_and_files')
@@ -294,7 +361,7 @@ class DeployStackActionTest(base.TestCase):
             self, get_orchestration_client_mock,
             mock_get_object_client, mock_get_template_contents,
             mock_process_multiple_environments_and_files,
-            mock_time):
+            mock_time, mock_prune):
 
         mock_ctx = mock.MagicMock()
         # setup swift
@@ -361,6 +428,8 @@ class DeployStackActionTest(base.TestCase):
             "overcloud-swift-rings", "swift-rings.tar.gz",
             "overcloud-swift-rings/swift-rings.tar.gz-%d" % 1473366264)
 
+    @mock.patch('tripleo_common.actions.deployment.DeployStackAction.'
+                '_prune_unused_services', return_value=False)
     @mock.patch('tripleo_common.actions.deployment.time')
     @mock.patch('heatclient.common.template_utils.'
                 'process_multiple_environments_and_files')
@@ -371,7 +440,7 @@ class DeployStackActionTest(base.TestCase):
     def test_run_create_failed(
         self, get_orchestration_client_mock, mock_get_object_client,
         mock_get_template_contents,
-        mock_process_multiple_environments_and_files, mock_time):
+        mock_process_multiple_environments_and_files, mock_time, mock_prune):
 
         mock_ctx = mock.MagicMock()
         # setup swift
@@ -408,6 +477,8 @@ class DeployStackActionTest(base.TestCase):
             error="Error during stack creation: ERROR: Oops\n")
         self.assertEqual(expected, action.run(mock_ctx))
 
+    @mock.patch('tripleo_common.actions.deployment.DeployStackAction.'
+                '_prune_unused_services', return_value=False)
     @mock.patch('tripleo_common.update.check_neutron_mechanism_drivers')
     @mock.patch('tripleo_common.actions.deployment.time')
     @mock.patch('heatclient.common.template_utils.'
@@ -420,7 +491,7 @@ class DeployStackActionTest(base.TestCase):
         self, get_orchestration_client_mock, mock_get_object_client,
         mock_get_template_contents,
         mock_process_multiple_environments_and_files, mock_time,
-        mock_check_neutron_drivers):
+        mock_check_neutron_drivers, mock_prune):
 
         mock_ctx = mock.MagicMock()
         # setup swift
@@ -511,6 +582,61 @@ class DeployStackActionTest(base.TestCase):
             self.assertEqual('ANOTER FAKE CERT',
                              my_params['CAMap']['overcloud-ca']['content'])
 
+    def test_prune_unused_services(self):
+        resource_registry = {
+            'OS::TripleO::Services::Foo': 'bar.yaml',
+            'OS::TripleO::Services::Baz': 'OS::Heat::None',
+        }
+        swift = mock.MagicMock()
+        mock_put = mock.MagicMock()
+        swift.put_object = mock_put
+        action = deployment.DeployStackAction(1, 'overcloud',
+                                              skip_deploy_identifier=True)
+
+        test_role_data = [{
+            'name': 'Controller',
+            'ServicesDefault': [
+                'OS::TripleO::Services::Foo',
+                'OS::TripleO::Services::Baz']
+            }]
+
+        test_role_data_result = [{
+            'name': 'Controller',
+            'ServicesDefault': [
+                'OS::TripleO::Services::Foo']
+            }]
+
+        action.role_data = test_role_data
+
+        action._prune_unused_services(resource_registry, swift)
+
+        data = yaml.safe_dump(test_role_data_result, default_flow_style=False)
+        mock_put.assert_called_once_with('overcloud', 'roles_data.yaml', data)
+
+    def test_prune_unused_services_no_removal(self):
+        resource_registry = {
+            'OS::TripleO::Services::Foo': 'bar.yaml',
+            'OS::TripleO::Services::Baz': 'biz.yaml',
+        }
+        swift = mock.MagicMock()
+        mock_put = mock.MagicMock()
+        swift.put_object = mock_put
+        action = deployment.DeployStackAction(1, 'overcloud',
+                                              skip_deploy_identifier=True)
+
+        test_role_data = [{
+            'name': 'Controller',
+            'ServicesDefault': [
+                'OS::TripleO::Services::Foo',
+                'OS::TripleO::Services::Baz']
+            }]
+
+        action.role_data = test_role_data
+
+        action._prune_unused_services(resource_registry, swift)
+
+        mock_put.assert_not_called()
+
 
 class OvercloudRcActionTestCase(base.TestCase):
     @mock.patch('tripleo_common.actions.base.TripleOAction.'

tripleo_common/tests/actions/test_templates.py

@@ -134,7 +134,7 @@ class J2SwiftLoaderTest(base.TestCase):
         return swift
 
     def test_include_absolute_path(self):
-        j2_loader = templates.J2SwiftLoader(self._setup_swift(), None)
+        j2_loader = templates.J2SwiftLoader(self._setup_swift(), None, '')
         template = jinja2.Environment(loader=j2_loader).from_string(
             r'''
             Included this:
@@ -162,7 +162,7 @@ class J2SwiftLoaderTest(base.TestCase):
             ''')
 
     def test_include_not_found(self):
-        j2_loader = templates.J2SwiftLoader(self._setup_swift(), None)
+        j2_loader = templates.J2SwiftLoader(self._setup_swift(), None, '')
         template = jinja2.Environment(loader=j2_loader).from_string(
             r'''
             Included this:
@@ -173,7 +173,7 @@ class J2SwiftLoaderTest(base.TestCase):
             template.render)
 
     def test_include_invalid_path(self):
-        j2_loader = templates.J2SwiftLoader(self._setup_swift(), 'bar')
+        j2_loader = templates.J2SwiftLoader(self._setup_swift(), 'bar', '')
         template = jinja2.Environment(loader=j2_loader).from_string(
             r'''
             Included this:
@@ -335,13 +335,12 @@ class ProcessTemplatesActionTest(base.TestCase):
         swift.get_object = mock.MagicMock()
         swift.get_container = mock.MagicMock()
         get_obj_client_mock.return_value = swift
-        mock_ctx = mock.MagicMock()
 
         # Test
         action = templates.ProcessTemplatesAction()
         action._j2_render_and_put(JINJA_SNIPPET_CONFIG,
                                   {'role': 'CustomRole'},
-                                  'customrole-config.yaml', context=mock_ctx)
+                                  'customrole-config.yaml', swift)
 
         action_result = swift.put_object._mock_mock_calls[0]
 
@@ -363,14 +362,13 @@ class ProcessTemplatesActionTest(base.TestCase):
         swift.get_container = mock.MagicMock(
             side_effect=return_container_files)
         get_obj_client_mock.return_value = swift
-        mock_ctx = mock.MagicMock()
 
         # Test
         action = templates.ProcessTemplatesAction()
         action._j2_render_and_put(r"{% include 'foo.yaml' %}",
                                   {'role': 'CustomRole'},
                                   'customrole-config.yaml',
-                                  context=mock_ctx)
+                                  swift)
 
         action_result = swift.put_object._mock_mock_calls[0]
 
@@ -394,14 +392,13 @@ class ProcessTemplatesActionTest(base.TestCase):
         swift.get_container = mock.MagicMock(
             side_effect=return_container_files)
         get_obj_client_mock.return_value = swift
-        mock_ctx = mock.MagicMock()
 
         # Test
         action = templates.ProcessTemplatesAction()
         action._j2_render_and_put(r"{% include 'foo.yaml' %}",
                                   {'role': 'CustomRole'},
                                   'bar/customrole-config.yaml',
-                                  context=mock_ctx)
+                                  swift)
 
         action_result = swift.put_object._mock_mock_calls[0]