airship/armada
Code Issues Proposed changes

Fix release name bug

Browse Source 
The release name was being treated as multiple different values to
mean the same thing, when paired with the 'release_prefix'.  This
commit addresses the bug, changing all instances to use the
'release' value instead of 'chart_name' or others.

Note: This is an impacting change, in the sense that it will
cause more reliable behavior in Armada's Apply processing which
could have actual impact while upgrading components installed with
a previous version of Armada.  Previuosly undeleted FAILED releases
may now be deleted, and armada test and delete actions may now
run as expected where they didn't run before.

Change-Id: I9893e506274e974cdc8826b1812becf9b89a0ab6
This commit is contained in:
Marshall Margenau 2018-06-14 10:31:52 -05:00
parent 902caee084
commit 52bf21989f
11 changed files with 62 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
armada/api/controller/test.py
View File

@ -23,7 +23,7 @@ from armada.common import policy
from armada import const
from armada.handlers.tiller import Tiller
from armada.handlers.manifest import Manifest
from armada.utils.release import release_prefix
from armada.utils.release import release_prefixer
from armada.utils import validate
CONF = cfg.CONF
@ -169,8 +169,8 @@ class TestReleasesManifestController(api.BaseResource):
for group in armada_obj.get(const.KEYWORD_ARMADA).get(
const.KEYWORD_GROUPS):
for ch in group.get(const.KEYWORD_CHARTS):
release_name = release_prefix(
prefix, ch.get('chart').get('chart_name'))
release_name = release_prefixer(
prefix, ch.get('chart').get('release'))
if release_name in known_releases:
self.logger.info('RUNNING: %s tests', release_name)

2
armada/cli/apply.py
View File

@ -51,7 +51,7 @@ To override a specific value in a Manifest, run:
\b
$ armada apply examples/simple.yaml \
--set manifest:simple-armada:release_name="wordpress"
--set manifest:simple-armada:release="wordpress"
Or to override several values in a Manifest, reference a values.yaml-formatted
file:

6
armada/cli/delete.py
View File

@ -21,7 +21,7 @@ from armada.cli import CliAction
from armada import const
from armada.handlers.manifest import Manifest
from armada.handlers.tiller import Tiller
from armada.utils.release import release_prefix
from armada.utils.release import release_prefixer
CONF = cfg.CONF
@ -130,8 +130,8 @@ class DeleteChartManifest(CliAction):
for group in armada_obj.get(const.KEYWORD_ARMADA).get(
const.KEYWORD_GROUPS):
for ch in group.get(const.KEYWORD_CHARTS):
release_name = release_prefix(
prefix, ch.get('chart').get('chart_name'))
release_name = release_prefixer(
prefix, ch.get('chart').get('release'))
if release_name in known_release_names:
target_releases.append(release_name)
except yaml.YAMLError as e:

6
armada/cli/test.py
View File

@ -21,7 +21,7 @@ from armada.cli import CliAction
from armada import const
from armada.handlers.manifest import Manifest
from armada.handlers.tiller import Tiller
from armada.utils.release import release_prefix
from armada.utils.release import release_prefixer
CONF = cfg.CONF
@ -149,8 +149,8 @@ class TestChartManifest(CliAction):
for group in armada_obj.get(const.KEYWORD_ARMADA).get(
const.KEYWORD_GROUPS):
for ch in group.get(const.KEYWORD_CHARTS):
release_name = release_prefix(
prefix, ch.get('chart').get('chart_name'))
release_name = release_prefixer(
prefix, ch.get('chart').get('release'))
if release_name in known_release_names:
self.logger.info('RUNNING: %s tests', release_name)

45
armada/handlers/armada.py
View File

