From 90c79df86a9a9643235adbbd009a2fac37d07a9c Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Fri, 8 Mar 2019 14:37:22 +0000 Subject: [PATCH] Modify roles to remove unused services Historically we've included unused services in all the roles as a way for users to enable the services for a given role. By default many are set to OS::Heat::None. For large scale deployments this results in many extra resources managed by heat. This change adds a function to look at the resource registry of the environment and will remove any unused services from the roles data. Removing these services from the roles data reduces number of heat resource and hence possibly little deployment time. Change-Id: I54539f7dd694e0804006470221e55fd3cafea539 Co-Authored-By: Alex Schultz Co-Authored-By: Thomas Herve --- tripleo_common/actions/deployment.py | 58 ++++++++ tripleo_common/actions/templates.py | 33 ++--- .../tests/actions/test_deployment.py | 134 +++++++++++++++++- .../tests/actions/test_templates.py | 15 +- 4 files changed, 205 insertions(+), 35 deletions(-) diff --git a/tripleo_common/actions/deployment.py b/tripleo_common/actions/deployment.py index 331c9b146..9b398880c 100644 --- a/tripleo_common/actions/deployment.py +++ b/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 diff --git a/tripleo_common/actions/templates.py b/tripleo_common/actions/templates.py index aa7176712..0a64eeea6 100644 --- a/tripleo_common/actions/templates.py +++ b/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: - if isinstance(searchpath, six.string_types): - self.searchpath = [searchpath] - else: - self.searchpath = list(searchpath) - else: - self.searchpath = [] + self.searchpath = [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, - 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') - + def _j2_render_and_put(self, j2_template, j2_data, yaml_f, swift): # 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)) diff --git a/tripleo_common/tests/actions/test_deployment.py b/tripleo_common/tests/actions/test_deployment.py index 4e6407423..e56c79ba6 100644 --- a/tripleo_common/tests/actions/test_deployment.py +++ b/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.' diff --git a/tripleo_common/tests/actions/test_templates.py b/tripleo_common/tests/actions/test_templates.py index 240e30c62..f7cc9f11a 100644 --- a/tripleo_common/tests/actions/test_templates.py +++ b/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]