From 79edc6a65340f291f9d96f77d843d1d6baf5770e Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Tue, 25 Feb 2020 14:46:55 +0530 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 | 3 +- tripleo_common/tests/utils/test_template.py | 55 +++++++++- tripleo_common/utils/template.py | 114 +++++++++++++++----- 3 files changed, 140 insertions(+), 32 deletions(-) diff --git a/tripleo_common/actions/deployment.py b/tripleo_common/actions/deployment.py index 11e3818a7..d8b0fd5fe 100644 --- a/tripleo_common/actions/deployment.py +++ b/tripleo_common/actions/deployment.py @@ -193,7 +193,8 @@ class DeployStackAction(base.TripleOAction): # process all plan files and create or update a stack processed_data = template_utils.process_templates( - swift, heat, container=self.container + swift, heat, container=self.container, + prune_services=True ) stack_args = processed_data.copy() stack_args['timeout_mins'] = self.timeout_mins diff --git a/tripleo_common/tests/utils/test_template.py b/tripleo_common/tests/utils/test_template.py index b641d305a..653f20fd1 100644 --- a/tripleo_common/tests/utils/test_template.py +++ b/tripleo_common/tests/utils/test_template.py @@ -113,7 +113,7 @@ class J2SwiftLoaderTest(base.TestCase): return swift def test_include_absolute_path(self): - j2_loader = template_utils.J2SwiftLoader(self._setup_swift(), None) + j2_loader = template_utils.J2SwiftLoader(self._setup_swift(), None, '') template = jinja2.Environment(loader=j2_loader).from_string( r''' Included this: @@ -142,7 +142,7 @@ class J2SwiftLoaderTest(base.TestCase): ''') def test_include_not_found(self): - j2_loader = template_utils.J2SwiftLoader(self._setup_swift(), None) + j2_loader = template_utils.J2SwiftLoader(self._setup_swift(), None, '') template = jinja2.Environment(loader=j2_loader).from_string( r''' Included this: @@ -153,7 +153,8 @@ class J2SwiftLoaderTest(base.TestCase): template.render) def test_include_invalid_path(self): - j2_loader = template_utils.J2SwiftLoader(self._setup_swift(), 'bar') + j2_loader = template_utils.J2SwiftLoader(self._setup_swift(), + 'bar', '') template = jinja2.Environment(loader=j2_loader).from_string( r''' Included this: @@ -354,7 +355,7 @@ class ProcessTemplatesTest(base.TestCase): swift, r"{% include 'bar/foo.yaml' %}", {'role': 'CustomRole'}, - 'bar/customrole-config.yaml', + 'customrole-config.yaml', constants.DEFAULT_CONTAINER_NAME) result = swift.put_object._mock_mock_calls[0] @@ -458,3 +459,49 @@ class ProcessTemplatesTest(base.TestCase): } assert j2_mock.called_with(expected_j2_template, expected_j2_data, 'foo.yaml') + + 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 + 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'] + }] + + template_utils.prune_unused_services(swift, test_role_data, + resource_registry, 'overcloud') + + 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 + test_role_data = [{ + 'name': 'Controller', + 'ServicesDefault': [ + 'OS::TripleO::Services::Foo', + 'OS::TripleO::Services::Baz'] + }] + + template_utils.prune_unused_services(swift, test_role_data, + resource_registry, 'overcloud') + mock_put.assert_not_called() diff --git a/tripleo_common/utils/template.py b/tripleo_common/utils/template.py index dc7b7e3c2..5a29c34c4 100644 --- a/tripleo_common/utils/template.py +++ b/tripleo_common/utils/template.py @@ -37,16 +37,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('') @@ -65,11 +59,8 @@ class J2SwiftLoader(jinja2.BaseLoader): raise jinja2.exceptions.TemplateNotFound(template) -def j2_render_and_put(swift, j2_template, j2_data, - container=constants.DEFAULT_CONTAINER_NAME, - outfile_name=None): - - yaml_f = outfile_name or j2_template.replace('.j2.yaml', '.yaml') +def j2_render_and_put(swift, j2_template, j2_data, yaml_f, + container=constants.DEFAULT_CONTAINER_NAME): # Search for templates relative to the current template path first template_base = os.path.dirname(yaml_f) @@ -242,16 +233,16 @@ def process_custom_roles(swift, heat, j2_data = {'role': r_map[role], 'networks': network_data} j2_render_and_put(swift, j2_template, - j2_data, container, - out_f_path) + j2_data, out_f_path, + container) else: # Backwards compatibility with templates # that specify {{role}} vs {{role.name}} j2_data = {'role': role, 'networks': network_data} LOG.debug("role legacy path for role %s" % role) j2_render_and_put(swift, j2_template, - j2_data, container, - out_f_path) + j2_data, out_f_path, + container) else: LOG.info("Skipping rendering of %s, defined in %s" % (out_f_path, j2_excl_data)) @@ -276,8 +267,8 @@ def process_custom_roles(swift, heat, out_f_path = os.path.join(os.path.dirname(f), out_f) if not (out_f_path in excl_templates): j2_render_and_put(swift, j2_template, - j2_data, container, - out_f_path) + j2_data, out_f_path, + container) else: LOG.info("Skipping rendering of %s, defined in %s" % (out_f_path, j2_excl_data)) @@ -290,11 +281,54 @@ def process_custom_roles(swift, heat, j2_data = {'roles': role_data, 'networks': network_data} out_f = f.replace('.j2.yaml', '.yaml') j2_render_and_put(swift, j2_template, - j2_data, container, - out_f) + j2_data, out_f, + container) + return role_data -def process_templates(swift, heat, container=constants.DEFAULT_CONTAINER_NAME): +def prune_unused_services(swift, role_data, + resource_registry, + container=constants.DEFAULT_CONTAINER_NAME): + """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 OS::Heat::None + resources. + + :param resource_registry: tripleo resource registry dict + :param swift: swift client + :param resource_registry: tripleo resource registry dict + :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 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 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(container, + constants.OVERCLOUD_J2_ROLES_NAME, + yaml.safe_dump(role_data, + default_flow_style=False)) + return True + + +def build_heat_args(swift, heat, container=constants.DEFAULT_CONTAINER_NAME): error_text = None try: plan_env = plan_utils.get_env(swift, container) @@ -310,7 +344,7 @@ def process_templates(swift, heat, container=constants.DEFAULT_CONTAINER_NAME): # method called below should handle the case where the files are # not found in swift, but if they are found and an exception # occurs during processing, then it will be raised. - process_custom_roles(swift, heat, container) + role_data = process_custom_roles(swift, heat, container) except Exception as err: LOG.exception("Error occurred while processing custom roles.") raise RuntimeError(six.text_type(err)) @@ -334,6 +368,7 @@ def process_templates(swift, heat, container=constants.DEFAULT_CONTAINER_NAME): env_files, env = plan_utils.process_environments_and_files( swift, env_paths) parameters.convert_docker_params(env) + except Exception as err: error_text = six.text_type(err) LOG.exception("Error occurred while processing plan files.") @@ -341,15 +376,40 @@ def process_templates(swift, heat, container=constants.DEFAULT_CONTAINER_NAME): # cleanup any local temp files for f in temp_env_paths: os.remove(f) - if error_text: raise RuntimeError(six.text_type(error_text)) - files = dict(list(template_files.items()) + list(env_files.items())) + heat_args = { + 'template': template, + 'template_files': template_files, + 'env': env, + 'env_files': env_files + } + return heat_args, role_data + + +def process_templates(swift, heat, container=constants.DEFAULT_CONTAINER_NAME, + prune_services=False): + heat_args, role_data = build_heat_args(swift, heat, container) + if prune_services: + try: + # Prune OS::Heat::None resources + resource_reg = heat_args['env'].get('resource_registry', {}) + roles_updated = prune_unused_services( + swift, role_data, resource_reg) + if roles_updated: + heat_args, _ = build_heat_args(swift, heat, container) + + except Exception as err: + LOG.exception("Error occurred while prunning prune_services.") + raise RuntimeError(six.text_type(err)) + + files = dict(list(heat_args['template_files'].items()) + list( + heat_args['env_files'].items())) return { 'stack_name': container, - 'template': template, - 'environment': env, + 'template': heat_args['template'], + 'environment': heat_args['env'], 'files': files }