From 71ca8096fe3646754d51854ec04fc69fde7f9f9a Mon Sep 17 00:00:00 2001 From: Ana Krivokapic Date: Thu, 22 Dec 2016 14:14:02 +0100 Subject: [PATCH] Enhance plan creation and update with plan-environment This change enhances the plan creation and update workflows to consume the plan environment file and import its contents into the Mistral environment. It also removes importing the root template and root environment from the capabilities map file, as these fields will now be imported from the plan environment file. Implements: blueprint enhance-plan-creation-with-plan-environment Depends-On: I95e3e3a25104623d6fcf38e99403cebbd591b92d Change-Id: I961624723d127aebbaacd0c2b481211d83dde3f6 --- .../notes/5.8.0-d1ca2298ba598431.yaml | 5 + tripleo_common/actions/heat_capabilities.py | 2 - tripleo_common/actions/plan.py | 186 ++++++++----- tripleo_common/constants.py | 4 + tripleo_common/exception.py | 4 + .../tests/actions/test_heat_capabilities.py | 4 +- tripleo_common/tests/actions/test_plan.py | 250 +++++++++++------- 7 files changed, 290 insertions(+), 165 deletions(-) diff --git a/releasenotes/notes/5.8.0-d1ca2298ba598431.yaml b/releasenotes/notes/5.8.0-d1ca2298ba598431.yaml index 36359986a..e781c8b83 100644 --- a/releasenotes/notes/5.8.0-d1ca2298ba598431.yaml +++ b/releasenotes/notes/5.8.0-d1ca2298ba598431.yaml @@ -11,6 +11,11 @@ features: - Add an new Action which generates environment parameters for configuring fencing. - Add utility functions for deleting/emptying swift containers. + - Enhance the plan create and plan update workflows to support plan import. + A new plan environment file (located in t-h-t) is now used to store the + Mistral environment, so it can easily be imported and exported. Root + template and root environment settings (previously stored in the + capabilities map file) are now being stored in this file. fixes: - Fixes `bug 1644756 `__ so that flavour matching works as expected with the object-storage role. diff --git a/tripleo_common/actions/heat_capabilities.py b/tripleo_common/actions/heat_capabilities.py index 6b6a6aa3f..0d05beed6 100644 --- a/tripleo_common/actions/heat_capabilities.py +++ b/tripleo_common/actions/heat_capabilities.py @@ -90,8 +90,6 @@ class GetCapabilitiesAction(base.TripleOAction): # change capabilities format data_to_return = {} - capabilities.pop('root_environment') - capabilities.pop('root_template') for topic in capabilities['topics']: title = topic.get('title', '_title_holder') diff --git a/tripleo_common/actions/plan.py b/tripleo_common/actions/plan.py index 9e5edbed8..834c6385b 100644 --- a/tripleo_common/actions/plan.py +++ b/tripleo_common/actions/plan.py @@ -36,6 +36,58 @@ default_container_headers = { } +class PlanEnvMixin(object): + @staticmethod + def get_plan_env_dict(swift, container): + """Retrieves the plan environment from Swift. + + Loads a plan environment file with a given container name from Swift. + Makes sure that the file contains valid YAML and that the mandatory + fields are present in the environment. + + If the plan environment file is missing from Swift, fall back to the + capabilities-map.yaml. + + Returns the plan environment dictionary, and a boolean indicator + whether the plan environment file was missing from Swift. + """ + plan_env_missing = False + + try: + plan_env = swift.get_object(container, + constants.PLAN_ENVIRONMENT)[1] + except swiftexceptions.ClientException: + # If the plan environment file is missing from Swift, look for + # capabilities-map.yaml instead + plan_env_missing = True + try: + plan_env = swift.get_object(container, + 'capabilities-map.yaml')[1] + except swiftexceptions.ClientException as err: + raise exception.PlanOperationError( + "File missing from container: %s" % err) + + try: + plan_env_dict = yaml.safe_load(plan_env) + except yaml.YAMLError as err: + raise exception.PlanOperationError( + "Error parsing the yaml file: %s" % err) + + if plan_env_missing: + plan_env_dict = { + 'environments': [{'path': plan_env_dict['root_environment']}], + 'template': plan_env_dict['root_template'], + 'version': 1.0 + } + + for key in ('environments', 'template', 'version'): + if key not in plan_env_dict: + raise exception.PlanOperationError( + "%s missing key: %s" % (constants.PLAN_ENVIRONMENT, key)) + + return plan_env_dict, plan_env_missing + + class CreateContainerAction(base.TripleOAction): """Creates an object container @@ -65,12 +117,13 @@ class CreateContainerAction(base.TripleOAction): oc.put_container(self.container, headers=default_container_headers) -class CreatePlanAction(base.TripleOAction): +class CreatePlanAction(base.TripleOAction, PlanEnvMixin): """Creates a plan - Given a container, creates a Mistral environment with the same name, - parses the capabilities map file and sets initial plan template and - environment files. + Given a container, creates a Mistral environment with the same name. + The contents of the environment are imported from the plan environment + file, which must contain entries for `template`, `environments` and + `version` at a minimum. """ def __init__(self, container): @@ -78,12 +131,11 @@ class CreatePlanAction(base.TripleOAction): self.container = container def run(self): - oc = self.get_object_client() + swift = self.get_object_client() + mistral = self.get_workflow_client() env_data = { 'name': self.container, } - env_vars = {} - error_text = None if not pattern_validator(constants.PLAN_NAME_PATTERN, self.container): message = ("Unable to create plan. The plan name must " @@ -92,7 +144,7 @@ class CreatePlanAction(base.TripleOAction): # Check to see if an environment with that name already exists try: - self.get_workflow_client().environments.get(self.container) + mistral.environments.get(self.container) except mistralclient_base.APIException: # The environment doesn't exist, as expected. Proceed. pass @@ -101,41 +153,44 @@ class CreatePlanAction(base.TripleOAction): "already exists") return mistral_workflow_utils.Result(error=message) + # Get plan environment from Swift try: - # parses capabilities to get root_template, root_environment - mapfile = yaml.safe_load( - oc.get_object(self.container, 'capabilities-map.yaml')[1]) + plan_env_dict, plan_env_missing = self.get_plan_env_dict( + swift, self.container) + except exception.PlanOperationError as err: + return mistral_workflow_utils.Result(error=six.text_type(err)) - if mapfile['root_template']: - env_vars['template'] = mapfile['root_template'] - if mapfile['root_environment']: - env_vars['environments'] = [ - {'path': mapfile['root_environment']}] - - env_data['variables'] = json.dumps(env_vars, sort_keys=True,) - # creates environment - self.get_workflow_client().environments.create(**env_data) - except yaml.YAMLError as yaml_err: - error_text = "Error parsing the yaml file: %s" % yaml_err - except swiftexceptions.ClientException as obj_err: - error_text = "File missing from container: %s" % obj_err - except KeyError as key_err: - error_text = ("capabilities-map.yaml missing key: " - "%s" % key_err) + # Create mistral environment + env_data['variables'] = json.dumps(plan_env_dict, sort_keys=True) + try: + mistral.environments.create(**env_data) except Exception as err: - error_text = "Error occurred creating plan: %s" % err + message = "Error occurred creating plan: %s" % err + return mistral_workflow_utils.Result(error=message) - if error_text: - return mistral_workflow_utils.Result(error=error_text) + # Delete the plan environment file from Swift, as it is no long needed. + # (If we were to leave the environment file behind, we would have to + # take care to keep it in sync with the actual contents of the Mistral + # environment. To avoid that, we simply delete it.) + # TODO(akrivoka): Once the 'Deployment plan management changes' spec + # (https://review.openstack.org/#/c/438918/) is implemented, we will no + # longer use Mistral environments for holding the plan data, so this + # code can go away. + if not plan_env_missing: + try: + swift.delete_object(self.container, constants.PLAN_ENVIRONMENT) + except swiftexceptions.ClientException as err: + message = "Error deleting file from container: %s" % err + return mistral_workflow_utils.Result(error=message) -class UpdatePlanAction(base.TripleOAction): - """Update a plan +class UpdatePlanAction(base.TripleOAction, PlanEnvMixin): + """Updates a plan - Given a container, update the Mistral environment with the same name, - parses the capabilities map file and sets the initial plan template - and environment files if they have changed since the plan was originally - created. + Given a container, update the Mistral environment with the same name. + The contents of the environment are imported (overwritten) from the plan + environment file, which must contain entries for `template`, `environments` + and `version` at a minimum. """ def __init__(self, container): @@ -146,41 +201,36 @@ class UpdatePlanAction(base.TripleOAction): swift = self.get_object_client() mistral = self.get_workflow_client() - error_text = None - + # Get plan environment from Swift try: - mapobject = swift.get_object(self.container, - 'capabilities-map.yaml')[1] - mapfile = yaml.safe_load(mapobject) - - mistral_env = mistral.environments.get(self.container) - - # We always want the root template to match whatever is in the - # capabilities map, so update that regardless. - mistral_env.variables['root_template'] = mapfile['root_template'] - - # Check to see if the root environment is already listed, if it - # isn't - add it. - # TODO(d0ugal): Users could get into a situation where they have - # an old root environment still added and they - # then have two after the new one is added. - root_env = {'path': mapfile['root_environment']} - if root_env not in mistral_env.variables['environments']: - mistral_env.variables['environments'].insert(0, root_env) + plan_env_dict, plan_env_missing = self.get_plan_env_dict( + swift, self.container) + except exception.PlanOperationError as err: + return mistral_workflow_utils.Result(error=six.text_type(err)) + # Update mistral environment with contents from plan environment file + variables = json.dumps(plan_env_dict, sort_keys=True) + try: mistral.environments.update( - name=self.container, - variables=mistral_env.variables, - ) - except yaml.YAMLError as yaml_err: - error_text = "Error parsing the yaml file: %s" % yaml_err - except swiftexceptions.ClientException as obj_err: - error_text = "File missing from container: %s" % obj_err - except KeyError as key_err: - error_text = ("capabilities-map.yaml missing key: " - "%s" % key_err) - if error_text: - return mistral_workflow_utils.Result(error=error_text) + name=self.container, variables=variables) + except mistralclient_base.APIException: + message = "Error updating mistral environment: %s" % self.container + return mistral_workflow_utils.Result(error=message) + + # Delete the plan environment file from Swift, as it is no long needed. + # (If we were to leave the environment file behind, we would have to + # take care to keep it in sync with the actual contents of the Mistral + # environment. To avoid that, we simply delete it.) + # TODO(akrivoka): Once the 'Deployment plan management changes' spec + # (https://review.openstack.org/#/c/438918/) is implemented, we will no + # longer use Mistral environments for holding the plan data, so this + # code can go away. + if not plan_env_missing: + try: + swift.delete_object(self.container, constants.PLAN_ENVIRONMENT) + except swiftexceptions.ClientException as err: + message = "Error deleting file from container: %s" % err + return mistral_workflow_utils.Result(error=message) class ListPlansAction(base.TripleOAction): diff --git a/tripleo_common/constants.py b/tripleo_common/constants.py index a3fe75f3b..f6e447bb4 100644 --- a/tripleo_common/constants.py +++ b/tripleo_common/constants.py @@ -106,3 +106,7 @@ PLAN_NAME_PATTERN = '^[a-zA-Z0-9-]+$' # The default version of the Bare metal API to set in overcloudrc. # 1.29 is the latest API version in Ironic Ocata supported by ironicclient. DEFAULT_BAREMETAL_API_VERSION = '1.29' + +# The name of the file which holds the Mistral environment contents for plan +# import/export +PLAN_ENVIRONMENT = 'plan-environment.yaml' diff --git a/tripleo_common/exception.py b/tripleo_common/exception.py index e4a69e615..e12a6438f 100644 --- a/tripleo_common/exception.py +++ b/tripleo_common/exception.py @@ -103,3 +103,7 @@ class StateTransitionFailed(Exception): class RootDeviceDetectionError(Exception): """Failed to detect the root device""" + + +class PlanOperationError(Exception): + """Error while performing a deployment plan operation""" diff --git a/tripleo_common/tests/actions/test_heat_capabilities.py b/tripleo_common/tests/actions/test_heat_capabilities.py index 77071dbfc..aa7d26c58 100644 --- a/tripleo_common/tests/actions/test_heat_capabilities.py +++ b/tripleo_common/tests/actions/test_heat_capabilities.py @@ -21,9 +21,7 @@ from tripleo_common.actions import heat_capabilities from tripleo_common.tests import base -MAPPING_YAML_CONTENTS = """root_template: /path/to/overcloud.yaml -root_environment: /path/to/environment.yaml -topics: +MAPPING_YAML_CONTENTS = """topics: - title: Fake Single Environment Group Configuration description: environment_groups: diff --git a/tripleo_common/tests/actions/test_plan.py b/tripleo_common/tests/actions/test_plan.py index c759d1076..160db6b49 100644 --- a/tripleo_common/tests/actions/test_plan.py +++ b/tripleo_common/tests/actions/test_plan.py @@ -12,6 +12,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import json import mock from heatclient import exc as heatexceptions @@ -20,52 +21,49 @@ from mistralclient.api import base as mistral_base from swiftclient import exceptions as swiftexceptions from tripleo_common.actions import plan +from tripleo_common import constants from tripleo_common import exception from tripleo_common.tests import base -MAPPING_YAML_CONTENTS = """root_template: /path/to/overcloud.yaml -root_environment: /path/to/environment.yaml -topics: - - title: Fake Single Environment Group Configuration - description: - environment_groups: - - title: - description: Random fake string of text - environments: - - file: /path/to/network-isolation.json - title: Default Configuration - description: +JSON_CONTENTS = json.dumps({ + "environments": [{ + "path": "overcloud-resource-registry-puppet.yaml" + }, { + "path": "environments/services/sahara.yaml" + }], + "parameter_defaults": { + "BlockStorageCount": 42, + "OvercloudControlFlavor": "yummy" + }, + "passwords": { + "AdminPassword": "aaaa", + "ZaqarPassword": "zzzz" + }, + "template": "overcloud.yaml", + "version": 1.0 +}, sort_keys=True) - - title: Fake Multiple Environment Group Configuration - description: - environment_groups: - - title: Random Fake 1 - description: Random fake string of text - environments: - - file: /path/to/ceph-storage-env.yaml - title: Fake1 - description: Random fake string of text - - title: Random Fake 2 - description: - environments: - - file: /path/to/poc-custom-env.yaml - title: Fake2 - description: +YAML_CONTENTS = """ +version: 1.0 + +template: overcloud.yaml +environments: +- path: overcloud-resource-registry-puppet.yaml +- path: environments/services/sahara.yaml +parameter_defaults: + BlockStorageCount: 42 + OvercloudControlFlavor: yummy +passwords: + AdminPassword: aaaa + ZaqarPassword: zzzz """ -INVALID_MAPPING_CONTENTS = """ -root_environment: /path/to/environment.yaml -topics: - - title: Fake Single Environment Group Configuration - description: - environment_groups: - - title: - description: Random fake string of text - environments: - - file: /path/to/network-isolation.json - title: Default Configuration - description: +YAML_CONTENTS_INVALID = "{bad_yaml" + +# `environments` is missing +YAML_CONTENTS_MISSING_KEY = """ +template: overcloud.yaml """ RESOURCES_YAML_CONTENTS = """heat_template_version: 2016-04-08 @@ -148,11 +146,11 @@ class CreatePlanActionTest(base.TestCase): super(CreatePlanActionTest, self).setUp() # A container that name enforces all validation rules self.container_name = 'Test-container-3' - self.capabilities_name = 'capabilities-map.yaml' + self.plan_environment_name = constants.PLAN_ENVIRONMENT # setup swift self.swift = mock.MagicMock() - self.swift.get_object.return_value = ({}, MAPPING_YAML_CONTENTS) + self.swift.get_object.return_value = ({}, YAML_CONTENTS) swift_patcher = mock.patch( 'tripleo_common.actions.base.TripleOAction.get_object_client', return_value=self.swift) @@ -168,47 +166,23 @@ class CreatePlanActionTest(base.TestCase): mistral_patcher.start() self.addCleanup(mistral_patcher.stop) - def test_run(self): - + def test_run_success(self): action = plan.CreatePlanAction(self.container_name) action.run() self.swift.get_object.assert_called_once_with( self.container_name, - self.capabilities_name + self.plan_environment_name ) + self.swift.delete_object.assert_called_once() + self.mistral.environments.create.assert_called_once_with( name='Test-container-3', - variables=('{"environments":' - ' [{"path": "/path/to/environment.yaml"}], ' - '"template": "/path/to/overcloud.yaml"}') + variables=JSON_CONTENTS ) - def test_run_with_invalid_yaml(self): - - self.swift.get_object.return_value = ({}, 'invalid: %') - - action = plan.CreatePlanAction(self.container_name) - result = action.run() - - error_str = 'Error parsing the yaml file' - # don't bother checking the exact yaml error (it's long) - self.assertEqual(result.error.split(':')[0], error_str) - - def test_run_with_invalid_string(self): - - self.swift.get_object.return_value = ({}, 'this is just a string') - - action = plan.CreatePlanAction(self.container_name) - result = action.run() - - error_str = 'Error occurred creating plan' - # don't bother checking the exact error (python versions different) - self.assertEqual(result.error.split(':')[0], error_str) - - def test_run_with_invalid_plan_name(self): - + def test_run_invalid_plan_name(self): action = plan.CreatePlanAction("invalid_underscore") result = action.run() @@ -217,27 +191,6 @@ class CreatePlanActionTest(base.TestCase): # don't bother checking the exact error (python versions different) self.assertEqual(result.error.split(':')[0], error_str) - def test_run_with_no_file(self): - - self.swift.get_object.side_effect = swiftexceptions.ClientException( - 'atest2') - - action = plan.CreatePlanAction(self.container_name) - result = action.run() - - error_str = 'File missing from container: atest2' - self.assertEqual(result.error, error_str) - - def test_run_with_missing_key(self): - - self.swift.get_object.return_value = ({}, INVALID_MAPPING_CONTENTS) - - action = plan.CreatePlanAction(self.container_name) - result = action.run() - - error_str = "capabilities-map.yaml missing key: 'root_template'" - self.assertEqual(result.error, error_str) - def test_run_mistral_env_already_exists(self): self.mistral.environments.get.side_effect = None self.mistral.environments.get.return_value = 'test-env' @@ -250,6 +203,119 @@ class CreatePlanActionTest(base.TestCase): self.assertEqual(result.error, error_str) self.mistral.environments.create.assert_not_called() + def test_run_missing_file(self): + self.swift.get_object.side_effect = swiftexceptions.ClientException( + self.plan_environment_name) + + action = plan.CreatePlanAction(self.container_name) + result = action.run() + + error_str = ('File missing from container: %s' % + self.plan_environment_name) + self.assertEqual(result.error, error_str) + + def test_run_invalid_yaml(self): + self.swift.get_object.return_value = ({}, YAML_CONTENTS_INVALID) + + action = plan.CreatePlanAction(self.container_name) + result = action.run() + + error_str = 'Error parsing the yaml file' + self.assertEqual(result.error.split(':')[0], error_str) + + def test_run_missing_key(self): + self.swift.get_object.return_value = ({}, YAML_CONTENTS_MISSING_KEY) + + action = plan.CreatePlanAction(self.container_name) + result = action.run() + + error_str = ("%s missing key: environments" % + self.plan_environment_name) + self.assertEqual(result.error, error_str) + + +class UpdatePlanActionTest(base.TestCase): + + def setUp(self): + super(UpdatePlanActionTest, self).setUp() + self.container_name = 'Test-container-3' + self.plan_environment_name = constants.PLAN_ENVIRONMENT + + # setup swift + self.swift = mock.MagicMock() + self.swift.get_object.return_value = ({}, YAML_CONTENTS) + swift_patcher = mock.patch( + 'tripleo_common.actions.base.TripleOAction.get_object_client', + return_value=self.swift) + swift_patcher.start() + self.addCleanup(swift_patcher.stop) + + # setup mistral + self.mistral = mock.MagicMock() + mistral_patcher = mock.patch( + 'tripleo_common.actions.base.TripleOAction.get_workflow_client', + return_value=self.mistral) + mistral_patcher.start() + self.addCleanup(mistral_patcher.stop) + + def test_run_success(self): + action = plan.UpdatePlanAction(self.container_name) + action.run() + + self.swift.get_object.assert_called_once_with( + self.container_name, + self.plan_environment_name + ) + + self.swift.delete_object.assert_called_once() + + self.mistral.environments.update.assert_called_once_with( + name='Test-container-3', + variables=JSON_CONTENTS + ) + + def test_run_mistral_env_missing(self): + self.mistral.environments.update.side_effect = ( + mistral_base.APIException) + + action = plan.UpdatePlanAction(self.container_name) + result = action.run() + + error_str = ("Error updating mistral environment: %s" % + self.container_name) + self.assertEqual(result.error, error_str) + self.swift.delete_object.assert_not_called() + + def test_run_missing_file(self): + self.swift.get_object.side_effect = swiftexceptions.ClientException( + self.plan_environment_name) + + action = plan.UpdatePlanAction(self.container_name) + result = action.run() + + error_str = ('File missing from container: %s' % + self.plan_environment_name) + self.assertEqual(result.error, error_str) + + def test_run_invalid_yaml(self): + self.swift.get_object.return_value = ({}, YAML_CONTENTS_INVALID) + + action = plan.UpdatePlanAction(self.container_name) + result = action.run() + + error_str = 'Error parsing the yaml file' + self.assertEqual(result.error.split(':')[0], error_str) + + def test_run_missing_key(self): + self.swift.get_object.return_value = ({}, YAML_CONTENTS_MISSING_KEY) + + action = plan.UpdatePlanAction(self.container_name) + result = action.run() + + error_str = ("%s missing key: environments" % + self.plan_environment_name) + self.assertEqual(result.error, error_str) + class ListPlansActionTest(base.TestCase):