From adfe3ae5053c0935b1bb55de9ba92ead5b2109c1 Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Wed, 14 Nov 2018 17:05:28 +0000 Subject: [PATCH] test: Refactor test handler While authoring [0], it was discovered that Armada has duplicate logic for deciding if Helm test cleanup should be enabled as well as the tests themselves. Because of this, changes to test logic (e.g. adding pre-test actions), requires changing all traces of the repeated logic, which can lead to inconsistent behavior if not properly addressed. This change moves all test decision logic to a singular Test handler, implemented by the `Test` class. This change does NOT change the expected behavior of testing during upgrades; however, tests initiated from the API and CLI will not execute when testing a manifest if they are disabled in a chart, unless using the `--enable-all` flag. [0] https://review.openstack.org/617834 Change-Id: I1530d7637b0eb6a83f048895053a5db80d033046 --- armada/api/controller/test.py | 55 +++-- armada/cli/test.py | 59 +++-- armada/handlers/armada.py | 8 - armada/handlers/chart_deploy.py | 41 ++-- armada/handlers/test.py | 111 +++++++++- armada/tests/unit/api/test_test_controller.py | 16 +- armada/tests/unit/handlers/test_armada.py | 54 ++--- armada/tests/unit/handlers/test_test.py | 201 +++++++++++++++++- armada/utils/release.py | 4 +- doc/source/commands/test.rst | 2 + swagger/swaggerV3-api.yaml | 8 + 11 files changed, 401 insertions(+), 158 deletions(-) diff --git a/armada/api/controller/test.py b/armada/api/controller/test.py index 2201fe29..d1804c52 100644 --- a/armada/api/controller/test.py +++ b/armada/api/controller/test.py @@ -21,8 +21,8 @@ from oslo_config import cfg from armada import api from armada.common import policy from armada import const -from armada.handlers.test import test_release_for_success from armada.handlers.manifest import Manifest +from armada.handlers.test import Test from armada.utils.release import release_prefixer from armada.utils import validate @@ -36,14 +36,11 @@ class TestReleasesReleaseNameController(api.BaseResource): @policy.enforce('armada:test_release') def on_get(self, req, resp, release): - self.logger.info('RUNNING: %s', release) with self.get_tiller(req, resp) as tiller: - cleanup = req.get_param_as_bool('cleanup') - if cleanup is None: - cleanup = False - success = test_release_for_success( - tiller, release, cleanup=cleanup) + + test_handler = Test(release, tiller, cleanup=cleanup) + success = test_handler.test_release_for_success() if success: msg = { @@ -56,8 +53,6 @@ class TestReleasesReleaseNameController(api.BaseResource): 'message': 'MESSAGE: Test Fail' } - self.logger.info(msg) - resp.body = json.dumps(msg) resp.status = falcon.HTTP_200 resp.content_type = 'application/json' @@ -135,29 +130,29 @@ class TestReleasesManifestController(api.BaseResource): const.KEYWORD_GROUPS): for ch in group.get(const.KEYWORD_CHARTS): chart = ch['chart'] + release_name = release_prefixer(prefix, chart.get('release')) - cleanup = req.get_param_as_bool('cleanup') - if cleanup is None: - test_chart_override = chart.get('test', {}) - if isinstance(test_chart_override, bool): - self.logger.warn( - 'Boolean value for chart `test` key is deprecated ' - 'and will be removed. Use `test.enabled` instead.') - # Use old default value. - cleanup = True - else: - cleanup = test_chart_override.get('options', {}).get( - 'cleanup', False) if release_name in known_releases: - self.logger.info('RUNNING: %s tests', release_name) - success = test_release_for_success( - tiller, release_name, cleanup=cleanup) - if success: - self.logger.info("PASSED: %s", release_name) - message['test']['passed'].append(release_name) - else: - self.logger.info("FAILED: %s", release_name) - message['test']['failed'].append(release_name) + cleanup = req.get_param_as_bool('cleanup') + enable_all = req.get_param_as_bool('enable_all') + cg_test_charts = group.get('test_charts') + test_values = chart.get('test', {}) + + test_handler = Test( + release_name, + tiller, + cg_test_charts=cg_test_charts, + cleanup=cleanup, + enable_all=enable_all, + test_values=test_values) + + if test_handler.test_enabled: + success = test_handler.test_release_for_success() + + if success: + message['test']['passed'].append(release_name) + else: + message['test']['failed'].append(release_name) else: self.logger.info('Release %s not found - SKIPPING', release_name) diff --git a/armada/cli/test.py b/armada/cli/test.py index f3636f71..998426a2 100644 --- a/armada/cli/test.py +++ b/armada/cli/test.py @@ -19,8 +19,8 @@ from oslo_config import cfg from armada.cli import CliAction from armada import const -from armada.handlers.test import test_release_for_success from armada.handlers.manifest import Manifest +from armada.handlers.test import Test from armada.handlers.tiller import Tiller from armada.utils.release import release_prefixer @@ -79,20 +79,26 @@ SHORT_DESC = "Command tests releases." help=("Delete test pods upon completion."), is_flag=True, default=None) +@click.option( + '--enable-all', + help=("Run all tests for all releases regardless of any disabled chart " + "tests."), + is_flag=True, + default=False) @click.option('--debug', help="Enable debug logging.", is_flag=True) @click.pass_context def test_charts(ctx, file, release, tiller_host, tiller_port, tiller_namespace, - target_manifest, cleanup, debug): + target_manifest, cleanup, enable_all, debug): CONF.debug = debug TestChartManifest(ctx, file, release, tiller_host, tiller_port, - tiller_namespace, target_manifest, - cleanup).safe_invoke() + tiller_namespace, target_manifest, cleanup, + enable_all).safe_invoke() class TestChartManifest(CliAction): def __init__(self, ctx, file, release, tiller_host, tiller_port, - tiller_namespace, target_manifest, cleanup): + tiller_namespace, target_manifest, cleanup, enable_all): super(TestChartManifest, self).__init__() self.ctx = ctx @@ -103,6 +109,7 @@ class TestChartManifest(CliAction): self.tiller_namespace = tiller_namespace self.target_manifest = target_manifest self.cleanup = cleanup + self.enable_all = enable_all def invoke(self): with Tiller( @@ -117,13 +124,8 @@ class TestChartManifest(CliAction): if self.release: if not self.ctx.obj.get('api', False): - self.logger.info("RUNNING: %s tests", self.release) - success = test_release_for_success( - tiller, self.release, cleanup=self.cleanup) - if success: - self.logger.info("PASSED: %s", self.release) - else: - self.logger.info("FAILED: %s", self.release) + test_handler = Test(self.release, tiller, cleanup=self.cleanup) + test_handler.test_release_for_success() else: client = self.ctx.obj.get('CLIENT') query = { @@ -150,32 +152,21 @@ class TestChartManifest(CliAction): const.KEYWORD_GROUPS): for ch in group.get(const.KEYWORD_CHARTS): chart = ch['chart'] + release_name = release_prefixer( prefix, chart.get('release')) - if release_name in known_release_names: - cleanup = self.cleanup - if cleanup is None: - test_chart_override = chart.get('test', {}) - if isinstance(test_chart_override, bool): - self.logger.warn( - 'Boolean value for chart `test` key is' - ' deprecated and support for this will' - ' be removed. Use `test.enabled` ' - 'instead.') - # Use old default value. - cleanup = True - else: - cleanup = test_chart_override.get( - 'options', {}).get('cleanup', False) - self.logger.info('RUNNING: %s tests', release_name) - success = test_release_for_success( - tiller, release_name, cleanup=cleanup) - if success: - self.logger.info("PASSED: %s", release_name) - else: - self.logger.info("FAILED: %s", release_name) + test_values = chart.get('test', {}) + test_handler = Test( + release_name, + tiller, + cleanup=self.cleanup, + enable_all=self.enable_all, + test_values=test_values) + + if test_handler.test_enabled: + test_handler.test_release_for_success() else: self.logger.info('Release %s not found - SKIPPING', release_name) diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index 0fb50632..0bf502ce 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -200,14 +200,6 @@ class Armada(object): # TODO(MarshM): Deprecate the `test_charts` key cg_test_all_charts = chartgroup.get('test_charts') - if isinstance(cg_test_all_charts, bool): - LOG.warn('The ChartGroup `test_charts` key is deprecated, ' - 'and support for this will be removed. See the ' - 'Chart `test` key for more information.') - else: - # This key defaults to True. Individual charts must - # explicitly disable helm tests if they choose - cg_test_all_charts = True cg_charts = chartgroup.get(const.KEYWORD_CHARTS, []) charts = map(lambda x: x.get('chart', {}), cg_charts) diff --git a/armada/handlers/chart_deploy.py b/armada/handlers/chart_deploy.py index ab6965d6..30671fda 100644 --- a/armada/handlers/chart_deploy.py +++ b/armada/handlers/chart_deploy.py @@ -19,8 +19,8 @@ import yaml from armada import const from armada.exceptions import armada_exceptions from armada.handlers.chartbuilder import ChartBuilder -from armada.handlers.test import test_release_for_success from armada.handlers.release_diff import ReleaseDiff +from armada.handlers.test import Test from armada.handlers.wait import ChartWait from armada.exceptions import tiller_exceptions import armada.utils.release as r @@ -202,34 +202,25 @@ class ChartDeploy(object): chart_wait.wait(timer) # Test - test_chart_override = chart.get('test') - # Use old default value when not using newer `test` key - test_cleanup = True - if test_chart_override is None: - test_enabled = cg_test_all_charts - elif isinstance(test_chart_override, bool): - LOG.warn('Boolean value for chart `test` key is' - ' deprecated and support for this will' - ' be removed. Use `test.enabled` ' - 'instead.') - test_enabled = test_chart_override - else: - # NOTE: helm tests are enabled by default - test_enabled = test_chart_override.get('enabled', True) - test_cleanup = test_chart_override.get('options', {}).get( - 'cleanup', False) - just_deployed = ('install' in result) or ('upgrade' in result) last_test_passed = old_release and r.get_last_test_result(old_release) - run_test = test_enabled and (just_deployed or not last_test_passed) + test_values = chart.get('test') + test_handler = Test( + release_name, + self.tiller, + cg_test_charts=cg_test_all_charts, + test_values=test_values) + + run_test = test_handler.test_enabled and (just_deployed or + not last_test_passed) if run_test: timer = int(round(deadline - time.time())) - self._test_chart(release_name, timer, test_cleanup) + self._test_chart(release_name, timer, test_handler) return result - def _test_chart(self, release_name, timeout, cleanup): + def _test_chart(self, release_name, timeout, test_handler): if self.dry_run: LOG.info( 'Skipping test during `dry-run`, would have tested ' @@ -242,12 +233,8 @@ class ChartDeploy(object): LOG.error(reason) raise armada_exceptions.ArmadaTimeoutException(reason) - success = test_release_for_success( - self.tiller, release_name, timeout=timeout, cleanup=cleanup) - if success: - LOG.info("Test passed for release: %s", release_name) - else: - LOG.info("Test failed for release: %s", release_name) + success = test_handler.test_release_for_success(timeout=timeout) + if not success: raise tiller_exceptions.TestFailedException(release_name) def get_diff(self, old_chart, old_values, new_chart, values): diff --git a/armada/handlers/test.py b/armada/handlers/test.py index a463682f..aa7fe032 100644 --- a/armada/handlers/test.py +++ b/armada/handlers/test.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +from oslo_log import log as logging + from armada import const TESTRUN_STATUS_UNKNOWN = 0 @@ -19,16 +21,103 @@ TESTRUN_STATUS_SUCCESS = 1 TESTRUN_STATUS_FAILURE = 2 TESTRUN_STATUS_RUNNING = 3 - -def test_release_for_success(tiller, - release, - timeout=const.DEFAULT_TILLER_TIMEOUT, - cleanup=False): - test_suite_run = tiller.test_release( - release, timeout=timeout, cleanup=cleanup) - return get_test_suite_run_success(test_suite_run) +LOG = logging.getLogger(__name__) -def get_test_suite_run_success(test_suite_run): - return all( - r.status == TESTRUN_STATUS_SUCCESS for r in test_suite_run.results) +class Test(object): + + def __init__(self, + release_name, + tiller, + cg_test_charts=None, + cleanup=None, + enable_all=False, + test_values=None): + """Initialize a test handler to run Helm tests corresponding to a + release. + + :param release_name: Name of a Helm release + :param tiller: Tiller object + :param cg_test_charts: Chart group `test_charts` key + :param cleanup: Triggers cleanup; overrides `test.options.cleanup` + :param enable_all: Run tests regardless of the value of `test.enabled` + :param test_values: Test values retrieved from a chart's `test` key + + :type release_name: str + :type tiller: Tiller object + :type cg_test_charts: bool + :type cleanup: bool + :type enable_all: bool + :type test_values: dict or bool (deprecated) + """ + + self.release_name = release_name + self.tiller = tiller + self.cleanup = cleanup + + # NOTE(drewwalters96): Support the chart_group `test_charts` key until + # its deprecation period ends. The `test.enabled`, `enable_all` flag, + # and deprecated, boolean `test` key override this value if provided. + if cg_test_charts is not None: + LOG.warn('Chart group key `test_charts` is deprecated and will be ' + 'removed. Use `test.enabled` instead.') + self.test_enabled = cg_test_charts + else: + self.test_enabled = True + + # NOTE: Support old, boolean `test` key until deprecation period ends. + if (type(test_values) == bool): + LOG.warn('Boolean value for chart `test` key is deprecated and ' + 'will be removed. Use `test.enabled` instead.') + + self.test_enabled = test_values + + # NOTE: Use old, default cleanup value (i.e. True) if none is + # provided. + if self.cleanup is None: + self.cleanup = True + elif test_values: + test_enabled_opt = test_values.get('enabled') + if test_enabled_opt is not None: + self.test_enabled = test_enabled_opt + + # NOTE(drewwalters96): `self.cleanup`, the cleanup value provided + # by the API/CLI, takes precedence over the chart value + # `test.cleanup`. + if self.cleanup is None: + test_options = test_values.get('options', {}) + self.cleanup = test_options.get('cleanup', False) + else: + # Default cleanup value + if self.cleanup is None: + self.cleanup = False + + if enable_all: + self.test_enabled = True + + def test_release_for_success(self, timeout=const.DEFAULT_TILLER_TIMEOUT): + """Run the Helm tests corresponding to a release for success (i.e. exit + code 0). + + :param timeout: Timeout value for a release's tests completion + :type timeout: int + + :rtype: Helm test suite run result + """ + LOG.info('RUNNING: %s tests', self.release_name) + + test_suite_run = self.tiller.test_release( + self.release_name, timeout=timeout, cleanup=self.cleanup) + + success = self.get_test_suite_run_success(test_suite_run) + if success: + LOG.info('PASSED: %s', self.release_name) + else: + LOG.info('FAILED: %s', self.release_name) + + return success + + @classmethod + def get_test_suite_run_success(self, test_suite_run): + return all( + r.status == TESTRUN_STATUS_SUCCESS for r in test_suite_run.results) diff --git a/armada/tests/unit/api/test_test_controller.py b/armada/tests/unit/api/test_test_controller.py index 9c8d7973..d5f7fa49 100644 --- a/armada/tests/unit/api/test_test_controller.py +++ b/armada/tests/unit/api/test_test_controller.py @@ -59,7 +59,7 @@ class TestReleasesManifestControllerTest(base.BaseControllerTest): class TestReleasesReleaseNameControllerTest(base.BaseControllerTest): - @mock.patch.object(test, 'test_release_for_success') + @mock.patch.object(test.Test, 'test_release_for_success') @mock.patch.object(api, 'Tiller') def test_test_controller_test_pass(self, mock_tiller, mock_test_release_for_success): @@ -73,14 +73,13 @@ class TestReleasesReleaseNameControllerTest(base.BaseControllerTest): release = 'fake-release' resp = self.app.simulate_get('/api/v1.0/test/{}'.format(release)) - mock_test_release_for_success.assert_has_calls( - [mock.call(mock_tiller.return_value, release, cleanup=False)]) + mock_test_release_for_success.assert_called_once() self.assertEqual(200, resp.status_code) self.assertEqual('MESSAGE: Test Pass', json.loads(resp.text)['message']) m_tiller.__exit__.assert_called() - @mock.patch.object(test, 'test_release_for_success') + @mock.patch.object(test.Test, 'test_release_for_success') @mock.patch.object(api, 'Tiller') def test_test_controller_test_fail(self, mock_tiller, mock_test_release_for_success): @@ -98,7 +97,7 @@ class TestReleasesReleaseNameControllerTest(base.BaseControllerTest): json.loads(resp.text)['message']) m_tiller.__exit__.assert_called() - @mock.patch.object(test, 'test_release_for_success') + @mock.patch.object(test.Test, 'test_release_for_success') @mock.patch.object(api, 'Tiller') def test_test_controller_cleanup(self, mock_tiller, mock_test_release_for_success): @@ -112,8 +111,7 @@ class TestReleasesReleaseNameControllerTest(base.BaseControllerTest): release = 'fake-release' resp = self.app.simulate_get( '/api/v1.0/test/{}'.format(release), query_string='cleanup=true') - mock_test_release_for_success.assert_has_calls( - [mock.call(m_tiller, release, cleanup=True)]) + mock_test_release_for_success.assert_called_once() self.assertEqual(200, resp.status_code) self.assertEqual('MESSAGE: Test Pass', json.loads(resp.text)['message']) @@ -125,7 +123,7 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): @mock.patch.object(test, 'Manifest') @mock.patch.object(api, 'Tiller') - @mock.patch.object(test, 'test_release_for_success') + @mock.patch.object(test.Test, 'test_release_for_success') def test_test_controller_tiller_exc_returns_500( self, mock_test_release_for_success, mock_tiller, _): rules = {'armada:test_manifest': '@'} @@ -227,7 +225,7 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): class TestReleasesReleaseNameControllerNegativeTest(base.BaseControllerTest): @mock.patch.object(api, 'Tiller') - @mock.patch.object(test, 'test_release_for_success') + @mock.patch.object(test.Test, 'test_release_for_success') def test_test_controller_tiller_exc_returns_500( self, mock_test_release_for_success, mock_tiller): rules = {'armada:test_release': '@'} diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index ab1876c7..06321da0 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -17,7 +17,6 @@ import yaml from armada import const from armada.handlers import armada -from armada.handlers import chart_deploy from armada.handlers.test import TESTRUN_STATUS_SUCCESS, TESTRUN_STATUS_FAILURE from armada.tests.unit import base from armada.tests.test_utils import AttrDict, makeMockThreadSafe @@ -337,9 +336,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): @mock.patch.object(armada.Armada, 'post_flight_ops') @mock.patch.object(armada.Armada, 'pre_flight_ops') @mock.patch('armada.handlers.chart_deploy.ChartBuilder') - @mock.patch.object(chart_deploy, 'test_release_for_success') - def _do_test(mock_test_release_for_success, mock_chartbuilder, - mock_pre_flight, mock_post_flight): + @mock.patch('armada.handlers.chart_deploy.Test') + def _do_test(mock_test, mock_chartbuilder, mock_pre_flight, + mock_post_flight): # Instantiate Armada object. yaml_documents = list(yaml.safe_load_all(TEST_YAML)) @@ -351,8 +350,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): chart_group = armada_obj.manifest['armada']['chart_groups'][0] charts = chart_group['chart_group'] - cg_test_all_charts = chart_group.get('test_charts', True) + cg_test_all_charts = chart_group.get('test_charts') + mock_test_release = mock_test.return_value.test_release_for_success if test_failure_to_run: def fail(tiller, release, timeout=None, cleanup=False): @@ -361,9 +361,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): raise tiller_exceptions.ReleaseException( release, status, 'Test') - mock_test_release_for_success.side_effect = fail + mock_test_release.side_effect = fail else: - mock_test_release_for_success.return_value = test_success + mock_test_release.return_value = test_success # Stub out irrelevant methods called by `armada.sync()`. mock_chartbuilder.get_source_path.return_value = None @@ -377,7 +377,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): expected_install_release_calls = [] expected_update_release_calls = [] expected_uninstall_release_calls = [] - expected_test_release_for_success_calls = [] + expected_test_constructor_calls = [] for c in charts: chart = c['chart'] @@ -389,7 +389,6 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): native_wait_enabled = (chart['wait'].get('native', {}).get( 'enabled', True)) - expected_apply = True if release_name not in [x.name for x in known_releases]: expected_install_release_calls.append( mock.call( @@ -435,9 +434,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): break if status == const.STATUS_DEPLOYED: - if not diff: - expected_apply = False - else: + if diff: upgrade = chart.get('upgrade', {}) disable_hooks = upgrade.get('no_hooks', False) force = upgrade.get('force', False) @@ -462,25 +459,12 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): timeout=mock.ANY)) test_chart_override = chart.get('test') - # Use old default value when not using newer `test` key - test_cleanup = True - if test_chart_override is None: - test_this_chart = cg_test_all_charts - elif isinstance(test_chart_override, bool): - test_this_chart = test_chart_override - else: - test_this_chart = test_chart_override.get('enabled', True) - test_cleanup = test_chart_override.get('options', {}).get( - 'cleanup', False) - - if test_this_chart and (expected_apply or - not expected_last_test_result): - expected_test_release_for_success_calls.append( - mock.call( - m_tiller, - release_name, - timeout=mock.ANY, - cleanup=test_cleanup)) + expected_test_constructor_calls.append( + mock.call( + release_name, + m_tiller, + cg_test_charts=cg_test_all_charts, + test_values=test_chart_override)) any_order = not chart_group['sequenced'] # Verify that at least 1 release is either installed or updated. @@ -511,10 +495,10 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): # Verify that the expected number of deployed releases are # tested with expected arguments. self.assertEqual( - len(expected_test_release_for_success_calls), - mock_test_release_for_success.call_count) - mock_test_release_for_success.assert_has_calls( - expected_test_release_for_success_calls, any_order=any_order) + len(expected_test_constructor_calls), mock_test.call_count) + + mock_test.assert_has_calls( + expected_test_constructor_calls, any_order=True) _do_test() diff --git a/armada/tests/unit/handlers/test_test.py b/armada/tests/unit/handlers/test_test.py index 93afda5e..6459ed36 100644 --- a/armada/tests/unit/handlers/test_test.py +++ b/armada/tests/unit/handlers/test_test.py @@ -14,8 +14,8 @@ import mock -from armada.handlers import tiller from armada.handlers import test +from armada.handlers import tiller from armada.tests.unit import base from armada.tests.test_utils import AttrDict @@ -32,7 +32,9 @@ class TestHandlerTestCase(base.ArmadaTestCase): tiller_obj.test_release = mock.Mock() tiller_obj.test_release.return_value = AttrDict( **{'results': results}) - success = test.test_release_for_success(tiller_obj, release) + + test_handler = test.Test(release, tiller_obj) + success = test_handler.test_release_for_success() self.assertEqual(expected_success, success) @@ -62,3 +64,198 @@ class TestHandlerTestCase(base.ArmadaTestCase): AttrDict(**{'status': test.TESTRUN_STATUS_SUCCESS}), AttrDict(**{'status': test.TESTRUN_STATUS_RUNNING}) ]) + + def test_cg_disabled(self): + """Test that tests are disabled when a chart group disables all + tests. + """ + test_handler = test.Test( + release_name='release', tiller=mock.Mock(), cg_test_charts=False) + + assert test_handler.test_enabled is False + + def test_cg_disabled_test_key_enabled(self): + """Test that tests are enabled when a chart group disables all + tests and the deprecated, boolean `test` key is enabled. + """ + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + cg_test_charts=False, + test_values=True) + + assert test_handler.test_enabled is True + + def test_cg_disabled_test_values_enabled(self): + """Test that tests are enabled when a chart group disables all + tests and the `test.enabled` key is False. + """ + test_values = {'enabled': True} + + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + cg_test_charts=False, + test_values=test_values) + + assert test_handler.test_enabled is True + + def test_cg_enabled_test_key_disabled(self): + """Test that tests are disabled when a chart group enables all + tests and the deprecated, boolean `test` key is disabled. + """ + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + cg_test_charts=True, + test_values=False) + + assert test_handler.test_enabled is False + + def test_cg_enabled_test_values_disabled(self): + """Test that tests are disabled when a chart group enables all + tests and the deprecated, boolean `test` key is disabled. + """ + test_values = {'enabled': False} + + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + cg_test_charts=True, + test_values=test_values) + + assert test_handler.test_enabled is False + + def test_enable_all_cg_disabled(self): + """Test that tests are enabled when the `enable_all` parameter is + True and the chart group `test_enabled` key is disabled. + """ + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + cg_test_charts=False, + enable_all=True) + + assert test_handler.test_enabled is True + + def test_enable_all_test_key_disabled(self): + """Test that tests are enabled when the `enable_all` parameter is + True and the deprecated, boolean `test` key is disabled. + """ + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + enable_all=True, + test_values=False) + + assert test_handler.test_enabled is True + + def test_enable_all_test_values_disabled(self): + """Test that tests are enabled when the `enable_all` parameter is + True and the `test.enabled` key is False. + """ + test_values = {'enabled': False} + + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + enable_all=True, + test_values=test_values) + + assert test_handler.test_enabled is True + + def test_deprecated_test_key_false(self): + """Test that tests can be disabled using the deprecated, boolean value + for a chart's test key. + """ + test_handler = test.Test( + release_name='release', tiller=mock.Mock(), test_values=True) + + assert test_handler.test_enabled + + def test_deprecated_test_key_true(self): + """Test that cleanup is enabled by default when tests are enabled using + the deprecated, boolean value for a chart's `test` key. + """ + test_handler = test.Test( + release_name='release', tiller=mock.Mock(), test_values=True) + + assert test_handler.test_enabled is True + assert test_handler.cleanup is True + + def test_tests_disabled(self): + """Test that tests are disabled by a chart's values using the + `test.enabled` path. + """ + test_values = {'enabled': False} + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + test_values=test_values) + + assert test_handler.test_enabled is False + + def test_tests_enabled(self): + """Test that cleanup is disabled (by default) when tests are enabled by + a chart's values using the `test.enabled` path. + """ + test_values = {'enabled': True} + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + test_values=test_values) + + assert test_handler.test_enabled is True + assert test_handler.cleanup is False + + def test_tests_enabled_cleanup_enabled(self): + """Test that the test handler uses the values provided by a chart's + `test` key. + """ + test_values = {'enabled': True, 'options': {'cleanup': True}} + + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + test_values=test_values) + + assert test_handler.test_enabled is True + assert test_handler.cleanup is True + + def test_tests_enabled_cleanup_disabled(self): + """Test that the test handler uses the values provided by a chart's + `test` key. + """ + test_values = {'enabled': True, 'options': {'cleanup': False}} + + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + test_values=test_values) + + assert test_handler.test_enabled is True + assert test_handler.cleanup is False + + def test_no_test_values(self): + """Test that the default values are enforced when no chart `test` + values are provided (i.e. tests are enabled and cleanup is disabled). + """ + test_handler = test.Test(release_name='release', tiller=mock.Mock()) + + assert test_handler.test_enabled is True + assert test_handler.cleanup is False + + def test_override_cleanup(self): + """Test that a cleanup value passed to the Test handler (i.e. from the + API/CLI) takes precedence over a chart's `test.cleanup` value. + """ + test_values = {'enabled': True, 'options': {'cleanup': False}} + + test_handler = test.Test( + release_name='release', + tiller=mock.Mock(), + cleanup=True, + test_values=test_values) + + assert test_handler.test_enabled is True + assert test_handler.cleanup is True diff --git a/armada/utils/release.py b/armada/utils/release.py index dfd02df2..dbd51d9b 100644 --- a/armada/utils/release.py +++ b/armada/utils/release.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from armada.handlers.test import get_test_suite_run_success +from armada.handlers.test import Test def release_prefixer(prefix, release): @@ -52,4 +52,4 @@ def get_last_test_result(release): status = release.info.status if not status.HasField('last_test_suite_run'): return None - return get_test_suite_run_success(status.last_test_suite_run) + return Test.get_test_suite_run_success(status.last_test_suite_run) diff --git a/doc/source/commands/test.rst b/doc/source/commands/test.rst index 4892dc95..d305d986 100644 --- a/doc/source/commands/test.rst +++ b/doc/source/commands/test.rst @@ -24,6 +24,8 @@ Commands $ armada test --release blog-1 Options: + --cleanup Delete test pods after test completion + --enable-all Run disabled chart tests --file TEXT armada manifest --release TEXT helm release --tiller-host TEXT Tiller Host IP diff --git a/swagger/swaggerV3-api.yaml b/swagger/swaggerV3-api.yaml index 8308b878..131ac6aa 100644 --- a/swagger/swaggerV3-api.yaml +++ b/swagger/swaggerV3-api.yaml @@ -155,6 +155,7 @@ paths: - $ref: "#/components/parameters/tiller-port" - $ref: "#/components/parameters/tiller-namespace" - $ref: "#/components/parameters/target-manifest" + - $ref: "#/components/parameters/enable-all" requestBody: $ref: "#/components/requestBodies/manifest-body" responses: @@ -337,6 +338,13 @@ components: description: Flag to allow for chart cleanup schema: type: boolean + enable-all: + in: query + name: enable_all + required: false + description: Flag to test disabled tests + schema: + type: boolean dry-run: in: query name: dry_run