Merge "Fix for get manifest"

armada/exceptions/__init__.py

@@ -12,6 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from armada.exceptions.manifest_exceptions import BuildChartException
+from armada.exceptions.manifest_exceptions import BuildChartGroupException
+from armada.exceptions.manifest_exceptions import ChartDependencyException
 from armada.exceptions.manifest_exceptions import ManifestException
 
-__all__ = ['ManifestException']
+__all__ = [
+    'BuildChartException', 'BuildChartGroupException',
+    'ChartDependencyException', 'ManifestException'
+]

armada/exceptions/manifest_exceptions.py

@@ -25,3 +25,40 @@ class ManifestException(base.ArmadaBaseException):
     """
 
     message = 'An error occurred while generating the manifest: %(details)s.'
+
+
+class BuildChartException(ManifestException):
+    """
+    An exception occurred while attempting to build the chart for an
+    Armada manifest.  The exception will return with details as to why.
+
+    **Troubleshoot:**
+    *Coming Soon*
+    """
+
+    message = 'An error occurred while trying to build chart: %(details)s.'
+
+
+class BuildChartGroupException(ManifestException):
+    """
+    An exception occurred while attempting to build the chart group for an
+    Armada manifest.  The exception will return with details as to why.
+
+    **Troubleshoot:**
+    *Coming Soon*
+    """
+
+    message = 'An error occurred while building chart group: %(details)s.'
+
+
+class ChartDependencyException(ManifestException):
+    """
+    An exception occurred while attempting to build a chart dependency for an
+    Armada manifest.  The exception will return with details as to why.
+
+    **Troubleshoot:**
+    *Coming Soon*
+    """
+
+    message = 'An error occurred while building a dependency chart: ' \
+              '%(details)s.'

armada/handlers/manifest.py

@@ -112,8 +112,8 @@ class Manifest(object):
         for chart in self.charts:
             if chart.get('metadata', {}).get('name') == name:
                 return chart
-        raise exceptions.ManifestException(
+        raise exceptions.BuildChartException(
-            details='Could not find a {} named "{}"'.format(
+            details='Could not build {} named "{}"'.format(
                 const.DOCUMENT_CHART, name))
 
     def find_chart_group_document(self, name):
@@ -128,26 +128,10 @@ class Manifest(object):
         for group in self.groups:
             if group.get('metadata', {}).get('name') == name:
                 return group
-        raise exceptions.ManifestException(
+        raise exceptions.BuildChartGroupException(
-            details='Could not find a {} named "{}"'.format(
+            details='Could not build {} named "{}"'.format(
                 const.DOCUMENT_GROUP, name))
 
-    def build_charts_deps(self):
-        """Build chart dependencies for every ``chart``.
-
-        :returns: None
-        """
-        for chart in self.charts:
-            self.build_chart_deps(chart)
-
-    def build_chart_groups(self):
-        """Build chart dependencies for every ``chart_group``.
-
-        :returns: None
-        """
-        for chart_group in self.groups:
-            self.build_chart_group(chart_group)
-
     def build_chart_deps(self, chart):
         """Recursively build chart dependencies for ``chart``.
 
@@ -159,7 +143,6 @@ class Manifest(object):
             under ``chart['data']['dependencies']`` could not be found.
         """
         try:
-            dep = None
             chart_dependencies = chart.get('data', {}).get('dependencies', [])
             for iter, dep in enumerate(chart_dependencies):
                 if isinstance(dep, dict):
@@ -170,9 +153,10 @@ class Manifest(object):
                     'chart': chart_dep.get('data', {})
                 }
         except Exception:
-            raise exceptions.ManifestException(
+            raise exceptions.ChartDependencyException(
-                details="Could not find dependency chart {} in {}".format(
+                details="Could not build dependencies for chart {} in {}".
-                    dep, const.DOCUMENT_CHART))
+                format(
+                    chart.get('metadata').get('name'), const.DOCUMENT_CHART))
         else:
             return chart
 
@@ -193,15 +177,17 @@ class Manifest(object):
                 if isinstance(chart, dict):
                     continue
                 chart_dep = self.find_chart_document(chart)
+                self.build_chart_deps(chart_dep)
                 chart_group['data']['chart_group'][iter] = {
                     'chart': chart_dep.get('data', {})
                 }
-        except Exception:
+        except exceptions.ManifestException:
-            raise exceptions.ManifestException(
+            cg_name = chart_group.get('metadata', {}).get('name')
-                details="Could not find chart {} in {}".format(
+            raise exceptions.BuildChartGroupException(
-                    chart, const.DOCUMENT_GROUP))
+                details="Could not build chart group {} in {}".format(
-        else:
+                    cg_name, const.DOCUMENT_GROUP))
-            return chart_group
+
+        return chart_group
 
     def build_armada_manifest(self):
         """Builds the Armada manifest while pulling out data
@@ -212,36 +198,27 @@ class Manifest(object):
         :raises ManifestException: If a chart group's data listed
             under ``chart_group['data']`` could not be found.
         """
-        try:
+        for iter, group in enumerate(
-            group = None
+                self.manifest.get('data', {}).get('chart_groups', [])):
-            for iter, group in enumerate(
+            if isinstance(group, dict):
-                    self.manifest.get('data', {}).get('chart_groups', [])):
+                continue
-                if isinstance(group, dict):
+            chart_grp = self.find_chart_group_document(group)
-                    continue
+            self.build_chart_group(chart_grp)
-                chart_grp = self.find_chart_group_document(group)
 
-                # Add name to chart group
+            # Add name to chart group
-                ch_grp_data = chart_grp.get('data', {})
+            ch_grp_data = chart_grp.get('data', {})
-                ch_grp_data['name'] = chart_grp.get('metadata', {}).get('name')
+            ch_grp_data['name'] = chart_grp.get('metadata', {}).get('name')
 
-                self.manifest['data']['chart_groups'][iter] = ch_grp_data
+            self.manifest['data']['chart_groups'][iter] = ch_grp_data
-        except Exception:
+
-            raise exceptions.ManifestException(
+        return self.manifest
-                "Could not find chart group {} in {}".format(
-                    group, const.DOCUMENT_MANIFEST))
-        else:
-            return self.manifest
 
     def get_manifest(self):
-        """Builds all of the documents including the dependencies of the
+        """Builds the Armada manifest
-        chart documents, the charts in the chart_groups, and the
-        Armada manifest
 
         :returns: The Armada manifest.
         :rtype: dict
         """
-        self.build_charts_deps()
-        self.build_chart_groups()
         self.build_armada_manifest()
 
         return {'armada': self.manifest.get('data', {})}

armada/tests/unit/api/test_test_controller.py

@@ -143,8 +143,9 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest):
         self.assertEqual(1, resp_body['details']['errorCount'])
         self.assertIn({
             'message':
-            ('An error occurred while generating the manifest: Could not '
+            ('An error occurred while building chart group: '
-             'find dependency chart helm-toolkit in armada/Chart/v1.'),
+             'Could not build chart group keystone-infra-services in '
+             'armada/ChartGroup/v1.'),
             'error':
             True,
             'kind':

armada/tests/unit/handlers/test_manifest.py

@@ -40,12 +40,12 @@ class ManifestTestCase(testtools.TestCase):
         self.assertIsInstance(armada_manifest.groups, list)
         self.assertIsNotNone(armada_manifest.manifest)
 
-        self.assertEqual(4, len(armada_manifest.charts))
+        self.assertEqual(5, len(armada_manifest.charts))
-        self.assertEqual(2, len(armada_manifest.groups))
+        self.assertEqual(3, len(armada_manifest.groups))
 
-        self.assertEqual([self.documents[x] for x in range(4)],
+        self.assertEqual([self.documents[x] for x in range(5)],
                          armada_manifest.charts)
-        self.assertEqual([self.documents[x] for x in range(4, 6)],
+        self.assertEqual([self.documents[x] for x in range(5, 8)],
                          armada_manifest.groups)
         self.assertEqual(self.documents[-1], armada_manifest.manifest)
 
@@ -59,12 +59,12 @@ class ManifestTestCase(testtools.TestCase):
         self.assertIsInstance(armada_manifest.groups, list)
         self.assertIsNotNone(armada_manifest.manifest)
 
-        self.assertEqual(4, len(armada_manifest.charts))
+        self.assertEqual(5, len(armada_manifest.charts))
-        self.assertEqual(2, len(armada_manifest.groups))
+        self.assertEqual(3, len(armada_manifest.groups))
 
-        self.assertEqual([self.documents[x] for x in range(4)],
+        self.assertEqual([self.documents[x] for x in range(5)],
                          armada_manifest.charts)
-        self.assertEqual([self.documents[x] for x in range(4, 6)],
+        self.assertEqual([self.documents[x] for x in range(5, 8)],
                          armada_manifest.groups)
         self.assertEqual(self.documents[-1], armada_manifest.manifest)
         self.assertEqual('armada-manifest',
@@ -87,12 +87,12 @@ class ManifestTestCase(testtools.TestCase):
         self.assertIsInstance(armada_manifest.groups, list)
         self.assertIsNotNone(armada_manifest.manifest)
 
-        self.assertEqual(4, len(armada_manifest.charts))
+        self.assertEqual(5, len(armada_manifest.charts))
-        self.assertEqual(2, len(armada_manifest.groups))
+        self.assertEqual(3, len(armada_manifest.groups))
 
-        self.assertEqual([self.documents[x] for x in range(4)],
+        self.assertEqual([self.documents[x] for x in range(5)],
                          armada_manifest.charts)
-        self.assertEqual([self.documents[x] for x in range(4, 6)],
+        self.assertEqual([self.documents[x] for x in range(5, 8)],
                          armada_manifest.groups)
         self.assertEqual(armada_manifest.manifest, self.documents[-1])
         self.assertEqual('armada-manifest',
@@ -107,7 +107,8 @@ class ManifestTestCase(testtools.TestCase):
                          armada_manifest.manifest['metadata']['name'])
 
     def test_get_manifest(self):
-        armada_manifest = manifest.Manifest(self.documents)
+        armada_manifest = manifest.Manifest(
+            self.documents, target_manifest='armada-manifest')
         obtained_manifest = armada_manifest.get_manifest()
         self.assertIsInstance(obtained_manifest, dict)
         self.assertEqual(obtained_manifest['armada'],
@@ -116,7 +117,7 @@ class ManifestTestCase(testtools.TestCase):
     def test_find_documents(self):
         armada_manifest = manifest.Manifest(self.documents)
         chart_documents, chart_groups, manifests = armada_manifest. \
-            _find_documents()
+            _find_documents(target_manifest='armada-manifest')
 
         # checking if all the chart documents are present
         self.assertIsInstance(chart_documents, list)
@@ -174,13 +175,13 @@ class ManifestTestCase(testtools.TestCase):
         ok_chart = armada_manifest. \
             find_chart_group_document('openstack-keystone')
         self.assertIsInstance(ok_chart, dict)
-        self.assertEqual(self.documents[-2], ok_chart)
+        self.assertEqual(self.documents[-3], ok_chart)
 
         armada_manifest = manifest.Manifest(self.documents)
         kis_chart = armada_manifest.find_chart_group_document(
             'keystone-infra-services')
         self.assertIsInstance(kis_chart, dict)
-        self.assertEqual(self.documents[-3], kis_chart)
+        self.assertEqual(self.documents[-4], kis_chart)
 
     def test_verify_build_armada_manifest(self):
         armada_manifest = manifest.Manifest(self.documents)
@@ -375,11 +376,11 @@ class ManifestNegativeTestCase(testtools.TestCase):
 
     def test_get_documents_missing_charts(self):
         # Validates exceptions.ManifestException is thrown if no chart is
-        # found. Charts are first 4 documents in sample YAML.
+        # found. Charts are first 5 documents in sample YAML.
         error_re = ('Documents must be a list of documents with at least one '
                     'of each of the following schemas: .*')
         self.assertRaisesRegexp(exceptions.ManifestException, error_re,
-                                manifest.Manifest, self.documents[4:])
+                                manifest.Manifest, self.documents[5:])
 
     def test_get_documents_missing_chart_groups(self):
         # Validates exceptions.ManifestException is thrown if no chart is
@@ -392,16 +393,16 @@ class ManifestNegativeTestCase(testtools.TestCase):
 
     def test_find_chart_document_negative(self):
         armada_manifest = manifest.Manifest(self.documents)
-        error_re = r'Could not find a %s named "%s"' % (const.DOCUMENT_CHART,
+        error_re = r'Could not build %s named "%s"' % (const.DOCUMENT_CHART,
-                                                        'invalid')
+                                                       'invalid')
-        self.assertRaisesRegexp(exceptions.ManifestException, error_re,
+        self.assertRaisesRegexp(exceptions.BuildChartException, error_re,
                                 armada_manifest.find_chart_document, 'invalid')
 
     def test_find_group_document_negative(self):
         armada_manifest = manifest.Manifest(self.documents)
-        error_re = r'Could not find a %s named "%s"' % (const.DOCUMENT_GROUP,
+        error_re = r'Could not build %s named "%s"' % (const.DOCUMENT_GROUP,
-                                                        'invalid')
+                                                       'invalid')
-        self.assertRaisesRegexp(exceptions.ManifestException, error_re,
+        self.assertRaisesRegexp(exceptions.BuildChartGroupException, error_re,
                                 armada_manifest.find_chart_group_document,
                                 'invalid')
 

armada/tests/unit/resources/keystone-manifest.yaml

@@ -102,6 +102,27 @@ data:
   dependencies:
     - helm-toolkit
 ---
+schema: armada/Chart/v1
+metadata:
+  schema: metadata/Document/v1
+  name: ro-global
+  labels:
+    name: ro-global
+    component: ro
+  layeringDefinition:
+    abstract: false
+    layer: global
+    actions:
+      - method: merge
+        path: .
+  storagePolicy: cleartext
+data:
+  source: {location: '', subpath: charts/ro, type: git}
+  release: ro-release
+  chart_name: ro
+  namespace: openstack
+  dependencies: []
+---
 schema: armada/ChartGroup/v1
 metadata:
   schema: metadata/Document/v1
@@ -123,6 +144,19 @@ data:
   chart_group:
     - keystone
 ---
+schema: armada/ChartGroup/v1
+metadata:
+  schema: metadata/Document/v1
+  name: ro-inventory-notifier
+  layeringDefinition:
+    abstract: false
+    layer: global
+  storagePolicy: cleartext
+data:
+  description: Deploy RO
+  chart_group:
+    - ro
+---
 schema: armada/Manifest/v1
 metadata:
   schema: metadata/Document/v1

armada/utils/validate.py

@@ -16,6 +16,7 @@ import jsonschema
 import os
 import pkg_resources
 import requests
+import traceback
 import yaml
 
 from oslo_log import log as logging
@@ -74,6 +75,7 @@ def _validate_armada_manifest(manifest):
     except ManifestException as me:
         vmsg = ValidationMessage(
             message=str(me), error=True, name='ARM001', level='Error')
+        LOG.error(traceback.format_exc())
         LOG.error('ValidationMessage: %s', vmsg.get_output_json())
         details.append(vmsg.get_output())
         return False, details