From 1463c2e4a87597380afa64c5b05526a03b41eb8d Mon Sep 17 00:00:00 2001 From: Will Thames Date: Sat, 8 Sep 2018 10:08:09 +1000 Subject: [PATCH] Use a sensible default for k8s merge_type (#45284) * Use a sensible default for k8s merge_type The sensible default for merge_type is `['strategic-merge', 'merge']. However, we can't make this the default default, as we need to support users who are using openshift 0.6.0, where the merge_type parameter is unsupported. * Refactor k8s test suite for merge_type tests Allow tests with pre-merge-type openshift and post-merge-type openshift. --- lib/ansible/module_utils/k8s/raw.py | 15 ++-- lib/ansible/modules/clustering/k8s/k8s.py | 3 + .../targets/k8s/playbooks/full_test.yml | 8 ++ .../targets/k8s/playbooks/merge_type_fail.yml | 16 ++++ .../k8s/playbooks/roles/k8s/defaults/main.yml | 1 + .../roles/k8s}/files/crd-resource.yml | 0 .../roles/k8s}/files/setup-crd.yml | 0 .../k8s/playbooks/roles/k8s/tasks/crd.yml | 76 +++++++++++++++++++ .../{ => playbooks/roles/k8s}/tasks/main.yml | 52 +------------ test/integration/targets/k8s/runme.sh | 25 ++++++ 10 files changed, 139 insertions(+), 57 deletions(-) create mode 100644 test/integration/targets/k8s/playbooks/full_test.yml create mode 100644 test/integration/targets/k8s/playbooks/merge_type_fail.yml create mode 100644 test/integration/targets/k8s/playbooks/roles/k8s/defaults/main.yml rename test/integration/targets/k8s/{ => playbooks/roles/k8s}/files/crd-resource.yml (100%) rename test/integration/targets/k8s/{ => playbooks/roles/k8s}/files/setup-crd.yml (100%) create mode 100644 test/integration/targets/k8s/playbooks/roles/k8s/tasks/crd.yml rename test/integration/targets/k8s/{ => playbooks/roles/k8s}/tasks/main.yml (84%) create mode 100755 test/integration/targets/k8s/runme.sh diff --git a/lib/ansible/module_utils/k8s/raw.py b/lib/ansible/module_utils/k8s/raw.py index f2bfada047..7cad777bea 100644 --- a/lib/ansible/module_utils/k8s/raw.py +++ b/lib/ansible/module_utils/k8s/raw.py @@ -210,18 +210,19 @@ class KubernetesRawModule(KubernetesAnsibleModule): if self.check_mode: k8s_obj = dict_merge(existing.to_dict(), definition) else: - if self.params['merge_type']: - from distutils.version import LooseVersion - if LooseVersion(self.openshift_version) < LooseVersion("0.6.2"): + from distutils.version import LooseVersion + if LooseVersion(self.openshift_version) < LooseVersion("0.6.2"): + if self.params['merge_type']: self.fail_json(msg="openshift >= 0.6.2 is required for merge_type") - for merge_type in self.params['merge_type']: + else: + k8s_obj, error = self.patch_resource(resource, definition, existing, name, + namespace) + else: + for merge_type in self.params['merge_type'] or ['strategic-merge', 'merge']: k8s_obj, error = self.patch_resource(resource, definition, existing, name, namespace, merge_type=merge_type) if not error: break - else: - k8s_obj, error = self.patch_resource(resource, definition, existing, name, - namespace) if error: self.fail_json(**error) diff --git a/lib/ansible/modules/clustering/k8s/k8s.py b/lib/ansible/modules/clustering/k8s/k8s.py index 825c206db1..678a51dd5c 100644 --- a/lib/ansible/modules/clustering/k8s/k8s.py +++ b/lib/ansible/modules/clustering/k8s/k8s.py @@ -50,6 +50,9 @@ options: - See U(https://kubernetes.io/docs/tasks/run-application/update-api-object-kubectl-patch/#use-a-json-merge-patch-to-update-a-deployment) - Requires openshift >= 0.6.2 - If more than one merge_type is given, the merge_types will be tried in order + - If openshift >= 0.6.2, this defaults to C(['strategic-merge', 'merge']), which is ideal for using the same parameters + on resource kinds that combine Custom Resources and built-in resources. For openshift < 0.6.2, the default + is simply C(strategic-merge). choices: - json - merge diff --git a/test/integration/targets/k8s/playbooks/full_test.yml b/test/integration/targets/k8s/playbooks/full_test.yml new file mode 100644 index 0000000000..e86f39176f --- /dev/null +++ b/test/integration/targets/k8s/playbooks/full_test.yml @@ -0,0 +1,8 @@ +- hosts: localhost + connection: local + gather_facts: no + vars: + ansible_python_interpreter: "{{ ansible_playbook_python }}" + + roles: + - k8s diff --git a/test/integration/targets/k8s/playbooks/merge_type_fail.yml b/test/integration/targets/k8s/playbooks/merge_type_fail.yml new file mode 100644 index 0000000000..1876ac4c8e --- /dev/null +++ b/test/integration/targets/k8s/playbooks/merge_type_fail.yml @@ -0,0 +1,16 @@ +- hosts: localhost + connection: local + gather_facts: no + vars: + ansible_python_interpreter: "{{ ansible_playbook_python }}" + recreate_crd_default_merge_expectation: recreate_crd is failed + + tasks: + - python_requirements_facts: + dependencies: + - openshift==0.6.0 + - kubernetes==6.0.0 + + - include_role: + name: k8s + tasks_from: crd diff --git a/test/integration/targets/k8s/playbooks/roles/k8s/defaults/main.yml b/test/integration/targets/k8s/playbooks/roles/k8s/defaults/main.yml new file mode 100644 index 0000000000..13e29e6892 --- /dev/null +++ b/test/integration/targets/k8s/playbooks/roles/k8s/defaults/main.yml @@ -0,0 +1 @@ +recreate_crd_default_merge_expectation: recreate_crd is not failed diff --git a/test/integration/targets/k8s/files/crd-resource.yml b/test/integration/targets/k8s/playbooks/roles/k8s/files/crd-resource.yml similarity index 100% rename from test/integration/targets/k8s/files/crd-resource.yml rename to test/integration/targets/k8s/playbooks/roles/k8s/files/crd-resource.yml diff --git a/test/integration/targets/k8s/files/setup-crd.yml b/test/integration/targets/k8s/playbooks/roles/k8s/files/setup-crd.yml similarity index 100% rename from test/integration/targets/k8s/files/setup-crd.yml rename to test/integration/targets/k8s/playbooks/roles/k8s/files/setup-crd.yml diff --git a/test/integration/targets/k8s/playbooks/roles/k8s/tasks/crd.yml b/test/integration/targets/k8s/playbooks/roles/k8s/tasks/crd.yml new file mode 100644 index 0000000000..57dfb23ce5 --- /dev/null +++ b/test/integration/targets/k8s/playbooks/roles/k8s/tasks/crd.yml @@ -0,0 +1,76 @@ +# TODO: This is the only way I could get the kubeconfig, I don't know why. Running the lookup outside of debug seems to return an empty string +#- debug: msg={{ lookup('env', 'K8S_AUTH_KUBECONFIG') }} +# register: kubeconfig + +# Kubernetes resources + +- block: + - name: Create a namespace + k8s: + name: testing + kind: namespace + + - name: install custom resource definitions + k8s: + definition: "{{ lookup('file', role_path + '/files/setup-crd.yml') }}" + + - name: create custom resource definition + k8s: + definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" + namespace: testing + register: create_crd + + - name: patch custom resource definition + k8s: + definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" + namespace: testing + register: recreate_crd + ignore_errors: yes + + - name: assert that recreating crd is as expected + assert: + that: + - recreate_crd_default_merge_expectation + + - block: + - name: recreate custom resource definition with merge_type + k8s: + definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" + merge_type: merge + namespace: testing + register: recreate_crd_with_merge + + - name: recreate custom resource definition with merge_type list + k8s: + definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" + merge_type: + - strategic-merge + - merge + namespace: testing + register: recreate_crd_with_merge_list + when: recreate_crd is successful + + + - name: remove crd + k8s: + definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" + namespace: testing + state: absent + + always: + - name: remove crd + k8s: + definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" + namespace: testing + state: absent + ignore_errors: yes + + - name: Delete all namespaces + k8s: + state: absent + definition: + - kind: Namespace + apiVersion: v1 + metadata: + name: testing1 + ignore_errors: yes diff --git a/test/integration/targets/k8s/tasks/main.yml b/test/integration/targets/k8s/playbooks/roles/k8s/tasks/main.yml similarity index 84% rename from test/integration/targets/k8s/tasks/main.yml rename to test/integration/targets/k8s/playbooks/roles/k8s/tasks/main.yml index dcd4c82530..beb66b423b 100644 --- a/test/integration/targets/k8s/tasks/main.yml +++ b/test/integration/targets/k8s/playbooks/roles/k8s/tasks/main.yml @@ -296,50 +296,6 @@ that: item.resources[0].status.phase == 'Active' loop: "{{ k8s_namespaces.results }}" - - name: install custom resource definitions - k8s: - definition: "{{ lookup('file', role_path + '/files/setup-crd.yml') }}" - - - name: create custom resource definition - k8s: - definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" - namespace: testing4 - register: create_crd - - - name: recreate custom resource definition - k8s: - definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" - namespace: testing4 - register: recreate_crd - ignore_errors: yes - - - name: assert that recreating crd fails - assert: - that: - - recreate_crd is failed - - - name: recreate custom resource definition with merge_type - k8s: - definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" - merge_type: merge - namespace: testing4 - register: recreate_crd_with_merge - - - name: recreate custom resource definition with merge_type list - k8s: - definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" - merge_type: - - strategic-merge - - merge - namespace: testing4 - register: recreate_crd_with_merge_list - - - name: remove crd - k8s: - definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" - namespace: testing4 - state: absent - - name: Delete resources from a list k8s: state: absent @@ -368,12 +324,6 @@ loop: "{{ k8s_facts.results }}" always: - - name: remove crd - k8s: - definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}" - namespace: testing4 - state: absent - ignore_errors: yes - name: Delete all namespaces k8s: @@ -400,3 +350,5 @@ metadata: name: testing5 ignore_errors: yes + +- include_tasks: crd.yml diff --git a/test/integration/targets/k8s/runme.sh b/test/integration/targets/k8s/runme.sh new file mode 100755 index 0000000000..9807f1ffe7 --- /dev/null +++ b/test/integration/targets/k8s/runme.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash + +# We don't set -u here, due to pypa/virtualenv#150 +set -ex + +MYTMPDIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir') + +#trap 'rm -rf "${MYTMPDIR}"' EXIT + +# This is needed for the ubuntu1604py3 tests +# Ubuntu patches virtualenv to make the default python2 +# but for the python3 tests we need virtualenv to use python3 +PYTHON=${ANSIBLE_TEST_PYTHON_INTERPRETER:-python} + +# Test graceful failure for older versions of openshift +virtualenv --system-site-packages --python "${PYTHON}" "${MYTMPDIR}/openshift-0.6.0" +source "${MYTMPDIR}/openshift-0.6.0/bin/activate" +$PYTHON -m pip install 'openshift==0.6.0' 'kubernetes==6.0.0' +ansible-playbook -v playbooks/merge_type_fail.yml "$@" + +# Run full test suite +virtualenv --system-site-packages --python "${PYTHON}" "${MYTMPDIR}/openshift-recent" +source "${MYTMPDIR}/openshift-recent/bin/activate" +$PYTHON -m pip install 'openshift>=0.7.0' +ansible-playbook -v playbooks/full_test.yml "$@"