From 5b98699b1845133d338a6df06622c61821b4da31 Mon Sep 17 00:00:00 2001 From: Jiri Stransky Date: Thu, 5 Apr 2018 15:12:52 +0200 Subject: [PATCH] Remove no-ops from user-env too Only removing them from plan-env could get the no-ops re-inserted by accident from user-env on next plan update, if the next plan-update action would be done with user-env persistence (any of update/upgrade operations). Change-Id: Ib9cfff3e38cb05ffe03b6ba06498d0655119b97d Closes-Bug: #1761499 --- tripleo_common/actions/plan.py | 27 ++++++++------ tripleo_common/constants.py | 3 ++ tripleo_common/tests/actions/test_plan.py | 44 +++++++++++++++++++++++ tripleo_common/tests/utils/test_plan.py | 25 +++++++++++-- tripleo_common/utils/plan.py | 15 ++++++++ 5 files changed, 102 insertions(+), 12 deletions(-) diff --git a/tripleo_common/actions/plan.py b/tripleo_common/actions/plan.py index 8fd85e1a4..ad4eb69e7 100644 --- a/tripleo_common/actions/plan.py +++ b/tripleo_common/actions/plan.py @@ -486,19 +486,26 @@ class RemoveNoopDeployStepAction(base.TripleOAction): return actions.Result(error=msg) swift = self.get_object_client(context) - plan_env = plan_utils.get_env(swift, self.container) # Get output and check if DeployStep are None - steps = ['OS::TripleO::DeploymentSteps'] + removals = ['OS::TripleO::DeploymentSteps'] for output in stack.to_dict().get('outputs', {}): if output['output_key'] == 'RoleData': for role in output['output_value']: - steps.append("OS::TripleO::Tasks::%sPreConfig" % role) - steps.append("OS::TripleO::Tasks::%sPostConfig" % role) - # Remove noop Steps - for step in steps: - if step in plan_env.get('resource_registry', {}): - if plan_env['resource_registry'][step] == 'OS::Heat::None': - plan_env['resource_registry'].pop(step) - # Push plan_env + removals.append("OS::TripleO::Tasks::%sPreConfig" % role) + removals.append("OS::TripleO::Tasks::%sPostConfig" % role) + + plan_env = plan_utils.get_env(swift, self.container) + self.remove_noops_from_env(removals, plan_env) plan_utils.put_env(swift, plan_env) + + user_env = plan_utils.get_user_env(swift, self.container) + self.remove_noops_from_env(removals, user_env) + plan_utils.put_user_env(swift, self.container, user_env) + + def remove_noops_from_env(self, removals, env): + # Remove noop Steps + for rm in removals: + if rm in env.get('resource_registry', {}): + if env['resource_registry'][rm] == 'OS::Heat::None': + env['resource_registry'].pop(rm) diff --git a/tripleo_common/constants.py b/tripleo_common/constants.py index 8cc7d1157..a18431559 100644 --- a/tripleo_common/constants.py +++ b/tripleo_common/constants.py @@ -143,6 +143,9 @@ DEFAULT_VOLUME_API_VERSION = '3' # import/export PLAN_ENVIRONMENT = 'plan-environment.yaml' +# Name of the environment with merged parameters from CLI +USER_ENVIRONMENT = 'user-environment.yaml' + # The name of the file which holds container image default parameters CONTAINER_DEFAULTS_ENVIRONMENT = ('environments/' 'containers-default-parameters.yaml') diff --git a/tripleo_common/tests/actions/test_plan.py b/tripleo_common/tests/actions/test_plan.py index 8ecfffb9e..8ced19ba7 100644 --- a/tripleo_common/tests/actions/test_plan.py +++ b/tripleo_common/tests/actions/test_plan.py @@ -587,3 +587,47 @@ class GatherRolesActionTest(base.TestCase): # assert that a role was loaded from self.current_roles self.assertTrue(result.data['gathered_roles'], [SAMPLE_ROLE_OBJ, self.available_role]) + + +class RemoveNoopDeployStepActionTest(base.TestCase): + + def setUp(self): + super(RemoveNoopDeployStepActionTest, self).setUp() + self.container = 'overcloud' + self.ctx = mock.MagicMock() + self.heat = mock.MagicMock() + self.swift = mock.MagicMock() + self.rr_before = { + 'OS::TripleO::Foo': 'bar.yaml', + 'OS::TripleO::DeploymentSteps': 'OS::Heat::None', + } + self.rr_after = { + 'OS::TripleO::Foo': 'bar.yaml', + } + + @mock.patch('tripleo_common.actions.plan.plan_utils') + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') + @mock.patch( + 'tripleo_common.actions.base.TripleOAction.get_orchestration_client') + def test_roles_gathered(self, mock_orch_client, mock_obj_client, + mock_plan_utils): + mock_obj_client.return_value = self.swift + mock_orch_client.return_value = self.heat + mock_plan_utils.get_env.return_value = { + 'name': self.container, + 'resource_registry': self.rr_before, + } + mock_plan_utils.get_user_env.return_value = { + 'resource_registry': self.rr_before, + } + action = plan.RemoveNoopDeployStepAction(self.container) + action.run(self.ctx) + + mock_plan_utils.put_env.assert_called_with( + self.swift, + { + 'name': self.container, + 'resource_registry': self.rr_after, + }) + mock_plan_utils.put_user_env.assert_called_with( + self.swift, self.container, {'resource_registry': self.rr_after}) diff --git a/tripleo_common/tests/utils/test_plan.py b/tripleo_common/tests/utils/test_plan.py index 62c3e36fe..103ea78dc 100644 --- a/tripleo_common/tests/utils/test_plan.py +++ b/tripleo_common/tests/utils/test_plan.py @@ -21,7 +21,7 @@ from tripleo_common.tests import base from tripleo_common.utils import plan as plan_utils -YAML_CONTENTS = """ +PLAN_ENV_CONTENTS = """ version: 1.0 name: overcloud @@ -37,13 +37,18 @@ passwords: ZaqarPassword: zzzz """ +USER_ENV_CONTENTS = """ +resource_registry: + OS::TripleO::Foo: bar.yaml +""" + class PlanTest(base.TestCase): def setUp(self): super(PlanTest, self).setUp() self.container = 'overcloud' self.swift = mock.MagicMock() - self.swift.get_object.return_value = ({}, YAML_CONTENTS) + self.swift.get_object.return_value = ({}, PLAN_ENV_CONTENTS) def test_get_env(self): env = plan_utils.get_env(self.swift, self.container) @@ -57,6 +62,22 @@ class PlanTest(base.TestCase): self. assertRaises(Exception, plan_utils.get_env, self.swift, self.container) + def test_get_user_env(self): + self.swift.get_object.return_value = ({}, USER_ENV_CONTENTS) + env = plan_utils.get_user_env(self.swift, self.container) + + self.swift.get_object.assert_called_with( + self.container, 'user-environment.yaml') + self.assertEqual( + env['resource_registry']['OS::TripleO::Foo'], 'bar.yaml') + + def test_put_user_env(self): + contents = {'a': 'b'} + plan_utils.put_user_env(self.swift, self.container, contents) + + self.swift.put_object.assert_called_with( + self.container, 'user-environment.yaml', 'a: b\n') + def test_update_in_env(self): env = plan_utils.get_env(self.swift, self.container) diff --git a/tripleo_common/utils/plan.py b/tripleo_common/utils/plan.py index 460ac0c5c..1c53b14bc 100644 --- a/tripleo_common/utils/plan.py +++ b/tripleo_common/utils/plan.py @@ -56,3 +56,18 @@ def put_env(swift, env): constants.PLAN_ENVIRONMENT, yaml.safe_dump(env, default_flow_style=False) ) + + +def get_user_env(swift, container_name): + """Get user environment from Swift convert it to a dictionary.""" + return yaml.safe_load( + swift.get_object(container_name, constants.USER_ENVIRONMENT)[1]) + + +def put_user_env(swift, container_name, env): + """Convert given user environment to yaml and upload it to Swift.""" + swift.put_object( + container_name, + constants.USER_ENVIRONMENT, + yaml.safe_dump(env, default_flow_style=False) + )