Merge "Move to semantic diffing of charts"
This commit is contained in:
commit
e4a270b06d
@ -40,3 +40,29 @@ class ProtectedReleaseException(ArmadaException):
|
||||
'Armada encountered protected release %s in FAILED status' %
|
||||
reason)
|
||||
super(ProtectedReleaseException, self).__init__(self._message)
|
||||
|
||||
|
||||
class InvalidValuesYamlException(ArmadaException):
|
||||
'''
|
||||
Exception that occurs when Armada encounters invalid values.yaml content in
|
||||
a helm chart.
|
||||
'''
|
||||
|
||||
def __init__(self, chart_description):
|
||||
self._message = (
|
||||
'Armada encountered invalid values.yaml in helm chart: %s' %
|
||||
chart_description)
|
||||
super(InvalidValuesYamlException, self).__init__(self._message)
|
||||
|
||||
|
||||
class InvalidOverrideValuesYamlException(ArmadaException):
|
||||
'''
|
||||
Exception that occurs when Armada encounters invalid override yaml in
|
||||
helm chart.
|
||||
'''
|
||||
|
||||
def __init__(self, chart_description):
|
||||
self._message = (
|
||||
'Armada encountered invalid values.yaml in helm chart: %s' %
|
||||
chart_description)
|
||||
super(InvalidValuesYamlException, self).__init__(self._message)
|
||||
|
@ -12,7 +12,7 @@
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import difflib
|
||||
from deepdiff import DeepDiff
|
||||
import functools
|
||||
import time
|
||||
import yaml
|
||||
@ -370,7 +370,7 @@ class Armada(object):
|
||||
'cleanup', False)
|
||||
|
||||
chartbuilder = ChartBuilder(chart)
|
||||
protoc_chart = chartbuilder.get_helm_chart()
|
||||
new_chart = chartbuilder.get_helm_chart()
|
||||
|
||||
# Begin Chart timeout deadline
|
||||
deadline = time.time() + wait_timeout
|
||||
@ -385,7 +385,7 @@ class Armada(object):
|
||||
release_name, namespace)
|
||||
# extract the installed chart and installed values from the
|
||||
# latest release so we can compare to the intended state
|
||||
apply_chart, apply_values = self.find_release_chart(
|
||||
old_chart, old_values_string = self.find_release_chart(
|
||||
deployed_releases, release_name)
|
||||
|
||||
upgrade = chart.get('upgrade', {})
|
||||
@ -404,20 +404,26 @@ class Armada(object):
|
||||
if not self.disable_update_post and upgrade_post:
|
||||
post_actions = upgrade_post
|
||||
|
||||
# Show delta for both the chart templates and the chart
|
||||
# values
|
||||
# TODO(alanmeadows) account for .files differences
|
||||
# once we support those
|
||||
LOG.info('Checking upgrade chart diffs.')
|
||||
upgrade_diff = self.show_diff(chart, apply_chart,
|
||||
apply_values,
|
||||
chartbuilder.dump(), values,
|
||||
msg)
|
||||
try:
|
||||
old_values = yaml.safe_load(old_values_string)
|
||||
except yaml.YAMLError:
|
||||
chart_desc = '{} (previously deployed)'.format(
|
||||
old_chart.metadata.name)
|
||||
raise armada_exceptions.\
|
||||
InvalidOverrideValuesYamlException(chart_desc)
|
||||
|
||||
if not upgrade_diff:
|
||||
LOG.info("There are no updates found in this chart")
|
||||
LOG.info('Checking for updates to chart release inputs.')
|
||||
diff = self.get_diff(old_chart, old_values, new_chart,
|
||||
values)
|
||||
|
||||
if not diff:
|
||||
LOG.info("Found no updates to chart release inputs")
|
||||
continue
|
||||
|
||||
LOG.info("Found updates to chart release inputs")
|
||||
LOG.debug("%s", diff)
|
||||
msg['diff'].append({chart['release']: str(diff)})
|
||||
|
||||
# TODO(MarshM): Add tiller dry-run before upgrade and
|
||||
# consider deadline impacts
|
||||
|
||||
@ -426,7 +432,7 @@ class Armada(object):
|
||||
LOG.info('Beginning Upgrade, wait=%s, timeout=%ss',
|
||||
this_chart_should_wait, timer)
|
||||
tiller_result = self.tiller.update_release(
|
||||
protoc_chart,
|
||||
new_chart,
|
||||
release_name,
|
||||
namespace,
|
||||
pre_actions=pre_actions,
|
||||
@ -458,7 +464,7 @@ class Armada(object):
|
||||
LOG.info('Beginning Install, wait=%s, timeout=%ss',
|
||||
this_chart_should_wait, timer)
|
||||
tiller_result = self.tiller.install_release(
|
||||
protoc_chart,
|
||||
new_chart,
|
||||
release_name,
|
||||
namespace,
|
||||
values=yaml.safe_dump(values),
|
||||
@ -584,49 +590,65 @@ class Armada(object):
|
||||
LOG.info("Test failed for release: %s", release_name)
|
||||
raise tiller_exceptions.TestFailedException(release_name)
|
||||
|
||||
def show_diff(self, chart, installed_chart, installed_values, target_chart,
|
||||
target_values, msg):
|
||||
'''Produce a unified diff of the installed chart vs our intention'''
|
||||
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.
|
||||
|
||||
# TODO(MarshM) This gives decent output comparing values. Would be
|
||||
# nice to clean it up further. Are \\n or \n\n ever valid diffs?
|
||||
# Can these be cleanly converted to dicts, for easier compare?
|
||||
def _sanitize_diff_str(str):
|
||||
return str.replace('\\n', '\n').replace('\n\n', '\n').split('\n')
|
||||
Release inputs which are relevant are the override values given, and
|
||||
the chart content including:
|
||||
|
||||
source = _sanitize_diff_str(str(installed_chart.SerializeToString()))
|
||||
target = _sanitize_diff_str(str(target_chart))
|
||||
chart_diff = list(difflib.unified_diff(source, target, n=0))
|
||||
* default values (values.yaml),
|
||||
* templates and their content
|
||||
* files and their content
|
||||
* the above for each chart on which the chart depends transitively.
|
||||
|
||||
chart_release = chart.get('release', None)
|
||||
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.
|
||||
|
||||
if len(chart_diff) > 0:
|
||||
LOG.info("Found diff in Chart (%s)", chart_release)
|
||||
diff_msg = []
|
||||
for line in chart_diff:
|
||||
diff_msg.append(line)
|
||||
msg['diff'].append({'chart': diff_msg})
|
||||
: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
|
||||
'''
|
||||
|
||||
pretty_diff = '\n'.join(diff_msg)
|
||||
LOG.debug(pretty_diff)
|
||||
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
|
||||
}
|
||||
|
||||
source = _sanitize_diff_str(installed_values)
|
||||
target = _sanitize_diff_str(yaml.safe_dump(target_values))
|
||||
values_diff = list(difflib.unified_diff(source, target, n=0))
|
||||
return {
|
||||
'chart': {
|
||||
'values': default_values,
|
||||
'files': files,
|
||||
'templates': templates,
|
||||
'dependencies': dependencies
|
||||
},
|
||||
'values': values
|
||||
}
|
||||
|
||||
if len(values_diff) > 0:
|
||||
LOG.info("Found diff in values (%s)", chart_release)
|
||||
diff_msg = []
|
||||
for line in values_diff:
|
||||
diff_msg.append(line)
|
||||
msg['diff'].append({'values': diff_msg})
|
||||
old_input = make_release_input(old_chart, old_values,
|
||||
'previously deployed')
|
||||
new_input = make_release_input(new_chart, new_values,
|
||||
'currently being deployed')
|
||||
|
||||
pretty_diff = '\n'.join(diff_msg)
|
||||
LOG.debug(pretty_diff)
|
||||
|
||||
result = (len(chart_diff) > 0) or (len(values_diff) > 0)
|
||||
|
||||
return result
|
||||
return DeepDiff(old_input, new_input, view='tree')
|
||||
|
||||
def _chart_cleanup(self, prefix, charts, msg):
|
||||
LOG.info('Processing chart cleanup to remove unspecified releases.')
|
||||
|
@ -320,7 +320,8 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
|
||||
def _test_sync(self,
|
||||
known_releases,
|
||||
test_success=True,
|
||||
test_failure_to_run=False):
|
||||
test_failure_to_run=False,
|
||||
diff={'some_key': {'some diff'}}):
|
||||
"""Test install functionality from the sync() method."""
|
||||
|
||||
@mock.patch.object(armada.Armada, 'post_flight_ops')
|
||||
@ -333,7 +334,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
|
||||
# Instantiate Armada object.
|
||||
yaml_documents = list(yaml.safe_load_all(TEST_YAML))
|
||||
armada_obj = armada.Armada(yaml_documents)
|
||||
armada_obj.show_diff = mock.Mock()
|
||||
armada_obj.get_diff = mock.Mock()
|
||||
|
||||
chart_group = armada_obj.manifest['armada']['chart_groups'][0]
|
||||
charts = chart_group['chart_group']
|
||||
@ -358,6 +359,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
|
||||
mock_chartbuilder.get_source_path.return_value = None
|
||||
mock_chartbuilder.get_helm_chart.return_value = None
|
||||
|
||||
# Simulate chart diff, upgrade should only happen if non-empty.
|
||||
armada_obj.get_diff.return_value = diff
|
||||
|
||||
armada_obj.sync()
|
||||
|
||||
expected_install_release_calls = []
|
||||
@ -374,6 +378,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
|
||||
# multiple conditions, so this is enough.
|
||||
this_chart_should_wait = chart['wait']['timeout'] > 0
|
||||
|
||||
expected_apply = True
|
||||
if release_name not in [x[0] for x in known_releases]:
|
||||
expected_install_release_calls.append(
|
||||
mock.call(
|
||||
@ -418,17 +423,22 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
|
||||
break
|
||||
|
||||
if status == const.STATUS_DEPLOYED:
|
||||
if not diff:
|
||||
expected_apply = False
|
||||
else:
|
||||
upgrade = chart.get('upgrade', {})
|
||||
disable_hooks = upgrade.get('no_hooks', False)
|
||||
force = upgrade.get('force', False)
|
||||
recreate_pods = upgrade.get('recreate_pods', False)
|
||||
recreate_pods = upgrade.get(
|
||||
'recreate_pods', False)
|
||||
|
||||
expected_update_release_calls.append(
|
||||
mock.call(
|
||||
mock_chartbuilder().get_helm_chart(),
|
||||
"{}-{}".format(
|
||||
armada_obj.manifest['armada']
|
||||
['release_prefix'], chart['release']),
|
||||
armada_obj.manifest['armada'][
|
||||
'release_prefix'],
|
||||
chart['release']),
|
||||
chart['namespace'],
|
||||
pre_actions={},
|
||||
post_actions={},
|
||||
@ -451,7 +461,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
|
||||
test_cleanup = test_chart_override.get('options', {}).get(
|
||||
'cleanup', False)
|
||||
|
||||
if test_this_chart:
|
||||
if test_this_chart and expected_apply:
|
||||
expected_test_release_for_success_calls.append(
|
||||
mock.call(
|
||||
m_tiller,
|
||||
@ -508,23 +518,21 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
|
||||
def test_armada_sync_with_one_deployed_release(self):
|
||||
c1 = 'armada-test_chart_1'
|
||||
|
||||
known_releases = [[
|
||||
c1, None,
|
||||
self._get_chart_by_name(c1), None, const.STATUS_DEPLOYED
|
||||
]]
|
||||
known_releases = [[c1, None, None, "{}", const.STATUS_DEPLOYED]]
|
||||
self._test_sync(known_releases)
|
||||
|
||||
def test_armada_sync_with_one_deployed_release_no_diff(self):
|
||||
c1 = 'armada-test_chart_1'
|
||||
|
||||
known_releases = [[c1, None, None, "{}", const.STATUS_DEPLOYED]]
|
||||
self._test_sync(known_releases, diff=set())
|
||||
|
||||
def test_armada_sync_with_both_deployed_releases(self):
|
||||
c1 = 'armada-test_chart_1'
|
||||
c2 = 'armada-test_chart_2'
|
||||
|
||||
known_releases = [[
|
||||
c1, None,
|
||||
self._get_chart_by_name(c1), None, const.STATUS_DEPLOYED
|
||||
], [
|
||||
c2, None,
|
||||
self._get_chart_by_name(c2), None, const.STATUS_DEPLOYED
|
||||
]]
|
||||
known_releases = [[c1, None, None, "{}", const.STATUS_DEPLOYED],
|
||||
[c2, None, None, "{}", const.STATUS_DEPLOYED]]
|
||||
self._test_sync(known_releases)
|
||||
|
||||
def test_armada_sync_with_unprotected_releases(self):
|
||||
|
@ -1,3 +1,4 @@
|
||||
deepdiff==3.3.0
|
||||
gitpython
|
||||
grpcio==1.10.0
|
||||
grpcio-tools==1.10.0
|
||||
|
Loading…
Reference in New Issue
Block a user