airship/armada
Code Issues Proposed changes

Fix for get manifest

Browse Source 
In some use cases, some site level docs are only included in specific
manifests. This is so sites can call out what they want deployed, however
currently Armada is checking for all documents to exist and leads
to an invalid manifest exception.

This PS removes the '.build_charts_deps()' and 'build_chart_groups()' calls
in 'get_manifest()' so that only chart documents, and chart group documents
are built after finding them within 'build_armada_manfiest()' and
'build_chart_group()'. 'build_armada_manifest()' will now throw the
related 'Could not find chart group... exception' for related chart
and chart group issues.  Additional subclass exceptions were added along
with adding traceback to capture the chained exceptions.

Change-Id: Idc8a75b290ac0afb1e177203535b012d589b708f
This commit is contained in:
anthony.bellino 2018-09-04 19:34:11 +00:00
parent 51b1bb8a7a
commit cb57588968
7 changed files with 137 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
armada/exceptions/__init__.py
View File

@ -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'
]

37
armada/exceptions/manifest_exceptions.py
View File

@ -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.'

81
armada/handlers/manifest.py
View File

@ -112,8 +112,8 @@ class Manifest(object):
for chart in self.charts:
if chart.get('metadata', {}).get('name') == name:
return chart
raise exceptions.ManifestException(
details='Could not find a {} named "{}"'.format(
raise exceptions.BuildChartException(
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(
details='Could not find a {} named "{}"'.format(
raise exceptions.BuildChartGroupException(
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(
details="Could not find dependency chart {} in {}".format(
dep, const.DOCUMENT_CHART))
raise exceptions.ChartDependencyException(
details="Could not build dependencies for chart {} in {}".
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:
raise exceptions.ManifestException(
details="Could not find chart {} in {}".format(
chart, const.DOCUMENT_GROUP))
else:
return chart_group
except exceptions.ManifestException:
cg_name = chart_group.get('metadata', {}).get('name')
raise exceptions.BuildChartGroupException(
details="Could not build chart group {} in {}".format(
cg_name, const.DOCUMENT_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:
group = None
for iter, group in enumerate(
self.manifest.get('data', {}).get('chart_groups', [])):
if isinstance(group, dict):
continue
chart_grp = self.find_chart_group_document(group)
for iter, group in enumerate(
self.manifest.get('data', {}).get('chart_groups', [])):
if isinstance(group, dict):
continue
chart_grp = self.find_chart_group_document(group)
self.build_chart_group(chart_grp)
# Add name to chart group
ch_grp_data = chart_grp.get('data', {})
ch_grp_data['name'] = chart_grp.get('metadata', {}).get('name')
# Add name to chart group
ch_grp_data = chart_grp.get('data', {})
ch_grp_data['name'] = chart_grp.get('metadata', {}).get('name')
self.manifest['data']['chart_groups'][iter] = ch_grp_data
except Exception:
raise exceptions.ManifestException(
"Could not find chart group {} in {}".format(
group, const.DOCUMENT_MANIFEST))
else:
return self.manifest
self.manifest['data']['chart_groups'][iter] = ch_grp_data
return self.manifest
def get_manifest(self):
"""Builds all of the documents including the dependencies of the
chart documents, the charts in the chart_groups, and the
Armada manifest
"""Builds 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', {})}

5
armada/tests/unit/api/test_test_controller.py
View File

@ -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 '
'find dependency chart helm-toolkit in armada/Chart/v1.'),
('An error occurred while building chart group: '
'Could not build chart group keystone-infra-services in '
'armada/ChartGroup/v1.'),
'error':
True,
'kind':

49
armada/tests/unit/handlers/test_manifest.py
View File

@ -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(2, len(armada_manifest.groups))
self.assertEqual(5, len(armada_manifest.charts))
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(2, len(armada_manifest.groups))
self.assertEqual(5, len(armada_manifest.charts))
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(2, len(armada_manifest.groups))
self.assertEqual(5, len(armada_manifest.charts))
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,
'invalid')
self.assertRaisesRegexp(exceptions.ManifestException, error_re,
error_re = r'Could not build %s named "%s"' % (const.DOCUMENT_CHART,
'invalid')
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,
'invalid')
self.assertRaisesRegexp(exceptions.ManifestException, error_re,
error_re = r'Could not build %s named "%s"' % (const.DOCUMENT_GROUP,
'invalid')
self.assertRaisesRegexp(exceptions.BuildChartGroupException, error_re,
armada_manifest.find_chart_group_document,
'invalid')

34
armada/tests/unit/resources/keystone-manifest.yaml
View File

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

2
armada/utils/validate.py
View File

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