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):