From a22d2d0bb02135cdf06b7b9ef2f9df07139e6cfe Mon Sep 17 00:00:00 2001 From: Tim Heyer Date: Mon, 26 Jun 2017 14:13:16 +0000 Subject: [PATCH] Implement native tiller timeout support -Update tiller.py and armada.py to support native tiller timeout -Update documentation with the new yaml timeout keyword -Update tiller version to 2.4.2 -Create tests for timeout ability, as well as structure for further test development -Fix gRPC message size bug --- armada/cli/apply.py | 2 +- armada/handlers/armada.py | 60 ++++-------- armada/handlers/tiller.py | 18 ++-- armada/tests/unit/handlers/test_armada.py | 86 +++++++++++++++++ armada/tests/unit/handlers/test_tiller.py | 46 +++++++++ armada/tests/unit/handlers/test_wait.py | 86 ----------------- .../operations/guide-build-armada-yaml.rst | 96 +++++++++---------- examples/openstack-helm.yaml | 6 ++ 8 files changed, 214 insertions(+), 186 deletions(-) create mode 100644 armada/tests/unit/handlers/test_armada.py create mode 100644 armada/tests/unit/handlers/test_tiller.py delete mode 100644 armada/tests/unit/handlers/test_wait.py diff --git a/armada/cli/apply.py b/armada/cli/apply.py index d3106495..e7d34a7c 100644 --- a/armada/cli/apply.py +++ b/armada/cli/apply.py @@ -52,7 +52,7 @@ class ApplyChartsCommand(cmd.Command): parser.add_argument('--wait', action='store_true', default=False, help='Wait until all charts' 'have been deployed') - parser.add_argument('--timeout', action='store', + parser.add_argument('--timeout', action='store', type=int, default=3600, help='Specifies time to wait' ' for charts to deploy') return parser diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index ca2ae082..16c454e8 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -14,8 +14,6 @@ import difflib import yaml -from threading import Event, Timer -from time import sleep from oslo_config import cfg from oslo_log import log as logging @@ -35,7 +33,6 @@ logging.register_options(CONF) logging.setup(CONF, DOMAIN) LOG = logging.getLogger(__name__) -DEFAULT_TIMEOUT = 3600 class Armada(object): ''' @@ -50,7 +47,7 @@ class Armada(object): skip_pre_flight=False, dry_run=False, wait=False, - timeout=DEFAULT_TIMEOUT): + timeout=None): ''' Initialize the Armada Engine and establish a connection to Tiller @@ -61,7 +58,7 @@ class Armada(object): self.skip_pre_flight = skip_pre_flight self.dry_run = dry_run self.wait = wait - self.timeout = float(timeout) + self.timeout = timeout self.config = yaml.load(config) self.tiller = Tiller() @@ -116,6 +113,14 @@ class Armada(object): if chart.release_name is None: continue + # retrieve appropriate timeout value if 'wait' is specified + chart_timeout = None + if self.wait: + if self.timeout: + chart_timeout = self.timeout + elif getattr(chart, 'timeout', None): + chart_timeout = chart.timeout + # initialize helm chart and request a # protoc helm chart object which will # pull the sources down and walk the @@ -170,7 +175,9 @@ class Armada(object): post_actions, disable_hooks=chart. upgrade.no_hooks, - values=yaml.safe_dump(values)) + values=yaml.safe_dump(values), + wait=self.wait, + timeout=chart_timeout) # process install else: @@ -180,18 +187,15 @@ class Armada(object): chart.release_name, chart.namespace, prefix, - values=yaml.safe_dump(values)) + values=yaml.safe_dump(values), + wait=self.wait, + timeout=chart_timeout) LOG.debug("Cleaning up chart source in %s", chartbuilder.source_directory) chartbuilder.source_cleanup() - # if requested, wait for chart deployment - if self.wait: - LOG.info("Waiting for chart deployment") - self.wait_for_deployment() - if self.enable_chart_cleanup: self.tiller.chart_cleanup(prefix, self.config['armada']['charts']) @@ -222,35 +226,3 @@ class Armada(object): LOG.debug(line) return (len(chart_diff) > 0) or (len(values_diff) > 0) - - def wait_for_deployment(self): - FAIL_STATUS = 'Failed' - RUN_STATUS = 'Running' - SUCCESS_STATUS = 'Succeeded' - - pods = self.tiller.k8s.get_all_pods().items - timeout_event = Event() - timer = Timer(self.timeout, timeout_event.set) - - try: - timer.start() - while not len(pods) == 0 and not timeout_event.is_set(): - sleep(1) - pods_copy = list(pods) - for pod in pods_copy: - if pod.status.phase == FAIL_STATUS: - timer.cancel() - raise RuntimeError('Deploy failed {}' - .format(pod.metadata.name)) - elif (pod.status.phase == RUN_STATUS or - pod.status.phase == SUCCESS_STATUS): - pods.remove(pod) - except: - timer.cancel() - pass - - if timeout_event.is_set(): - raise RuntimeError('Deploy timeout {}' - .format([pod.metadata.name for pod in (pods)])) - else: - timer.cancel() diff --git a/armada/handlers/tiller.py b/armada/handlers/tiller.py index ad45197b..494a9667 100644 --- a/armada/handlers/tiller.py +++ b/armada/handlers/tiller.py @@ -23,7 +23,7 @@ from k8s import K8s from ..utils.release import release_prefix TILLER_PORT = 44134 -TILLER_VERSION = b'2.1.3' +TILLER_VERSION = b'2.4.2' TILLER_TIMEOUT = 300 RELEASE_LIMIT = 64 @@ -32,7 +32,7 @@ RELEASE_LIMIT = 64 # but until proper paging is supported, we need # to support a larger payload as the current # limit is exhausted with just 10 releases -MAX_MESSAGE_LENGTH = 4010241024 +MAX_MESSAGE_LENGTH = 429496729 LOG = logging.getLogger(__name__) @@ -176,7 +176,8 @@ class Tiller(object): def update_release(self, chart, dry_run, name, namespace, prefix, pre_actions=None, post_actions=None, - disable_hooks=False, values=None): + disable_hooks=False, values=None, + wait=False, timeout=None): ''' Update a Helm Release ''' @@ -195,7 +196,9 @@ class Tiller(object): dry_run=dry_run, disable_hooks=disable_hooks, values=values, - name="{}-{}".format(prefix, name)) + name="{}-{}".format(prefix, name), + wait=wait, + timeout=timeout) stub.UpdateRelease(release_request, self.timeout, metadata=self.metadata) @@ -203,7 +206,7 @@ class Tiller(object): self._post_update_actions(post_actions, namespace) def install_release(self, chart, dry_run, name, namespace, prefix, - values=None): + values=None, wait=False, timeout=None): ''' Create a Helm Release ''' @@ -220,7 +223,10 @@ class Tiller(object): dry_run=dry_run, values=values, name="{}-{}".format(prefix, name), - namespace=namespace) + namespace=namespace, + wait=wait, + timeout=timeout) + return stub.InstallRelease(release_request, self.timeout, metadata=self.metadata) diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py new file mode 100644 index 00000000..d1f7082e --- /dev/null +++ b/armada/tests/unit/handlers/test_armada.py @@ -0,0 +1,86 @@ +from armada.handlers.armada import Armada + +import mock +import unittest +import yaml + +class ArmadaTestCase(unittest.TestCase): + test_yaml = """ + endpoints: &endpoints + hello-world: + this: is an example + + armada: + release_prefix: armada + charts: + - chart: + name: test_chart_1 + release_name: test_chart_1 + namespace: test + values: {} + source: + type: null + location: null + subpath: null + reference: null + dependencies: [] + timeout: 50 + + - chart: + name: test_chart_2 + release_name: test_chart_2 + namespace: test + values: {} + source: + type: null + location: null + subpath: null + reference: null + dependencies: [] + timeout: 5 + """ + + @mock.patch('armada.handlers.armada.ChartBuilder') + @mock.patch('armada.handlers.tiller.Tiller') + def test_install(self, mock_tiller, mock_chartbuilder): + '''Test install functionality from the sync() method''' + + # instantiate Armada and Tiller objects + armada = Armada('', + skip_pre_flight=True, + wait=True, + timeout=None) + armada.tiller = mock_tiller + armada.config = yaml.load(self.test_yaml) + + chart_1 = armada.config['armada']['charts'][0]['chart'] + chart_2 = armada.config['armada']['charts'][1]['chart'] + + # mock irrelevant methods called by armada.sync() + mock_tiller.list_charts.return_value = [] + mock_chartbuilder.source_clone.return_value = None + mock_chartbuilder.get_helm_chart.return_value = None + mock_chartbuilder.source_cleanup.return_value = None + + armada.sync() + + # check params that should be passed to tiller.install_release() + method_calls = [mock.call(mock_chartbuilder().get_helm_chart(), + armada.dry_run, chart_1['release_name'], + chart_1['namespace'], + armada.config['armada']['release_prefix'], + values=yaml.safe_dump(chart_1['values']), + wait=armada.wait, + timeout=chart_1['timeout']), + mock.call(mock_chartbuilder().get_helm_chart(), + armada.dry_run, chart_2['release_name'], + chart_2['namespace'], + armada.config['armada']['release_prefix'], + values=yaml.safe_dump(chart_2['values']), + wait=armada.wait, + timeout=chart_2['timeout'])] + mock_tiller.install_release.assert_has_calls(method_calls) + + def test_upgrade(self): + '''Test upgrade functionality from the sync() method''' + # TODO diff --git a/armada/tests/unit/handlers/test_tiller.py b/armada/tests/unit/handlers/test_tiller.py new file mode 100644 index 00000000..9230e949 --- /dev/null +++ b/armada/tests/unit/handlers/test_tiller.py @@ -0,0 +1,46 @@ +from armada.handlers.tiller import Tiller + +import mock +import unittest + +class TillerTestCase(unittest.TestCase): + + @mock.patch('armada.handlers.tiller.grpc') + @mock.patch('armada.handlers.tiller.Config') + @mock.patch('armada.handlers.tiller.InstallReleaseRequest') + @mock.patch('armada.handlers.tiller.ReleaseServiceStub') + def test_install_release(self, mock_stub, mock_install_request, + mock_config, mock_grpc): + # instantiate Tiller object + mock_grpc.insecure_channel.return_value = None + tiller = Tiller() + + # set params + chart = mock.Mock() + dry_run = False + name = None + namespace = None + prefix = None + initial_values = None + updated_values = mock_config(raw=initial_values) + wait = False + timeout = None + + tiller.install_release(chart, dry_run, name, namespace, prefix, + values=initial_values, wait=wait, + timeout=timeout) + + mock_stub.assert_called_with(tiller.channel) + release_request = mock_install_request( + chart=chart, + dry_run=dry_run, + values=updated_values, + name="{}-{}".format(prefix, name), + namespace=namespace, + wait=wait, + timeout=timeout + ) + (mock_stub(tiller.channel).InstallRelease + .assert_called_with(release_request, + tiller.timeout, + metadata=tiller.metadata)) diff --git a/armada/tests/unit/handlers/test_wait.py b/armada/tests/unit/handlers/test_wait.py deleted file mode 100644 index 0b2e826e..00000000 --- a/armada/tests/unit/handlers/test_wait.py +++ /dev/null @@ -1,86 +0,0 @@ -from armada.handlers.armada import Armada - -import mock -import unittest -from threading import Thread -from time import sleep - -POD_NAME_COUNTER = 1 - -class PodGenerator(): - - def gen_pod(self, phase, message=None): - global POD_NAME_COUNTER - pod = mock.Mock() - pod.status.phase = phase - pod.metadata.name = 'pod_instance_{}'.format(POD_NAME_COUNTER) - POD_NAME_COUNTER += 1 - if message: - pod.status.message = message - return pod - - -class WaitTestCase(unittest.TestCase): - - @mock.patch('armada.handlers.armada.lint') - @mock.patch('armada.handlers.tiller.Tiller') - def test_wait(self, mock_tiller, mock_lint): - TIMEOUT = 5 - # instantiate Armada object - armada = Armada("../../examples/openstack-helm.yaml", - wait=True, - timeout=TIMEOUT) - armada.tiller = mock_tiller - - # TIMEOUT TEST - timeout_pod = PodGenerator().gen_pod('Unknown') - pods = mock.Mock() - pods.items = [timeout_pod] - mock_tiller.k8s.get_all_pods.return_value = pods - - with self.assertRaises(RuntimeError): - armada.wait_for_deployment() - mock_tiller.k8s.get_all_pods.assert_called_with() - - # FAILED_STATUS TEST - failed_pod = PodGenerator().gen_pod('Failed') - pods = mock.Mock() - pods.items = [failed_pod] - mock_tiller.k8s.get_all_pods.return_value = pods - - with self.assertRaises(RuntimeError): - armada.wait_for_deployment() - mock_tiller.k8s.get_all_pods.assert_called_with() - - # SUCCESS_STATUS TEST - success_pod = PodGenerator().gen_pod('Succeeded') - pods = mock.Mock() - pods.items = [success_pod] - mock_tiller.k8s.get_all_pods.return_value = pods - - try: - armada.wait_for_deployment() - except RuntimeError as e: - self.fail('Expected success but got {}'.format(e)) - mock_tiller.k8s.get_all_pods.assert_called_with() - - # SIMULATE_DEPLOYMENT TEST - simulation_pod = PodGenerator().gen_pod('Pending') - pods = mock.Mock() - pods.items = [simulation_pod] - mock_tiller.k8s.get_all_pods.return_value = pods - - method_call = Thread(target=armada.wait_for_deployment) - method_call.start() - - # let the method spin for a bit, then change pod status - sleep(TIMEOUT / 4.0) - simulation_pod.status.phase = 'Running' - - try: - # ensure the method_call thread ends after status change - method_call.join(5.0) - self.assertFalse(method_call.is_alive()) - except RuntimeError as e: - self.fail('Expected success but got {}'.format(e)) - mock_tiller.k8s.get_all_pods.assert_called_with() diff --git a/docs/source/operations/guide-build-armada-yaml.rst b/docs/source/operations/guide-build-armada-yaml.rst index 0edff2c4..98f94659 100644 --- a/docs/source/operations/guide-build-armada-yaml.rst +++ b/docs/source/operations/guide-build-armada-yaml.rst @@ -7,36 +7,29 @@ Keywords +---------------------+--------+----------------------+ | keyword | type | action | +=====================+========+======================+ -| ``armada`` | object | will | -| | | define an | +| ``armada`` | object | define an | | | | armada | | | | release | +---------------------+--------+----------------------+ -| ``release_prefix`` | string | will tag | -| | | all | +| ``release_prefix`` | string | tag appended to the | +| | | front of all | | | | charts | | | | released | | | | by the | | | | yaml in | | | | order to | -| | | manage it | -| | | the | -| | | lifecycle | -| | | of charts | +| | | manage them | +| | | throughout their | +| | | lifecycles | +---------------------+--------+----------------------+ -| ``charts`` | array | will | -| | | store the | -| | | definitio | -| | | ns | +| ``charts`` | array | stores the | +| | | definitions | | | | of all | -| | | your | | | | charts | +---------------------+--------+----------------------+ -| ``chart`` | object | will | -| | | define | -| | | what your | +| ``chart`` | object | definition | +| | | of the | | | | chart | -| | | | +---------------------+--------+----------------------+ Defining a chart @@ -69,21 +62,23 @@ Chart +-----------------+----------+------------------------------------------------------------------------+ | keyword | type | action | +=================+==========+========================================================================+ -| name | string | will be the name fo the chart | +| name | string | name for the chart | +-----------------+----------+------------------------------------------------------------------------+ -| release\_name | string | will be the name of the release | +| release\_name | string | name of the release | +-----------------+----------+------------------------------------------------------------------------+ -| namespace | string | will place your chart in define namespace | +| namespace | string | namespace of your chart | +-----------------+----------+------------------------------------------------------------------------+ -| install | object | will install the chart into your kubernetes cluster | +| timeout | int | time (in seconds) allotted for chart to deploy when 'wait' flag is set | +-----------------+----------+------------------------------------------------------------------------+ -| update | object | will updated any chart managed by the armada yaml | +| install | object | install the chart into your Kubernetes cluster | +-----------------+----------+------------------------------------------------------------------------+ -| values | object | will override any default values in the charts | +| update | object | update the chart managed by the armada yaml | +-----------------+----------+------------------------------------------------------------------------+ -| source | object | will provide a path to a ``git repo`` or ``local dir`` deploy chart. | +| values | object | override any default values in the charts | +-----------------+----------+------------------------------------------------------------------------+ -| dependencies | object | will reference any chart deps before install | +| source | object | provide a path to a ``git repo`` or ``local dir`` deploy chart. | ++-----------------+----------+------------------------------------------------------------------------+ +| dependencies | object | reference any chart dependencies before install | +-----------------+----------+------------------------------------------------------------------------+ Source @@ -92,13 +87,13 @@ Source +-------------+----------+---------------------------------------------------------------+ | keyword | type | action | +=============+==========+===============================================================+ -| type | string | will be the source to build the charts ``git`` or ``local`` | +| type | string | source to build the chart: ``git`` or ``local`` | +-------------+----------+---------------------------------------------------------------+ -| location | string | will be the ``url`` or ``path`` to the charts | +| location | string | ``url`` or ``path`` to the chart's parent directory | +-------------+----------+---------------------------------------------------------------+ -| subpath | string | will specify the path to target chart | +| subpath | string | relative path to target chart from parent | +-------------+----------+---------------------------------------------------------------+ -| reference | string | will be the branch of the repo | +| reference | string | branch of the repo | +-------------+----------+---------------------------------------------------------------+ .. note:: @@ -111,16 +106,17 @@ Simple Example :: armada: - release_prefix: "my_armada" - charts: - - chart: &cockroach + release_prefix: "my_armada" + charts: + - chart: &cockroach name: cockroach release_name: cockroach namespace: db + timeout: 20 install: no_hooks: false values: - Replicas: 1 + Replicas: 1 source: type: git location: git://github.com/kubernetes/charts/ @@ -134,35 +130,37 @@ Multichart Example :: armada: - release_prefix: "my_armada" - charts: - - chart: &common - name: common - release_name: null - namespace: null - values: {} - source: - type: git - location: git://github.com/kubernetes/charts/ - subpath: common - reference: master - dependencies: [] + release_prefix: "my_armada" + charts: + - chart: &common + name: common + release_name: null + namespace: null + timeout: None + values: {} + source: + type: git + location: git://github.com/kubernetes/charts/ + subpath: common + reference: master + dependencies: [] - - chart: &cockroach + - chart: &cockroach name: cockroach release_name: cockroach namespace: db + timeout: 100 install: no_hooks: false values: - Replicas: 1 + Replicas: 1 source: type: git location: git://github.com/kubernetes/charts/ subpath: stable/cockroachdb reference: master dependencies: - - *common + - *common References ~~~~~~~~~~ diff --git a/examples/openstack-helm.yaml b/examples/openstack-helm.yaml index fb00ba3b..08b44f9c 100644 --- a/examples/openstack-helm.yaml +++ b/examples/openstack-helm.yaml @@ -28,6 +28,7 @@ armada: name: mariadb release_name: mariadb namespace: openstack + timeout: 50 install: no_hooks: false upgrade: @@ -55,6 +56,7 @@ armada: name: memcached release_name: memcached namespace: openstack + timeout: 10 install: no_hooks: false upgrade: @@ -79,6 +81,7 @@ armada: name: etcd release_name: etcd namespace: openstack + timeout: 10 install: no_hooks: false upgrade: @@ -103,6 +106,7 @@ armada: name: rabbitmq release_name: rabbitmq namespace: openstack + timeout: 10 install: no_hooks: false upgrade: @@ -128,6 +132,7 @@ armada: name: keystone release_name: keystone namespace: openstack + timeout: 20 install: no_hooks: false upgrade: @@ -155,6 +160,7 @@ armada: name: horizon release_name: horizon namespace: openstack + timeout: 10 install: no_hooks: false upgrade: