K8s retry merge (#44821)

* Make merge_type a list and apply merge_type in order

Allow use case of preferring strategic-merge and failing
back to merge, or just preferring a different merge type

* Improve k8s module test coverage

lib/ansible/module_utils/k8s/raw.py

@@ -39,7 +39,7 @@ class KubernetesRawModule(KubernetesAnsibleModule):
     def argspec(self):
         argument_spec = copy.deepcopy(COMMON_ARG_SPEC)
         argument_spec.update(copy.deepcopy(AUTH_ARG_SPEC))
-        argument_spec['merge_type'] = dict(choices=['json', 'merge', 'strategic-merge'])
+        argument_spec['merge_type'] = dict(type='list', choices=['json', 'merge', 'strategic-merge'])
         return argument_spec
 
     def __init__(self, *args, **kwargs):
@@ -210,19 +210,21 @@ class KubernetesRawModule(KubernetesAnsibleModule):
             if self.check_mode:
                 k8s_obj = dict_merge(existing.to_dict(), definition)
             else:
-                try:
-                    params = dict(name=name, namespace=namespace)
                 if self.params['merge_type']:
                     from distutils.version import LooseVersion
                     if LooseVersion(self.openshift_version) < LooseVersion("0.6.2"):
                         self.fail_json(msg="openshift >= 0.6.2 is required for merge_type")
-                        params['content_type'] = 'application/{0}-patch+json'.format(self.params['merge_type'])
+                    for merge_type in self.params['merge_type']:
-                    k8s_obj = resource.patch(definition, **params).to_dict()
+                        k8s_obj, error = self.patch_resource(resource, definition, existing, name,
-                    match, diffs = self.diff_objects(existing.to_dict(), k8s_obj)
+                                                             namespace, merge_type=merge_type)
-                    result['result'] = k8s_obj
+                        if not error:
-                except DynamicApiError as exc:
+                            break
-                    self.fail_json(msg="Failed to patch object: {0}".format(exc.body),
+                else:
-                                   error=exc.status, status=exc.status, reason=exc.reason)
+                    k8s_obj, error = self.patch_resource(resource, definition, existing, name,
+                                                         namespace)
+                if error:
+                    self.fail_json(**error)
+
             match, diffs = self.diff_objects(existing.to_dict(), k8s_obj)
             result['result'] = k8s_obj
             result['changed'] = not match
@@ -230,6 +232,20 @@ class KubernetesRawModule(KubernetesAnsibleModule):
             result['diff'] = diffs
             return result
 
+    def patch_resource(self, resource, definition, existing, name, namespace, merge_type=None):
+        try:
+            params = dict(name=name, namespace=namespace)
+            if merge_type:
+                params['content_type'] = 'application/{0}-patch+json'.format(merge_type)
+            k8s_obj = resource.patch(definition, **params).to_dict()
+            match, diffs = self.diff_objects(existing.to_dict(), k8s_obj)
+            error = {}
+            return k8s_obj, {}
+        except DynamicApiError as exc:
+            error = dict(msg="Failed to patch object: {0}".format(exc.body),
+                         error=exc.status, status=exc.status, reason=exc.reason)
+            return None, error
+
     def create_project_request(self, definition):
         definition['kind'] = 'ProjectRequest'
         result = {'changed': False, 'result': {}}

lib/ansible/modules/clustering/k8s/k8s.py

@@ -49,10 +49,12 @@ options:
       want to use C(merge) if you see "strategic merge patch format is not supported"
     - 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
     choices:
     - json
     - merge
     - strategic-merge
+    type: list
     version_added: "2.7"
 
 requirements:

test/integration/targets/k8s/files/crd-resource.yml

@@ -0,0 +1,20 @@
+apiVersion: certmanager.k8s.io/v1alpha1
+kind: Certificate
+metadata:
+  name: acme-crt
+spec:
+  secretName: acme-crt-secret
+  dnsNames:
+  - foo.example.com
+  - bar.example.com
+  acme:
+    config:
+    - ingressClass: nginx
+      domains:
+      - foo.example.com
+      - bar.example.com
+  issuerRef:
+    name: letsencrypt-prod
+    # We can reference ClusterIssuers by changing the kind here.
+    # The default value is Issuer (i.e. a locally namespaced Issuer)
+    kind: Issuer

test/integration/targets/k8s/files/setup-crd.yml

@@ -0,0 +1,14 @@
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: certificates.certmanager.k8s.io
+spec:
+  group: certmanager.k8s.io
+  version: v1alpha1
+  scope: Namespaced
+  names:
+    kind: Certificate
+    plural: certificates
+    shortNames:
+      - cert
+      - certs

test/integration/targets/k8s/tasks/main.yml

@@ -1,21 +1,21 @@
 # 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') }}
+#- debug: msg={{ lookup('env', 'K8S_AUTH_KUBECONFIG') }}
-  register: kubeconfig
+#  register: kubeconfig
 
 # Kubernetes resources
-- name: Create a namespace
+
+- block:
+    - name: Create a namespace
       k8s:
         name: testing
         kind: namespace
       register: output
 
-- debug: msg={{ lookup("k8s", kind="Namespace", api_version="v1", resource_name='testing', kubeconfig=kubeconfig.msg) }}
+    - name: show output
-
-- name: show output
       debug:
         var: output
 
-- name: Create a service
+    - name: Create a service
       k8s:
         state: present
         resource_definition: &svc
@@ -38,21 +38,21 @@
               port: 8000
       register: output
 
-- name: show output
+    - name: show output
       debug:
         var: output
 
-- name: Create the service again
+    - name: Create the service again
       k8s:
         state: present
         resource_definition: *svc
       register: output
 
-- name: Service creation should be idempotent
+    - name: Service creation should be idempotent
       assert:
         that: not output.changed
 
-- name: Create PVC
+    - name: Create PVC
       k8s:
         state: present
         inline: &pvc
@@ -68,20 +68,20 @@
             accessModes:
             - ReadWriteOnce
 
-- name: Show output
+    - name: Show output
       debug:
         var: output
 
-- name: Create the PVC again
+    - name: Create the PVC again
       k8s:
         state: present
         inline: *pvc
 
-- name: PVC creation should be idempotent
+    - name: PVC creation should be idempotent
       assert:
         that: not output.changed
 
-- name: Create deployment
+    - name: Create deployment
       k8s:
         state: present
         inline: &deployment
@@ -116,33 +116,33 @@
               type: RollingUpdate
       register: output
 
-- name: Show output
+    - name: Show output
       debug:
         var: output
 
-- name: Create deployment again
+    - name: Create deployment again
       k8s:
         state: present
         inline: *deployment
       register: output
 
-- name: Deployment creation should be idempotent
+    - name: Deployment creation should be idempotent
       assert:
         that: not output.changed
 
-# OpenShift Resources
+    # OpenShift Resources
-- name: Create a project
+    - name: Create a project
       k8s:
         name: testing
         kind: project
         api_version: v1
       register: output
 
-- name: show output
+    - name: show output
       debug:
         var: output
 
-- name: Create deployment config
+    - name: Create deployment config
       k8s:
         state: present
         inline: &dc
@@ -177,22 +177,22 @@
               type: Rolling
       register: output
 
-- name: Show output
+    - name: Show output
       debug:
         var: output
 
-- name: Create deployment config again
+    - name: Create deployment config again
       k8s:
         state: present
         inline: *dc
       register: output
 
-- name: DC creation should be idempotent
+    - name: DC creation should be idempotent
       assert:
         that: not output.changed
 
-### Type tests
+    ### Type tests
-- name: Create a namespace from a string
+    - name: Create a namespace from a string
       k8s:
         definition: |+
           ---
@@ -201,11 +201,15 @@
           metadata:
             name: testing1
 
-- name: Namespace should exist
+    - name: Namespace should exist
-  assert:
+      k8s_facts:
-    that: '{{ lookup("k8s", kind="Namespace", api_version="v1", resource_name="testing1", kubeconfig=kubeconfig.msg).status.phase == "Active" }}'
+        kind: Namespace
+        api_version: v1
+        name: testing1
+      register: k8s_facts_testing1
+      failed_when: not k8s_facts_testing1.resources or k8s_facts_testing1.resources[0].status.phase != "Active"
 
-- name: Create resources from a multidocument yaml string
+    - name: Create resources from a multidocument yaml string
       k8s:
         definition: |+
           ---
@@ -219,14 +223,22 @@
           metadata:
             name: testing3
 
-- name: Resources should exist
+    - name: Lookup namespaces
-  assert:
+      k8s_facts:
-    that: lookup("k8s", kind="Namespace", api_version="v1", resource_name=item, kubeconfig=kubeconfig.msg).status.phase == "Active"
+        api_version: v1
+        kind: Namespace
+        name: "{{ item }}"
       loop:
         - testing2
         - testing3
+      register: k8s_namespaces
 
-- name: Delete resources from a multidocument yaml string
+    - name: Resources should exist
+      assert:
+        that: item.resources[0].status.phase == 'Active'
+      loop: "{{ k8s_namespaces.results }}"
+
+    - name: Delete resources from a multidocument yaml string
       k8s:
         state: absent
         definition: |+
@@ -241,16 +253,23 @@
           metadata:
             name: testing3
 
-- name: Resources should not exist
+    - name: Lookup namespaces
-  assert:
+      k8s_facts:
-    that: not ns or ns.status.phase == "Terminating"
+        api_version: v1
+        kind: Namespace
+        name: "{{ item }}"
       loop:
         - testing2
         - testing3
-  vars:
+      register: k8s_namespaces
-    ns: '{{ lookup("k8s", kind="Namespace", api_version="v1", resource_name=item, kubeconfig=kubeconfig.msg) }}'
 
-- name: Create resources from a list
+    - name: Resources should not exist
+      assert:
+        that:
+          - not item.resources or item.resources[0].status.phase == "Terminating"
+      loop: "{{ k8s_namespaces.results }}"
+
+    - name: Create resources from a list
       k8s:
         definition:
           - kind: Namespace
@@ -262,14 +281,66 @@
             metadata:
               name: testing5
 
-- name: Resources should exist
+    - name: Lookup namespaces
-  assert:
+      k8s_facts:
-    that: lookup("k8s", kind="Namespace", api_version="v1", resource_name=item, kubeconfig=kubeconfig.msg).status.phase == "Active"
+        api_version: v1
+        kind: Namespace
+        name: "{{ item }}"
       loop:
         - testing4
         - testing5
+      register: k8s_namespaces
 
-- name: Delete resources from a list
+    - name: Resources should exist
+      assert:
+        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
         definition:
@@ -282,11 +353,50 @@
             metadata:
               name: testing5
 
-- name: Resources should not exist
+    - k8s_facts:
-  assert:
+        api_version: v1
-    that: not ns or ns.status.phase == "Terminating"
+        kind: Namespace
+        name: "{{ item }}"
       loop:
         - testing4
         - testing5
-  vars:
+      register: k8s_facts
-    ns: '{{ lookup("k8s", kind="Namespace", api_version="v1", resource_name=item, kubeconfig=kubeconfig.msg) }}'
+
+    - name: Resources are terminating if still in results
+      assert:
+        that: not item.resources or item.resources[0].status.phase == "Terminating"
+      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:
+        state: absent
+        definition:
+          - kind: Namespace
+            apiVersion: v1
+            metadata:
+              name: testing1
+          - kind: Namespace
+            apiVersion: v1
+            metadata:
+              name: testing2
+          - kind: Namespace
+            apiVersion: v1
+            metadata:
+              name: testing3
+          - kind: Namespace
+            apiVersion: v1
+            metadata:
+              name: testing4
+          - kind: Namespace
+            apiVersion: v1
+            metadata:
+              name: testing5
+      ignore_errors: yes

test/runner/requirements/constraints.txt

@@ -17,3 +17,4 @@ ntlm-auth >= 1.0.6 # message encryption support
 requests-ntlm >= 1.1.0 # message encryption support
 requests-credssp >= 0.1.0 # message encryption support
 voluptuous >= 0.11.0 # Schema recursion via Self
+openshift >= 0.6.2 # merge_type support