From 9c73661c8bed060038707aa0ad608715a016a487 Mon Sep 17 00:00:00 2001 From: Scott Hussey Date: Wed, 22 Nov 2017 13:40:08 -0600 Subject: [PATCH] Change job deletion logic Update handler for chart pre-upgrade Jobs deletion to rely on Kubernetes propagationPolicy for deleting child Pods so that more generic labels can exist in an Armada manifest without impacting job-unrelated pods - Update K8s API integration to use propagationPolicy for job delete - Make default propagationPolicy 'Foreground' - Update documents to clarify structure of specifying pre-upgrade hooks - Fix tox file to support running unit tests behind a HTTP proxy Change-Id: I650543cfe05cc6a9661ab375e831bb425b7eeeab --- armada/handlers/k8s.py | 10 +++++++-- armada/handlers/tiller.py | 9 +++----- .../operations/guide-build-armada-yaml.rst | 22 +++++++++---------- tox.ini | 1 + 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/armada/handlers/k8s.py b/armada/handlers/k8s.py index 14920c47..f140c558 100644 --- a/armada/handlers/k8s.py +++ b/armada/handlers/k8s.py @@ -48,15 +48,21 @@ class K8s(object): self.batch_api = client.BatchV1Api() self.extension_api = client.ExtensionsV1beta1Api() - def delete_job_action(self, name, namespace="default"): + def delete_job_action(self, name, namespace="default", + propagation_policy='Foreground'): ''' :params name - name of the job :params namespace - name of pod that job + :params propagation_policy - The Kubernetes propagation_policy to apply + to the delete. Default 'Foreground' means + that child Pods to the Job will be deleted + before the Job is marked as deleted. ''' try: body = client.V1DeleteOptions() self.batch_api.delete_namespaced_job( - name=name, namespace=namespace, body=body) + name=name, namespace=namespace, body=body, + propagation_policy=propagation_policy) except ApiException as e: LOG.error("Exception when deleting a job: %s", e) diff --git a/armada/handlers/tiller.py b/armada/handlers/tiller.py index ece4f082..b8f34704 100644 --- a/armada/handlers/tiller.py +++ b/armada/handlers/tiller.py @@ -201,11 +201,6 @@ class Tiller(object): self.delete_resources( release_name, name, action_type, labels, namespace) - - # Ensure pods get deleted when job is deleted - if 'job' in action_type: - self.delete_resources( - release_name, name, 'pod', labels, namespace) except Exception: raise ex.PreUpdateJobDeleteException(name, namespace) @@ -481,12 +476,14 @@ class Tiller(object): label_selector = '' if resource_labels is not None: label_selector = label_selectors(resource_labels) + LOG.debug("Deleting resources in namespace %s matching" + "selectors %s.", namespace, label_selector) if 'job' in resource_type: - LOG.info("Deleting %s in namespace: %s", resource_name, namespace) get_jobs = self.k8s.get_namespace_job(namespace, label_selector) for jb in get_jobs.items: jb_name = jb.metadata.name + LOG.info("Deleting %s in namespace: %s", jb_name, namespace) self.k8s.delete_job_action(jb_name, namespace) diff --git a/docs/source/operations/guide-build-armada-yaml.rst b/docs/source/operations/guide-build-armada-yaml.rst index 72522d07..f673a517 100644 --- a/docs/source/operations/guide-build-armada-yaml.rst +++ b/docs/source/operations/guide-build-armada-yaml.rst @@ -130,7 +130,7 @@ Update - Actions +=============+==========+===============================================================+ | update | object | updates daemonsets in pre update actions | +-------------+----------+---------------------------------------------------------------+ -| delete | object | delete jobs in pre delete actions | +| delete | sequence | delete jobs in pre delete actions and child pods | +-------------+----------+---------------------------------------------------------------+ @@ -147,9 +147,9 @@ Update - Actions - Update/Delete +=============+==========+===============================================================+ | name | string | name of action | +-------------+----------+---------------------------------------------------------------+ -| type | string | type of kubernetes workload to execute | +| type | string | type of kubernetes workload to execute in scope for action | +-------------+----------+---------------------------------------------------------------+ -| labels | object | array of labels to query against kinds. (key: value) | +| labels | object | k:v mapping of labels to select Kubernetes resources | +-------------+----------+---------------------------------------------------------------+ .. note:: @@ -180,9 +180,9 @@ Example labels: component: blog install: - no_hook: false + no_hooks: false upgrade: - no_hook: false + no_hooks: false values: {} source: type: git @@ -204,9 +204,9 @@ Example wait: timeout: 100 install: - no_hook: false + no_hooks: false upgrade: - no_hook: false + no_hooks: false values: {} source: type: local @@ -228,9 +228,9 @@ Example wait: timeout: 100 install: - no_hook: false + no_hooks: false upgrade: - no_hook: false + no_hooks: false values: {} source: type: tar @@ -273,9 +273,9 @@ Example wait: timeout: 100 install: - no_hook: false + no_hooks: false upgrade: - no_hook: false + no_hooks: false pre: update: - name: test-daemonset diff --git a/tox.ini b/tox.ini index 6c915bc8..e02522ef 100644 --- a/tox.ini +++ b/tox.ini @@ -6,6 +6,7 @@ envlist = py35, pep8, coverage, bandit deps= -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt +passenv=HTTP_PROXY HTTPS_PROXY http_proxy https_proxy NO_PROXY no_proxy setenv= VIRTUAL_ENV={envdir} usedevelop = True