airship
/
armada
Code Issues Proposed changes

Fix release name bug 

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