From b79c4cd411bb6f699a592f50b9bd8983e010d8ec Mon Sep 17 00:00:00 2001 From: Sean Eagan Date: Tue, 21 Aug 2018 16:40:27 -0500 Subject: [PATCH] Release diffing: split out, add unit tests, include name This makes the following changes to release diffing: * Move into own handler. * Add unit tests. * Include `chart.metadata.name` (from Chart.yaml / dependency dir name) since that can affect the values passed to dependency charts, and it's sometimes used by templates via `{{ .Chart.name }}` in ways which may necessitate an upgrade. Change-Id: I241384971ed85e5658d71348fee2d6320bbb3c45 --- armada/handlers/armada.py | 65 +----- armada/handlers/release_diff.py | 98 ++++++++ .../tests/unit/handlers/test_release_diff.py | 210 ++++++++++++++++++ 3 files changed, 312 insertions(+), 61 deletions(-) create mode 100644 armada/handlers/release_diff.py create mode 100644 armada/tests/unit/handlers/test_release_diff.py diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index 412b1220..e7b1d85f 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from deepdiff import DeepDiff import functools import time import yaml @@ -29,6 +28,7 @@ from armada.exceptions import validate_exceptions from armada.handlers.chartbuilder import ChartBuilder from armada.handlers.manifest import Manifest from armada.handlers.override import Override +from armada.handlers.release_diff import ReleaseDiff from armada.handlers.test import test_release_for_success from armada.handlers.tiller import Tiller from armada.utils.release import release_prefixer @@ -590,66 +590,6 @@ class Armada(object): LOG.info("Test failed for release: %s", release_name) raise tiller_exceptions.TestFailedException(release_name) - def get_diff(self, old_chart, old_values, new_chart, new_values): - ''' - Get the diff between old and new chart release inputs to determine - whether an upgrade is needed. - - Release inputs which are relevant are the override values given, and - the chart content including: - - * default values (values.yaml), - * templates and their content - * files and their content - * the above for each chart on which the chart depends transitively. - - This excludes Chart.yaml content as that is rarely used by the chart - via ``{{ .Chart }}``, and even when it is does not usually necessitate - an upgrade. - - :param old_chart: The deployed chart. - :type old_chart: Chart - :param old_values: The deployed chart override values. - :type old_values: dict - :param new_chart: The chart to deploy. - :type new_chart: Chart - :param new_values: The chart override values to deploy. - :type new_values: dict - :return: Mapping of difference types to sets of those differences. - :rtype: dict - ''' - - def make_release_input(chart, values, desc): - # TODO(seaneagan): Should we include `chart.metadata` (Chart.yaml)? - try: - default_values = yaml.safe_load(chart.values.raw) - except yaml.YAMLError: - chart_desc = '{} ({})'.format(chart.metadata.name, desc) - raise armada_exceptions.InvalidValuesYamlException(chart_desc) - files = {f.type_url: f.value for f in chart.files} - templates = {t.name: t.data for t in chart.templates} - dependencies = { - d.metadata.name: make_release_input(d) - for d in chart.dependencies - } - - return { - 'chart': { - 'values': default_values, - 'files': files, - 'templates': templates, - 'dependencies': dependencies - }, - 'values': values - } - - old_input = make_release_input(old_chart, old_values, - 'previously deployed') - new_input = make_release_input(new_chart, new_values, - 'currently being deployed') - - return DeepDiff(old_input, new_input, view='tree') - def _chart_cleanup(self, prefix, charts, msg): LOG.info('Processing chart cleanup to remove unspecified releases.') @@ -669,3 +609,6 @@ class Armada(object): release) self.tiller.uninstall_release(release) msg['purge'].append(release) + + def get_diff(self, old_chart, old_values, new_chart, values): + return ReleaseDiff(old_chart, old_values, new_chart, values).get_diff() diff --git a/armada/handlers/release_diff.py b/armada/handlers/release_diff.py new file mode 100644 index 00000000..aff37482 --- /dev/null +++ b/armada/handlers/release_diff.py @@ -0,0 +1,98 @@ +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from deepdiff import DeepDiff +import yaml + +from armada.exceptions import armada_exceptions + + +class ReleaseDiff(object): + ''' + A utility for discovering diffs in helm release inputs, for example to + determine whether an upgrade is needed and what specific changes will be + applied. + + Release inputs which are relevant are the override values given, and + the chart content including: + + * default values (values.yaml), + * templates and their content + * files and their content + * the above for each chart on which the chart depends transitively. + + This excludes Chart.yaml content as that is rarely used by the chart + via ``{{ .Chart }}``, and even when it is does not usually necessitate + an upgrade. + + :param old_chart: The deployed chart. + :type old_chart: Chart + :param old_values: The deployed chart override values. + :type old_values: dict + :param new_chart: The chart to deploy. + :type new_chart: Chart + :param new_values: The chart override values to deploy. + :type new_values: dict + ''' + + def __init__(self, old_chart, old_values, new_chart, new_values): + self.old_chart = old_chart + self.old_values = old_values + self.new_chart = new_chart + self.new_values = new_values + + def get_diff(self): + ''' + Get the diff. + + :return: Mapping of difference types to sets of those differences. + :rtype: dict + ''' + + old_input = self.make_release_input(self.old_chart, self.old_values, + 'previously deployed') + new_input = self.make_release_input(self.new_chart, self.new_values, + 'currently being deployed') + + return DeepDiff(old_input, new_input, view='tree') + + def make_release_input(self, chart, values, desc): + return {'chart': self.make_chart_dict(chart, desc), 'values': values} + + def make_chart_dict(self, chart, desc): + try: + default_values = yaml.safe_load(chart.values.raw) + except yaml.YAMLError: + chart_desc = '{} ({})'.format(chart.metadata.name, desc) + raise armada_exceptions.InvalidValuesYamlException(chart_desc) + files = {f.type_url: f.value for f in chart.files} + templates = {t.name: t.data for t in chart.templates} + dependencies = { + d.metadata.name: self.make_chart_dict( + d, '{}({} dependency)'.format(desc, d.metadata.name)) + for d in chart.dependencies + } + + return { + # TODO(seaneagan): Are there use cases to include other + # `chart.metadata` (Chart.yaml) fields? If so, could include option + # under `upgrade` key in armada chart schema for this. Or perhaps + # can even add `upgrade.always` there to handle dynamic things + # used in charts like dates, environment variables, etc. + 'name': chart.metadata.name, + 'values': default_values, + 'files': files, + 'templates': templates, + 'dependencies': dependencies + } diff --git a/armada/tests/unit/handlers/test_release_diff.py b/armada/tests/unit/handlers/test_release_diff.py new file mode 100644 index 00000000..aa1caec6 --- /dev/null +++ b/armada/tests/unit/handlers/test_release_diff.py @@ -0,0 +1,210 @@ +# Copyright 2018 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from armada.handlers.release_diff import ReleaseDiff +from armada.tests.unit import base + +from google.protobuf.any_pb2 import Any +from hapi.chart.chart_pb2 import Chart +from hapi.chart.config_pb2 import Config +from hapi.chart.metadata_pb2 import Metadata +from hapi.chart.template_pb2 import Template + + +# Tests for diffs which can occur in both top-level or dependency charts, +# and thus are inherited by both of those test classes. +class _BaseReleaseDiffTestCase(): + + def setUp(self): + super(base.ArmadaTestCase, self).setUp() + self.old_chart = self.make_chart() + self.old_values = self.make_values() + + def make_chart(self): + chart = self._make_chart() + dep = self._make_chart() + dep.metadata.name = 'dep1' + sub_dep = self._make_chart() + sub_dep.metadata.name = 'dep1' + sub_sub_dep = self._make_chart() + sub_sub_dep.metadata.name = 'dep1' + sub_dep.dependencies.extend([sub_sub_dep]) + dep.dependencies.extend([sub_dep]) + chart.dependencies.extend([dep]) + return chart + + def _make_chart(self): + return Chart( + metadata=Metadata( + description='chart description', + name='chart_name', + version='0.1.2'), + templates=[ + Template( + name='template_name', data='template content'.encode()) + ], + files=[ + Any(type_url='./file_name.ext', value='file content'.encode()) + ], + dependencies=[], + values=Config(raw='{param: d1}')) + + def make_values(self): + return {'param': 'o1'} + + def _test_chart_diff(self, update_chart): + new_chart = self.make_chart() + chart_to_update = self.get_chart_to_update(new_chart) + update_chart(chart_to_update) + diff = ReleaseDiff(self.old_chart, self.old_values, new_chart, + self.old_values).get_diff() + self.assertTrue(diff) + + def get_chart_to_update(self, chart): + raise NotImplementedError('Implement in subclass') + + def test_metadata_non_name_diff_ignored(self): + new_chart = self.make_chart() + chart_to_update = self.get_chart_to_update(new_chart) + chart_to_update.metadata.description = 'new chart description' + diff = ReleaseDiff(self.old_chart, self.old_values, new_chart, + self.old_values).get_diff() + self.assertFalse(diff) + + def test_metadata_name_diff(self): + + def update_chart(chart): + chart.metadata.name = 'new_chart_name' + + self._test_chart_diff(update_chart) + + def test_default_values_diff(self): + + def update_chart(chart): + chart.values.raw = '{param: d2}' + + self._test_chart_diff(update_chart) + + def test_template_name_diff(self): + + def update_chart(chart): + chart.templates[0].name = 'new_template_name' + + self._test_chart_diff(update_chart) + + def test_template_data_diff(self): + + def update_chart(chart): + chart.templates[0].data = 'new template content'.encode() + + self._test_chart_diff(update_chart) + + def test_add_template_diff(self): + + def update_chart(chart): + chart.templates.extend([ + Template( + name='new_template_name', + data='new template content'.encode()) + ]) + + self._test_chart_diff(update_chart) + + def test_remove_template_diff(self): + + def update_chart(chart): + del chart.templates[0] + + self._test_chart_diff(update_chart) + + def test_file_type_url_diff(self): + + def update_chart(chart): + chart.files[0].type_url = './new_file_name.ext' + + self._test_chart_diff(update_chart) + + def test_file_value_diff(self): + + def update_chart(chart): + chart.files[0].value = 'new file content'.encode() + + self._test_chart_diff(update_chart) + + def test_add_file_diff(self): + + def update_chart(chart): + chart.files.extend([ + Any(type_url='./new_file_name.ext', + value='new file content'.encode()) + ]) + + self._test_chart_diff(update_chart) + + def test_remove_file_diff(self): + + def update_chart(chart): + del chart.files[0] + + self._test_chart_diff(update_chart) + + def test_add_dependency_diff(self): + + def update_chart(chart): + dep = self._make_chart() + dep.metadata.name = 'dep2' + chart.dependencies.extend([dep]) + + self._test_chart_diff(update_chart) + + def test_remove_dependency_diff(self): + + def update_chart(chart): + del chart.dependencies[0] + + self._test_chart_diff(update_chart) + + +# Test diffs (or absence of) in top-level chart / values. +class ReleaseDiffTestCase(_BaseReleaseDiffTestCase, base.ArmadaTestCase): + + def get_chart_to_update(self, chart): + return chart + + def test_same_input_no_diff(self): + diff = ReleaseDiff(self.old_chart, self.old_values, self.make_chart(), + self.make_values()).get_diff() + self.assertFalse(diff) + + def test_override_values_diff(self): + new_values = {'param': 'o2'} + diff = ReleaseDiff(self.old_chart, self.old_values, self.old_chart, + new_values).get_diff() + self.assertTrue(diff) + + +# Test diffs in dependencies. +class DependencyReleaseDiffTestCase(_BaseReleaseDiffTestCase, + base.ArmadaTestCase): + + def get_chart_to_update(self, chart): + return chart.dependencies[0] + + +# Test diffs in transitive dependencies. +class TransitiveDependencyReleaseDiffTestCase(_BaseReleaseDiffTestCase, + base.ArmadaTestCase): + + def get_chart_to_update(self, chart): + return chart.dependencies[0].dependencies[0]