From 35ffd0e4318fd56490df275ec71b48ab2a227cd1 Mon Sep 17 00:00:00 2001 From: Will Thames Date: Tue, 5 May 2020 09:36:46 +1000 Subject: [PATCH] Improve k8s Deployment and Daemonset wait conditions Ensure that Deployments and Daemonsets properly await all replicas to be available. Correctly handles the subtle edge case when a service account no longer exists. Note that this will dramatically slow Daemonset updates --- README.md | 2 +- molecule/default/molecule.yml | 2 + molecule/default/tasks/apply.yml | 156 +++++++++++++++++++++++++++++++ molecule/default/vars/main.yml | 3 + plugins/module_utils/raw.py | 6 +- plugins/module_utils/scale.py | 30 +++--- 6 files changed, 182 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 65ccc2ba..2f4480f1 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,7 @@ After the version is published, verify it exists on the [Kubernetes Collection G ## More Information -For more information about Ansible's Kubernetes integration, join the `#ansible-community` channel on Freenode IRC, and browse the resources in the [Kubernetes Working Group](https://github.com/ansible/community/wiki/Kubernetes) Community wiki page. +For more information about Ansible's Kubernetes integration, join the `#ansible-kubernetes` channel on Freenode IRC, and browse the resources in the [Kubernetes Working Group](https://github.com/ansible/community/wiki/Kubernetes) Community wiki page. ## License diff --git a/molecule/default/molecule.yml b/molecule/default/molecule.yml index 90ba2e53..37f7c3bc 100644 --- a/molecule/default/molecule.yml +++ b/molecule/default/molecule.yml @@ -21,6 +21,8 @@ provisioner: ansible_python_interpreter: '{{ ansible_playbook_python }}' env: ANSIBLE_FORCE_COLOR: 'true' + options: + vvv: True scenario: name: default test_sequence: diff --git a/molecule/default/tasks/apply.yml b/molecule/default/tasks/apply.yml index f5362220..0b2de35b 100644 --- a/molecule/default/tasks/apply.yml +++ b/molecule/default/tasks/apply.yml @@ -179,6 +179,162 @@ - k8s_service_3.result.spec.ports | length == 1 - k8s_service_3.result.spec.ports[0].port == 8081 + - name: Insert new service port + k8s: + definition: + apiVersion: v1 + kind: Service + metadata: + name: apply-svc + namespace: "{{ apply_namespace }}" + spec: + selector: + app: whatever + ports: + - name: mesh + port: 8080 + targetPort: 8080 + - name: http + port: 8081 + targetPort: 8081 + apply: yes + register: k8s_service_4 + + - name: Check ports are correct + assert: + that: + - k8s_service_4 is changed + - k8s_service_4.result.spec.ports | length == 2 + - k8s_service_4.result.spec.ports[0].port == 8080 + - k8s_service_4.result.spec.ports[1].port == 8081 + + - name: Remove new service port (check mode) + k8s: + definition: + apiVersion: v1 + kind: Service + metadata: + name: apply-svc + namespace: "{{ apply_namespace }}" + spec: + selector: + app: whatever + ports: + - name: http + port: 8081 + targetPort: 8081 + apply: yes + check_mode: yes + register: k8s_service_check + + - name: Check ports are correct + assert: + that: + - k8s_service_check is changed + - k8s_service_check.result.spec.ports | length == 1 + - k8s_service_check.result.spec.ports[0].port == 8081 + + - name: Remove new service port + k8s: + definition: + apiVersion: v1 + kind: Service + metadata: + name: apply-svc + namespace: "{{ apply_namespace }}" + spec: + selector: + app: whatever + ports: + - name: http + port: 8081 + targetPort: 8081 + apply: yes + register: k8s_service_5 + + - name: Check ports are correct + assert: + that: + - k8s_service_5 is changed + - k8s_service_5.result.spec.ports | length == 1 + - k8s_service_5.result.spec.ports[0].port == 8081 + + - name: Add a serviceaccount + k8s: + definition: + apiVersion: v1 + kind: ServiceAccount + metadata: + name: apply-deploy + namespace: "{{ apply_namespace }}" + + - name: Add a deployment + k8s: + definition: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: apply-deploy + namespace: "{{ apply_namespace }}" + spec: + replicas: 1 + selector: + matchLabels: + app: "{{ k8s_pod_name }}" + template: "{{ k8s_pod_template }}" + wait: yes + apply: yes + vars: + k8s_pod_name: apply-deploy + k8s_pod_image: gcr.io/kuar-demo/kuard-amd64:v0.10.0-green + k8s_pod_service_account: apply-deploy + k8s_pod_ports: + - containerPort: 8080 + name: http + protocol: TCP + + - name: Remove the serviceaccount + k8s: + state: absent + definition: + apiVersion: v1 + kind: ServiceAccount + metadata: + name: apply-deploy + namespace: "{{ apply_namespace }}" + + - name: Update the earlier deployment + k8s: + definition: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: apply-deploy + namespace: "{{ apply_namespace }}" + spec: + replicas: 1 + selector: + matchLabels: + app: "{{ k8s_pod_name }}" + template: "{{ k8s_pod_template }}" + wait: yes + apply: yes + vars: + k8s_pod_name: apply-deploy + k8s_pod_image: gcr.io/kuar-demo/kuard-amd64:v0.10.0-purple + k8s_pod_service_account: apply-deploy + k8s_pod_ports: + - containerPort: 8080 + name: http + protocol: TCP + register: deploy_after_serviceaccount_removal + ignore_errors: yes + + - name: Ensure that updating deployment after service account removal failed + assert: + that: + - deploy_after_serviceaccount_removal is failed + always: - name: Remove namespace k8s: diff --git a/molecule/default/vars/main.yml b/molecule/default/vars/main.yml index 8368ae27..2fdaa77f 100644 --- a/molecule/default/vars/main.yml +++ b/molecule/default/vars/main.yml @@ -4,6 +4,7 @@ k8s_pod_metadata: app: "{{ k8s_pod_name }}" k8s_pod_spec: + serviceAccount: "{{ k8s_pod_service_account }}" containers: - image: "{{ k8s_pod_image }}" imagePullPolicy: Always @@ -20,6 +21,8 @@ k8s_pod_spec: memory: "100Mi" ports: "{{ k8s_pod_ports }}" +k8s_pod_service_account: default + k8s_pod_command: [] k8s_pod_ports: [] diff --git a/plugins/module_utils/raw.py b/plugins/module_utils/raw.py index de214e90..828df558 100644 --- a/plugins/module_utils/raw.py +++ b/plugins/module_utils/raw.py @@ -469,7 +469,8 @@ class KubernetesRawModule(KubernetesAnsibleModule): # Furthermore deployment.status.availableReplicas == deployment.status.replicas == None if status is empty return (deployment.status and deployment.status.replicas is not None and deployment.status.availableReplicas == deployment.status.replicas and - deployment.status.observedGeneration == deployment.metadata.generation) + deployment.status.observedGeneration == deployment.metadata.generation and + not deployment.status.unavailableReplicas) def _pod_ready(pod): return (pod.status and pod.status.containerStatuses is not None and @@ -478,7 +479,8 @@ class KubernetesRawModule(KubernetesAnsibleModule): def _daemonset_ready(daemonset): return (daemonset.status and daemonset.status.desiredNumberScheduled is not None and daemonset.status.numberReady == daemonset.status.desiredNumberScheduled and - daemonset.status.observedGeneration == daemonset.metadata.generation) + daemonset.status.observedGeneration == daemonset.metadata.generation and + not daemonset.status.unavailableReplicas) def _custom_condition(resource): if not resource.status or not resource.status.conditions: diff --git a/plugins/module_utils/scale.py b/plugins/module_utils/scale.py index 4b798bb2..1c153952 100644 --- a/plugins/module_utils/scale.py +++ b/plugins/module_utils/scale.py @@ -29,11 +29,16 @@ from ansible.module_utils.six import string_types try: import yaml - from openshift import watch from openshift.dynamic.client import ResourceInstance - from openshift.helper.exceptions import KubernetesException -except ImportError as exc: - class KubernetesException(Exception): + from openshift.dynamic.exceptions import NotFoundError +except ImportError: + pass +try: + from openshift import watch +except ImportError: + try: + from openshift.dynamic.client import watch + except ImportError: pass @@ -114,7 +119,7 @@ class KubernetesAnsibleScaleModule(KubernetesAnsibleModule): try: existing = resource.get(name=name, namespace=namespace) return_attributes['result'] = existing.to_dict() - except KubernetesException as exc: + except NotFoundError as exc: self.fail_json(msg='Failed to retrieve requested object: {0}'.format(exc), error=exc.value.get('status')) @@ -189,15 +194,12 @@ class KubernetesAnsibleScaleModule(KubernetesAnsibleModule): """ Create a stream of events for the object """ w = None stream = None - try: - w = watch.Watch() - w._api_client = self.client.client - if namespace: - stream = w.stream(resource.get, serialize=False, namespace=namespace, timeout_seconds=wait_time) - else: - stream = w.stream(resource.get, serialize=False, namespace=namespace, timeout_seconds=wait_time) - except KubernetesException: - pass + w = watch.Watch() + w._api_client = self.client.client + if namespace: + stream = w.stream(resource.get, serialize=False, namespace=namespace, timeout_seconds=wait_time) + else: + stream = w.stream(resource.get, serialize=False, namespace=namespace, timeout_seconds=wait_time) return w, stream def _read_stream(self, resource, watcher, stream, name, replicas):