From 3e105e94dc017761b90f9007a8093fd9d8ac2652 Mon Sep 17 00:00:00 2001 From: Nishant Kumar Date: Wed, 23 Jan 2019 17:49:20 +0000 Subject: [PATCH] Move Tiller into Armada pod This change creates a Tiller sidecar in the Armada chart and configures Armada to use this Tiller by default for its operations. This allows Armada to communicate with this Tiller without exposing it to the rest of the cluster. This also removes `tiller_host` and `tiller_port` as API parameters as they should now just be configured using the configuration file. When the Tiller sidecar is enabled, configurations will be overridden to point to it. Otherwise Armada will rely on the Tiller pod lookup. While this will later enable the Tiller charts to be removed, they will not be in this change as there is currently no alternative in Airship to communicate with the cluster using Helm. Co-Authored-By: Michael Beaver Change-Id: Id881e379be580efd60bae400fa402ce238bfd6ef --- armada/api/__init__.py | 12 +----- .../tests/unit/api/test_armada_controller.py | 6 +-- .../unit/api/test_rollback_controller.py | 12 +----- .../tests/unit/api/test_tiller_controller.py | 40 +++--------------- charts/armada/templates/configmap-etc.yaml | 4 ++ charts/armada/templates/deployment-api.yaml | 42 +++++++++++++++++++ charts/armada/values.yaml | 21 ++++++++++ 7 files changed, 76 insertions(+), 61 deletions(-) diff --git a/armada/api/__init__.py b/armada/api/__init__.py index ab52c179..233a4b7e 100644 --- a/armada/api/__init__.py +++ b/armada/api/__init__.py @@ -112,17 +112,7 @@ class BaseResource(object): self.log_error(ctx, log.ERROR, msg) def get_tiller(self, req, resp): - dry_run = req.get_param_as_bool('dry_run') - tiller_port = req.get_param_as_int('tiller_port') or CONF.tiller_port - tiller_namespace = req.get_param( - 'tiller_namespace', - default=CONF.tiller_namespace) or CONF.tiller_namespace - - return Tiller( - tiller_host=req.get_param('tiller_host'), - tiller_port=tiller_port, - tiller_namespace=tiller_namespace, - dry_run=dry_run) + return Tiller(dry_run=req.get_param_as_bool('dry_run')) class ArmadaRequestContext(object): diff --git a/armada/tests/unit/api/test_armada_controller.py b/armada/tests/unit/api/test_armada_controller.py index 3b96782d..b409aa51 100644 --- a/armada/tests/unit/api/test_armada_controller.py +++ b/armada/tests/unit/api/test_armada_controller.py @@ -89,11 +89,7 @@ class ArmadaControllerTest(base.BaseControllerTest): }], **expected_armada_options) mock_armada.return_value.sync.assert_called() - mock_tiller.assert_called_with( - tiller_host=None, - tiller_port=44134, - tiller_namespace='kube-system', - dry_run=False) + mock_tiller.assert_called_with(dry_run=False) m_tiller.__exit__.assert_called() def test_armada_apply_no_href(self): diff --git a/armada/tests/unit/api/test_rollback_controller.py b/armada/tests/unit/api/test_rollback_controller.py index e2b46a12..eb037356 100644 --- a/armada/tests/unit/api/test_rollback_controller.py +++ b/armada/tests/unit/api/test_rollback_controller.py @@ -37,9 +37,6 @@ class RollbackReleaseControllerTest(base.BaseControllerTest): rollback_release = m_tiller.rollback_release rollback_release.return_value = None - tiller_host = 'host' - tiller_port = '8080' - tiller_namespace = 'tn' release = 'test-release' version = '2' dry_run = 'false' @@ -51,9 +48,6 @@ class RollbackReleaseControllerTest(base.BaseControllerTest): resp = self.app.simulate_post( '/api/v1.0/rollback/{}'.format(release), params={ - 'tiller_host': tiller_host, - 'tiller_port': tiller_port, - 'tiller_namespace': tiller_namespace, 'dry_run': dry_run, 'version': version, 'wait': wait, @@ -62,11 +56,7 @@ class RollbackReleaseControllerTest(base.BaseControllerTest): 'recreate_pods': recreate_pods }) - mock_tiller.assert_called_once_with( - tiller_host=tiller_host, - tiller_port=8080, - tiller_namespace=tiller_namespace, - dry_run=False) + mock_tiller.assert_called_once_with(dry_run=False) rollback_release.assert_called_once_with( release, 2, wait=True, timeout=123, force=True, recreate_pods=True) diff --git a/armada/tests/unit/api/test_tiller_controller.py b/armada/tests/unit/api/test_tiller_controller.py index a167e7eb..c31a5eb3 100644 --- a/armada/tests/unit/api/test_tiller_controller.py +++ b/armada/tests/unit/api/test_tiller_controller.py @@ -47,11 +47,7 @@ class TillerControllerTest(base.BaseControllerTest): self.assertEqual(expected, result.json) self.assertEqual('application/json', result.headers['content-type']) - mock_tiller.assert_called_once_with( - tiller_host=None, - tiller_port=44134, - tiller_namespace='kube-system', - dry_run=None) + mock_tiller.assert_called_once_with(dry_run=None) m_tiller.__exit__.assert_called() @mock.patch.object(api, 'Tiller') @@ -66,13 +62,7 @@ class TillerControllerTest(base.BaseControllerTest): m_tiller.tiller_version.return_value = 'fake_version' result = self.app.simulate_get( - '/api/v1.0/status', - params_csv=False, - params={ - 'tiller_host': 'fake_host', - 'tiller_port': '98765', - 'tiller_namespace': 'fake_ns' - }) + '/api/v1.0/status', params_csv=False, params={}) expected = { 'tiller': { 'version': 'fake_version', @@ -82,11 +72,7 @@ class TillerControllerTest(base.BaseControllerTest): self.assertEqual(expected, result.json) self.assertEqual('application/json', result.headers['content-type']) - mock_tiller.assert_called_once_with( - tiller_host='fake_host', - tiller_port=98765, - tiller_namespace='fake_ns', - dry_run=None) + mock_tiller.assert_called_once_with(dry_run=None) m_tiller.__exit__.assert_called() @mock.patch.object(api, 'Tiller') @@ -116,11 +102,7 @@ class TillerControllerTest(base.BaseControllerTest): } self.assertEqual(expected, result.json) - mock_tiller.assert_called_once_with( - tiller_host=None, - tiller_port=44134, - tiller_namespace='kube-system', - dry_run=None) + mock_tiller.assert_called_once_with(dry_run=None) m_tiller.list_releases.assert_called_once_with() m_tiller.__exit__.assert_called() @@ -143,13 +125,7 @@ class TillerControllerTest(base.BaseControllerTest): ] result = self.app.simulate_get( - '/api/v1.0/releases', - params_csv=False, - params={ - 'tiller_host': 'fake_host', - 'tiller_port': '98765', - 'tiller_namespace': 'fake_ns' - }) + '/api/v1.0/releases', params_csv=False, params={}) expected = { 'releases': { 'bar_namespace': ['foo'], @@ -158,11 +134,7 @@ class TillerControllerTest(base.BaseControllerTest): } self.assertEqual(expected, result.json) - mock_tiller.assert_called_once_with( - tiller_host='fake_host', - tiller_port=98765, - tiller_namespace='fake_ns', - dry_run=None) + mock_tiller.assert_called_once_with(dry_run=None) m_tiller.list_releases.assert_called_once_with() m_tiller.__exit__.assert_called() diff --git a/charts/armada/templates/configmap-etc.yaml b/charts/armada/templates/configmap-etc.yaml index 2736a763..b4006b34 100644 --- a/charts/armada/templates/configmap-etc.yaml +++ b/charts/armada/templates/configmap-etc.yaml @@ -42,6 +42,10 @@ limitations under the License. {{- set .Values.conf.armada.keystone_authtoken "password" $userIdentity.password | quote | trunc 0 -}} {{- end -}} +{{- if .Values.conf.tiller.enabled }} +{{- set .Values.conf.armada.DEFAULT "tiller_host" "127.0.0.1" | quote | trunc 0 -}} +{{- set .Values.conf.armada.DEFAULT "tiller_port" .Values.conf.tiller.port | quote | trunc 0 -}} +{{- end }} --- apiVersion: v1 kind: ConfigMap diff --git a/charts/armada/templates/deployment-api.yaml b/charts/armada/templates/deployment-api.yaml index 1ce46f14..23761edd 100644 --- a/charts/armada/templates/deployment-api.yaml +++ b/charts/armada/templates/deployment-api.yaml @@ -134,6 +134,48 @@ spec: mountPath: /etc/armada/policy.yaml subPath: policy.yaml readOnly: true +{{- if .Values.conf.tiller.enabled }} + - name: tiller +{{ tuple $envAll "tiller" | include "helm-toolkit.snippets.image" | indent 10 }} +{{ tuple $envAll $envAll.Values.pod.resources.tiller | include "helm-toolkit.snippets.kubernetes_resources" | indent 10 }} + env: + - name: TILLER_NAMESPACE + value: {{ .Values.conf.tiller.namespace | quote }} + - name: TILLER_HISTORY_MAX + value: {{ .Values.conf.tiller.history_max | quote }} + command: + - /tiller + - -logtostderr + - -v + - {{ .Values.conf.tiller.verbosity | quote }} +{{- if .Values.conf.tiller.trace }} + - -trace +{{- end }} + ports: + - name: tiller + containerPort: {{ .Values.conf.tiller.port }} + protocol: TCP + readinessProbe: + failureThreshold: 3 + httpGet: + path: /readiness + port: 44135 + scheme: HTTP + initialDelaySeconds: 1 + periodSeconds: 10 + successThreshold: 1 + timeoutSeconds: 1 + livenessProbe: + failureThreshold: 3 + httpGet: + path: /liveness + port: 44135 + scheme: HTTP + initialDelaySeconds: 1 + periodSeconds: 10 + successThreshold: 1 + timeoutSeconds: 1 +{{- end }} {{ if $mounts_armada_api.volumeMounts }}{{ toYaml $mounts_armada_api.volumeMounts | indent 12 }}{{ end }} volumes: - name: pod-etc-armada diff --git a/charts/armada/values.yaml b/charts/armada/values.yaml index 2f676baf..208176bf 100644 --- a/charts/armada/values.yaml +++ b/charts/armada/values.yaml @@ -31,12 +31,14 @@ images: ks_service: 'docker.io/openstackhelm/heat:newton' ks_user: 'docker.io/openstackhelm/heat:newton' image_repo_sync: docker.io/docker:17.07.0 + tiller: gcr.io/kubernetes-helm/tiller:v2.12.1 pull_policy: "IfNotPresent" local_registry: active: false exclude: - dep_check - image_repo_sync + - tiller network: api: @@ -168,6 +170,9 @@ secrets: conf: armada: + DEFAULT: {} + # When .conf.tiller.enabled is true `tiller_host` and `tiller_port` will + # be overridden by 127.0.0.1 and `.conf.tiller.port` respectively armada_api: bind_port: 8000 keystone_authtoken: @@ -195,6 +200,15 @@ conf: 'armada:validate_manifest': 'rule:admin_viewer' 'tiller:get_release': 'rule:admin_viewer' 'tiller:get_status': 'rule:admin_viewer' + tiller: + # If set to false then some form of Tiller needs to be provided + enabled: true + port: 44134 + verbosity: 5 + trace: false + namespace: kube-system + # Limit the maximum number of revisions saved per release. 0 for no limit. + history_max: 0 pod: env: @@ -260,6 +274,13 @@ pod: requests: memory: "128Mi" cpu: "100m" + tiller: + limits: + memory: "128Mi" + cpu: "100m" + requests: + memory: "128Mi" + cpu: "100m" jobs: ks_user: limits: