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

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 <https://bugs.launchpad.net/tripleo/+bug/1644756>`__ so
     that flavour matching works as expected with the object-storage role.

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

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,
+    Given a container, creates a Mistral environment with the same name.
-    parses the capabilities map file and sets initial plan template and
+    The contents of the environment are imported from the plan environment
-    environment files.
+    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
+            plan_env_dict, plan_env_missing = self.get_plan_env_dict(
-            mapfile = yaml.safe_load(
+                swift, self.container)
-                oc.get_object(self.container, 'capabilities-map.yaml')[1])
+        except exception.PlanOperationError as err:
+            return mistral_workflow_utils.Result(error=six.text_type(err))
 
-            if mapfile['root_template']:
+        # Create mistral environment
-                env_vars['template'] = mapfile['root_template']
+        env_data['variables'] = json.dumps(plan_env_dict, sort_keys=True)
-            if mapfile['root_environment']:
+        try:
-                env_vars['environments'] = [
+            mistral.environments.create(**env_data)
-                    {'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)
         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:
+        # Delete the plan environment file from Swift, as it is no long needed.
-            return mistral_workflow_utils.Result(error=error_text)
+        # (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):
+class UpdatePlanAction(base.TripleOAction, PlanEnvMixin):
-    """Update a plan
+    """Updates a plan
 
-    Given a container, update the Mistral environment with the same name,
+    Given a container, update the Mistral environment with the same name.
-    parses the capabilities map file and sets the initial plan template
+    The contents of the environment are imported (overwritten) from the plan
-    and environment files if they have changed since the plan was originally
+    environment file, which must contain entries for `template`, `environments`
-    created.
+     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,
+            plan_env_dict, plan_env_missing = self.get_plan_env_dict(
-                                         'capabilities-map.yaml')[1]
+                swift, self.container)
-            mapfile = yaml.safe_load(mapobject)
+        except exception.PlanOperationError as err:
-
+            return mistral_workflow_utils.Result(error=six.text_type(err))
-            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)
 
+        # 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,
+                name=self.container, variables=variables)
-                variables=mistral_env.variables,
+        except mistralclient_base.APIException:
-            )
+            message = "Error updating mistral environment: %s" % self.container
-        except yaml.YAMLError as yaml_err:
+            return mistral_workflow_utils.Result(error=message)
-            error_text = "Error parsing the yaml file: %s" % yaml_err
+
-        except swiftexceptions.ClientException as obj_err:
+        # Delete the plan environment file from Swift, as it is no long needed.
-            error_text = "File missing from container: %s" % obj_err
+        # (If we were to leave the environment file behind, we would have to
-        except KeyError as key_err:
+        # take care to keep it in sync with the actual contents of the Mistral
-            error_text = ("capabilities-map.yaml missing key: "
+        # environment. To avoid that, we simply delete it.)
-                          "%s" % key_err)
+        # TODO(akrivoka): Once the 'Deployment plan management changes' spec
-        if error_text:
+        # (https://review.openstack.org/#/c/438918/) is implemented, we will no
-            return mistral_workflow_utils.Result(error=error_text)
+        # 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):

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'

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"""

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
+MAPPING_YAML_CONTENTS = """topics:
-root_environment: /path/to/environment.yaml
-topics:
   - title: Fake Single Environment Group Configuration
     description:
     environment_groups:

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
+JSON_CONTENTS = json.dumps({
-root_environment: /path/to/environment.yaml
+    "environments": [{
-topics:
+        "path": "overcloud-resource-registry-puppet.yaml"
-  - title: Fake Single Environment Group Configuration
+    }, {
-    description:
+        "path": "environments/services/sahara.yaml"
-    environment_groups:
+    }],
-      - title:
+    "parameter_defaults": {
-        description: Random fake string of text
+        "BlockStorageCount": 42,
-        environments:
+        "OvercloudControlFlavor": "yummy"
-          - file: /path/to/network-isolation.json
+    },
-            title: Default Configuration
+    "passwords": {
-            description:
+        "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
+YAML_CONTENTS = """
-        description:
+version: 1.0
-        environments:
+
-          - file: /path/to/poc-custom-env.yaml
+template: overcloud.yaml
-            title: Fake2
+environments:
-            description:
+-  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 = """
+YAML_CONTENTS_INVALID = "{bad_yaml"
-root_environment: /path/to/environment.yaml
+
-topics:
+# `environments` is missing
-  - title: Fake Single Environment Group Configuration
+YAML_CONTENTS_MISSING_KEY = """
-    description:
+template: overcloud.yaml
-    environment_groups:
-      - title:
-        description: Random fake string of text
-        environments:
-          - file: /path/to/network-isolation.json
-            title: Default Configuration
-            description:
 """
 
 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":'
+            variables=JSON_CONTENTS
-                       ' [{"path": "/path/to/environment.yaml"}], '
-                       '"template": "/path/to/overcloud.yaml"}')
         )
 
-    def test_run_with_invalid_yaml(self):
+    def test_run_invalid_plan_name(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):
-
         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):