From 9a7c1f4006558226997c30af64b0f3bd0c666718 Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Fri, 16 Nov 2018 16:21:00 +0000 Subject: [PATCH] test: Add test-specific timeout option Currently, tests executed during chart deployment use the wait timeout value, `wait.timeout`. This value can be too large of a timeout value for Helm tests. This change introduces a timeout for tests, `test.timeout` that is only used as a timeout for running Helm tests for a release. Story: 2003899 Depends-On: https://review.openstack.org/618355 Change-Id: Iee746444d5aede0b84b1805eb19f59f0f03c8f9e --- armada/const.py | 1 + armada/handlers/chart_deploy.py | 15 ++----- armada/handlers/test.py | 17 ++++---- armada/schemas/armada-chart-schema.yaml | 2 + armada/tests/unit/handlers/test_test.py | 40 +++++++++++++++++++ .../operations/guide-build-armada-yaml.rst | 2 + 6 files changed, 59 insertions(+), 18 deletions(-) diff --git a/armada/const.py b/armada/const.py index 03dd16b5..4a952038 100644 --- a/armada/const.py +++ b/armada/const.py @@ -24,6 +24,7 @@ KEYWORD_RELEASE = 'release' # Armada DEFAULT_CHART_TIMEOUT = 900 +DEFAULT_TEST_TIMEOUT = 300 # Tiller DEFAULT_TILLER_TIMEOUT = 300 diff --git a/armada/handlers/chart_deploy.py b/armada/handlers/chart_deploy.py index ba31b604..139070d7 100644 --- a/armada/handlers/chart_deploy.py +++ b/armada/handlers/chart_deploy.py @@ -244,25 +244,18 @@ class ChartDeploy(object): 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_handler) + self._test_chart(release_name, test_handler) return result - def _test_chart(self, release_name, timeout, test_handler): + def _test_chart(self, release_name, test_handler): if self.dry_run: LOG.info( 'Skipping test during `dry-run`, would have tested ' - 'release=%s with timeout %ss.', release_name, timeout) + 'release=%s', release_name) return True - if timeout <= 0: - reason = ('Timeout expired before testing ' - 'release %s' % release_name) - LOG.error(reason) - raise armada_exceptions.ArmadaTimeoutException(reason) - - success = test_handler.test_release_for_success(timeout=timeout) + success = test_handler.test_release_for_success() if not success: raise tiller_exceptions.TestFailedException(release_name) diff --git a/armada/handlers/test.py b/armada/handlers/test.py index 94c9479a..2beab065 100644 --- a/armada/handlers/test.py +++ b/armada/handlers/test.py @@ -58,6 +58,8 @@ class Test(object): test_values = self.chart.get('test', None) + self.timeout = const.DEFAULT_TEST_TIMEOUT + # 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. @@ -79,6 +81,7 @@ class Test(object): # 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: @@ -90,6 +93,8 @@ class Test(object): if self.cleanup is None: test_options = test_values.get('options', {}) self.cleanup = test_options.get('cleanup', False) + + self.timeout = test_values.get('timeout', self.timeout) else: # Default cleanup value if self.cleanup is None: @@ -98,16 +103,14 @@ class Test(object): if enable_all: self.test_enabled = True - def test_release_for_success(self, timeout=const.DEFAULT_TILLER_TIMEOUT): + def test_release_for_success(self): """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 + :return: Helm test suite run result """ - LOG.info('RUNNING: %s tests', self.release_name) + LOG.info('RUNNING: %s tests with timeout=%ds', self.release_name, + self.timeout) try: self.delete_test_pods() @@ -116,7 +119,7 @@ class Test(object): self.release_name) test_suite_run = self.tiller.test_release( - self.release_name, timeout=timeout, cleanup=self.cleanup) + self.release_name, timeout=self.timeout, cleanup=self.cleanup) success = get_test_suite_run_success(test_suite_run) if success: diff --git a/armada/schemas/armada-chart-schema.yaml b/armada/schemas/armada-chart-schema.yaml index 98018f49..353eedc0 100644 --- a/armada/schemas/armada-chart-schema.yaml +++ b/armada/schemas/armada-chart-schema.yaml @@ -66,6 +66,8 @@ data: properties: enabled: type: boolean + timeout: + type: integer options: type: object properties: diff --git a/armada/tests/unit/handlers/test_test.py b/armada/tests/unit/handlers/test_test.py index 9bf45441..647366d9 100644 --- a/armada/tests/unit/handlers/test_test.py +++ b/armada/tests/unit/handlers/test_test.py @@ -14,6 +14,8 @@ import mock +from armada import const + from armada.handlers import test from armada.handlers import tiller from armada.tests.unit import base @@ -188,6 +190,16 @@ class TestHandlerTestCase(base.ArmadaTestCase): assert test_handler.test_enabled is True assert test_handler.cleanup is True + def test_deprecated_test_key_timeout(self): + """Test that the default Tiller timeout is used when tests are enabled + using the deprecated, boolean value for a chart's `test` key. + """ + mock_tiller = mock.Mock() + test_handler = test.Test( + chart={'test': True}, release_name='release', tiller=mock_tiller) + + assert test_handler.timeout == const.DEFAULT_TEST_TIMEOUT + def test_tests_disabled(self): """Test that tests are disabled by a chart's values using the `test.enabled` path. @@ -276,3 +288,31 @@ class TestHandlerTestCase(base.ArmadaTestCase): assert test_handler.test_enabled is True assert test_handler.cleanup is True + + def test_default_timeout_value(self): + """Test that the default timeout value is used if a test timeout value, + `test.timeout` is not provided. + """ + test_handler = test.Test( + chart={'test': { + 'enabled': True + }}, + release_name='release', + tiller=mock.Mock(), + cleanup=True) + + assert test_handler.timeout == const.DEFAULT_TILLER_TIMEOUT + + def test_timeout_value(self): + """Test that a chart's test timeout value, `test.timeout` overrides the + default test timeout. + """ + chart = {'test': {'enabled': True, 'timeout': 800}} + + test_handler = test.Test( + chart=chart, + release_name='release', + tiller=mock.Mock(), + cleanup=True) + + assert test_handler.timeout is chart['test']['timeout'] diff --git a/doc/source/operations/guide-build-armada-yaml.rst b/doc/source/operations/guide-build-armada-yaml.rst index 2743cb53..ae5c7ce5 100644 --- a/doc/source/operations/guide-build-armada-yaml.rst +++ b/doc/source/operations/guide-build-armada-yaml.rst @@ -174,6 +174,8 @@ Run helm tests on the chart after install/upgrade. +=============+==========+====================================================================+ | enabled | bool | whether to enable/disable helm tests for this chart (default True) | +-------------+----------+--------------------------------------------------------------------+ +| timeout | int | time (in sec) to wait for completion of Helm tests | ++-------------+----------+--------------------------------------------------------------------+ | options | object | See `Test Options`_. | +-------------+----------+--------------------------------------------------------------------+