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 <aschultz@redhat.com> Co-Authored-By: Thomas Herve <therve@redhat.com>
This commit is contained in:
parent
f6010681f7
commit
90c79df86a
|
@ -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
|
||||
|
|
|
@ -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))
|
||||
|
|
|
@ -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.'
|
||||
|
|
|
@ -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]
|
||||
|
||||
|
|
Loading…
Reference in New Issue