@ -28,7 +28,7 @@ from armada.handlers.chartbuilder import ChartBuilder
from armada.handlers.manifest import Manifest
from armada.handlers.override import Override
from armada.handlers.tiller import Tiller
from armada.utils.release import release_prefix
from armada.utils.release import release_prefixer
from armada.utils import source
from armada.utils import validate
@ -107,12 +107,12 @@ class Armada(object):
self.documents,
target_manifest=target_manifest).get_manifest()
def find_release_chart(self, known_releases, name):
def find_release_chart(self, known_releases, release_name):
'''
Find a release given a list of known_releases and a release name
'''
for chart_name, _, chart, values, _ in known_releases:
if chart_name == name:
for release, _, chart, values, _ in known_releases:
if release == release_name:
return chart, values
def pre_flight_ops(self):
@ -150,17 +150,12 @@ class Armada(object):
for release in failed_releases:
for group in manifest_data.get(const.KEYWORD_GROUPS, []):
for ch in group.get(const.KEYWORD_CHARTS, []):
ch_release_name = release_prefix(
prefix, ch.get('chart', {}).get('chart_name'))
ch_release_name = release_prefixer(
prefix, ch.get('chart', {}).get('release'))
if release[0] == ch_release_name:
if self.dry_run:
LOG.info('Skipping purge during `dry-run`, would '
'have purged failed release %s before '
'deployment,', release[0])
else:
LOG.info('Purging failed release %s '
'before deployment', release[0])
self.tiller.uninstall_release(release[0])
LOG.info('Purging failed release %s '
'before deployment.', release[0])
self.tiller.uninstall_release(release[0])
# Clone the chart sources
#
@ -282,7 +277,7 @@ class Armada(object):
wait_timeout = self.timeout
wait_labels = {}
release_name = release_prefix(prefix, release)
release_name = release_prefixer(prefix, release)
# Retrieve appropriate timeout value
if wait_timeout <= 0:
@ -473,7 +468,7 @@ class Armada(object):
self.post_flight_ops()
if self.enable_chart_cleanup:
self.tiller.chart_cleanup(
self._chart_cleanup(
prefix,
self.manifest[const.KEYWORD_ARMADA][const.KEYWORD_GROUPS])
@ -572,3 +567,21 @@ class Armada(object):
result = (len(chart_diff) > 0) or (len(values_diff) > 0)
return result
def _chart_cleanup(self, prefix, charts):
LOG.info('Processing chart cleanup to remove unspecified releases.')
valid_releases = []
for gchart in charts:
for chart in gchart.get(const.KEYWORD_CHARTS, []):
valid_releases.append(release_prefixer(
prefix, chart.get('chart', {}).get('release')))
actual_releases = [x.name for x in self.tiller.list_releases()]
release_diff = list(set(actual_releases) - set(valid_releases))
for release in release_diff:
if release.startswith(prefix):
LOG.info('Purging release %s as part of chart cleanup.',
release)
self.tiller.uninstall_release(release)

29
armada/handlers/tiller.py
View File

@ -32,7 +32,6 @@ from oslo_log import log as logging
from armada import const
from armada.exceptions import tiller_exceptions as ex
from armada.handlers.k8s import K8s
from armada.utils.release import release_prefix
from armada.utils.release import label_selectors
TILLER_VERSION = b'2.7.2'
@ -309,8 +308,10 @@ class Tiller(object):
for latest_release in self.list_releases():
try:
release = (
latest_release.name, latest_release.version,
latest_release.chart, latest_release.config.raw,
latest_release.name,
latest_release.version,
latest_release.chart,
latest_release.config.raw,
latest_release.info.status.Code.Name(
latest_release.info.status.code))
charts.append(release)
@ -568,28 +569,6 @@ class Tiller(object):
status = self.get_release_status(release)
raise ex.ReleaseException(release, status, 'Delete')
def chart_cleanup(self, prefix, charts):
'''
:params charts - list of yaml charts
:params known_release - list of releases in tiller
:result - will remove any chart that is not present in yaml
'''
valid_charts = []
for gchart in charts:
for chart in gchart.get('chart_group'):
valid_charts.append(release_prefix(
prefix, chart.get('chart').get('name')))
actual_charts = [x.name for x in self.list_releases()]
chart_diff = list(set(actual_charts) - set(valid_charts))
for chart in chart_diff:
if chart.startswith(prefix):
LOG.debug("Release: %s will be removed", chart)
self.uninstall_release(chart)
def delete_resources(self, release_name, resource_name, resource_type,
resource_labels, namespace, wait=False,
timeout=const.DEFAULT_TILLER_TIMEOUT):

8
armada/tests/unit/handlers/test_armada.py
View File

@ -18,7 +18,7 @@ import yaml
from armada import const
from armada.handlers import armada
from armada.tests.unit import base
from armada.utils.release import release_prefix
from armada.utils.release import release_prefixer
TEST_YAML = """
@ -270,14 +270,14 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
for c in charts:
chart = c['chart']
chart_name = chart['chart_name']
release = chart['release']
prefix = armada_obj.manifest['armada']['release_prefix']
release = release_prefix(prefix, chart_name)
release_name = release_prefixer(prefix, release)
# Simplified check because the actual code uses logical-or's
# multiple conditions, so this is enough.
this_chart_should_wait = chart['wait']['timeout'] > 0
if release not in [x[0] for x in known_releases]:
if release_name not in [x[0] for x in known_releases]:
expected_install_release_calls.append(
mock.call(
mock_chartbuilder().get_helm_chart(),

12
armada/tests/unit/utils/test_release.py
View File

@ -21,18 +21,18 @@ class ReleaseTestCase(unittest.TestCase):
def test_release_prefix_pass(self):
expected = 'armada-test'
prefix, chart = ('armada', 'test')
prefix, release = ('armada', 'test')
assert rel.release_prefix(prefix, chart) == expected
assert rel.release_prefixer(prefix, release) == expected
def test_release_prefix_int_string(self):
expected = 'armada-4'
prefix, chart = ('armada', 4)
prefix, release = ('armada', 4)
assert rel.release_prefix(prefix, chart) == expected
assert rel.release_prefixer(prefix, release) == expected
def test_release_prefix_int_int(self):
expected = '4-4'
prefix, chart = (4, 4)
prefix, release = (4, 4)
assert rel.release_prefix(prefix, chart) == expected
assert rel.release_prefixer(prefix, release) == expected

6
armada/utils/release.py
View File

@ -13,11 +13,11 @@
# limitations under the License.
def release_prefix(prefix, chart):
def release_prefixer(prefix, release):
'''
how to attach prefix to chart
attach prefix to release name
'''
return "{}-{}".format(prefix, chart)
return "{}-{}".format(prefix, release)
def label_selectors(labels):

2
doc/source/commands/apply.rst
View File

@ -24,7 +24,7 @@ Commands
To override a specific value in a Manifest, run:
$ armada apply examples/simple.yaml --set manifest:simple-armada:release_name="wordpress"
$ armada apply examples/simple.yaml --set manifest:simple-armada:release="wordpress"
Or to override several values in a Manifest, reference a values.yaml-
formatted file:

10
doc/source/operations/guide-build-armada-yaml.rst
View File

@ -89,7 +89,7 @@ Chart
+=================+==========+=======================================================================================+
| chart\_name | string | name for the chart |
+-----------------+----------+---------------------------------------------------------------------------------------+
| release\_name | string | name of the release |
| release | string | name of the release (Armada will prepend with ``release-prefix`` during processing) |
+-----------------+----------+---------------------------------------------------------------------------------------+
| namespace | string | namespace of your chart |
+-----------------+----------+---------------------------------------------------------------------------------------+
@ -172,7 +172,7 @@ Chart Example
name: blog-1
data:
chart_name: blog-1
release_name: blog-1
release: blog-1
namespace: default
wait:
timeout: 100
@ -232,7 +232,7 @@ Source Example
name: blog-1
data:
chart_name: blog-1
release_name: blog-1
release: blog-1
namespace: default
wait:
timeout: 100
@ -258,7 +258,7 @@ Source Example
name: blog-1
data:
chart_name: blog-1
release_name: blog-1
release: blog-1
namespace: default
wait:
timeout: 100
@ -282,7 +282,7 @@ Source Example
name: blog-1
data:
chart_name: blog-1
release_name: blog-1
release: blog-1
namespace: default
wait:
timeout: 100