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.

(cherry picked from commit 1463c2e4a8)

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"):
+                    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)
 

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

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

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

test/integration/targets/k8s/playbooks/roles/k8s/defaults/main.yml

@@ -0,0 +1 @@
+recreate_crd_default_merge_expectation: recreate_crd is not failed

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

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

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 "$@"