From 446919de6f5fbbcff8706d7eff147595140547fb Mon Sep 17 00:00:00 2001
From: Will Thames <will@thames.id.au>
Date: Tue, 9 Jul 2019 06:47:41 +1000
Subject: [PATCH] Add apply to k8s module (#49053)

* Add apply to k8s module

Use apply method for updating k8s resources.

* Improve apply documentation

* k8s: Make apply and merge_type mutually exclusive
---
 lib/ansible/config/module_defaults.yml        |   2 +
 lib/ansible/module_utils/k8s/raw.py           |  58 ++++--
 lib/ansible/modules/clustering/k8s/k8s.py     |  10 ++
 test/integration/targets/k8s/tasks/apply.yml  | 170 ++++++++++++++++++
 test/integration/targets/k8s/tasks/crd.yml    |   7 +-
 test/integration/targets/k8s/tasks/delete.yml |   8 +-
 .../targets/k8s/tasks/full_test.yml           |   5 +-
 test/integration/targets/k8s/tasks/main.yml   |   7 +-
 .../k8s/tasks/older_openshift_fail.yml        |  21 +++
 .../targets/k8s/tasks/openshift.yml           |   3 +-
 .../targets/k8s/tasks/validate_installed.yml  |   6 +-
 .../k8s/tasks/validate_not_installed.yml      |   2 +-
 test/integration/targets/k8s/tasks/waiter.yml |   1 +
 13 files changed, 276 insertions(+), 24 deletions(-)
 create mode 100644 test/integration/targets/k8s/tasks/apply.yml

diff --git a/lib/ansible/config/module_defaults.yml b/lib/ansible/config/module_defaults.yml
index ded586fee6a..98c1a5575f3 100644
--- a/lib/ansible/config/module_defaults.yml
+++ b/lib/ansible/config/module_defaults.yml
@@ -610,6 +610,8 @@ groupings:
   - k8s
   k8s_facts:
   - k8s
+  k8s_info:
+  - k8s
   k8s_service:
   - k8s
   k8s_scale:
diff --git a/lib/ansible/module_utils/k8s/raw.py b/lib/ansible/module_utils/k8s/raw.py
index c986a3d22f7..02a2ebf835b 100644
--- a/lib/ansible/module_utils/k8s/raw.py
+++ b/lib/ansible/module_utils/k8s/raw.py
@@ -31,8 +31,6 @@ from ansible.module_utils.six import string_types
 from ansible.module_utils.k8s.common import KubernetesAnsibleModule
 from ansible.module_utils.common.dict_transformations import dict_merge
 
-from distutils.version import LooseVersion
-
 
 try:
     import yaml
@@ -84,6 +82,7 @@ class KubernetesRawModule(KubernetesAnsibleModule):
         argument_spec['wait_condition'] = dict(type='dict', default=None, options=self.condition_spec)
         argument_spec['validate'] = dict(type='dict', default=None, options=self.validate_spec)
         argument_spec['append_hash'] = dict(type='bool', default=False)
+        argument_spec['apply'] = dict(type='bool')
         return argument_spec
 
     def __init__(self, k8s_kind=None, *args, **kwargs):
@@ -92,6 +91,7 @@ class KubernetesRawModule(KubernetesAnsibleModule):
 
         mutually_exclusive = [
             ('resource_definition', 'src'),
+            ('merge_type', 'apply'),
         ]
 
         KubernetesAnsibleModule.__init__(self, *args,
@@ -115,6 +115,13 @@ class KubernetesRawModule(KubernetesAnsibleModule):
         if self.params['merge_type']:
             if LooseVersion(self.openshift_version) < LooseVersion("0.6.2"):
                 self.fail_json(msg=missing_required_lib("openshift >= 0.6.2", reason="for merge_type"))
+        if self.params.get('apply') is not None:
+            if LooseVersion(self.openshift_version) < LooseVersion("0.9.0"):
+                self.fail_json(msg=missing_required_lib("openshift >= 0.9.0", reason="for apply"))
+            self.apply = self.params['apply']
+        else:
+            self.apply = LooseVersion(self.openshift_version) >= LooseVersion("0.9.0")
+
         if resource_definition:
             if isinstance(resource_definition, string_types):
                 try:
@@ -130,14 +137,14 @@ class KubernetesRawModule(KubernetesAnsibleModule):
             self.resource_definitions = self.load_resource_definitions(src)
 
         if not resource_definition and not src:
-            self.resource_definitions = [{
-                'kind': self.kind,
-                'apiVersion': self.api_version,
-                'metadata': {
-                    'name': self.name,
-                    'namespace': self.namespace
-                }
-            }]
+            implicit_definition = dict(
+                kind=self.kind,
+                apiVersion=self.api_version,
+                metadata=dict(name=self.name)
+            )
+            if self.namespace:
+                implicit_definition['metadata']['namespace'] = self.namespace
+            self.resource_definitions = [implicit_definition]
 
     def flatten_list_kind(self, list_resource, definitions):
         flattened = []
@@ -231,7 +238,9 @@ class KubernetesRawModule(KubernetesAnsibleModule):
             if self.append_hash and definition['kind'] in ['ConfigMap', 'Secret']:
                 name = '%s-%s' % (name, generate_hash(definition))
                 definition['metadata']['name'] = name
-            params = dict(name=name, namespace=namespace)
+            params = dict(name=name)
+            if namespace:
+                params['namespace'] = namespace
             existing = resource.get(**params)
         except NotFoundError:
             # Remove traceback so that it doesn't show up in later failures
@@ -271,6 +280,33 @@ class KubernetesRawModule(KubernetesAnsibleModule):
                             self.fail_json(msg="Resource deletion timed out", **result)
                 return result
         else:
+            if self.apply:
+                if self.check_mode:
+                    k8s_obj = definition
+                else:
+                    try:
+                        k8s_obj = resource.apply(definition, namespace=namespace).to_dict()
+                    except DynamicApiError as exc:
+                        msg = "Failed to apply object: {0}".format(exc.body)
+                        if self.warnings:
+                            msg += "\n" + "\n    ".join(self.warnings)
+                        self.fail_json(msg=msg, error=exc.status, status=exc.status, reason=exc.reason)
+                success = True
+                result['result'] = k8s_obj
+                if wait:
+                    success, result['result'], result['duration'] = self.wait(resource, definition, wait_timeout)
+                if existing:
+                    existing = existing.to_dict()
+                else:
+                    existing = {}
+                match, diffs = self.diff_objects(existing, result['result'])
+                result['changed'] = not match
+                result['diff'] = diffs
+                result['method'] = 'apply'
+                if not success:
+                    self.fail_json(msg="Resource apply timed out", **result)
+                return result
+
             if not existing:
                 if self.check_mode:
                     k8s_obj = definition
diff --git a/lib/ansible/modules/clustering/k8s/k8s.py b/lib/ansible/modules/clustering/k8s/k8s.py
index d50b6f02c60..88a95ac96a4 100644
--- a/lib/ansible/modules/clustering/k8s/k8s.py
+++ b/lib/ansible/modules/clustering/k8s/k8s.py
@@ -59,6 +59,7 @@ options:
     - 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).
+    - mutually exclusive with C(apply)
     choices:
     - json
     - merge
@@ -130,6 +131,15 @@ options:
       the generated hash and append_hash=no)
     type: bool
     version_added: "2.8"
+  apply:
+    description:
+    - C(apply) compares the desired resource definition with the previously supplied resource definition,
+      ignoring properties that are automatically generated
+    - C(apply) works better with Services than 'force=yes'
+    - C(apply) defaults to True if the openshift library is new enough to support it (0.9.0 or newer)
+    - mutually exclusive with C(merge_type)
+    type: bool
+    version_added: "2.9"
 
 requirements:
   - "python >= 2.7"
diff --git a/test/integration/targets/k8s/tasks/apply.yml b/test/integration/targets/k8s/tasks/apply.yml
new file mode 100644
index 00000000000..7a4abdfd6da
--- /dev/null
+++ b/test/integration/targets/k8s/tasks/apply.yml
@@ -0,0 +1,170 @@
+- block:
+    - python_requirements_info:
+        dependencies:
+          - openshift
+          - kubernetes
+
+    - set_fact:
+        apply_namespace: apply
+
+    - name: ensure namespace exists
+      k8s:
+        definition:
+          apiVersion: v1
+          kind: Namespace
+          metadata:
+            name: "{{ apply_namespace }}"
+
+    - name: add a configmap
+      k8s:
+        name: "apply-configmap"
+        namespace: "{{ apply_namespace }}"
+        definition:
+          kind: ConfigMap
+          apiVersion: v1
+          data:
+            one: "1"
+            two: "2"
+            three: "3"
+        apply: yes
+      register: k8s_configmap
+
+    - name: check configmap was created
+      assert:
+        that:
+          - k8s_configmap is changed
+          - k8s_configmap.result.metadata.annotations|default(False)
+
+    - name: add same configmap again
+      k8s:
+        definition:
+          kind: ConfigMap
+          apiVersion: v1
+          metadata:
+            name: "apply-configmap"
+            namespace: "{{ apply_namespace }}"
+          data:
+            one: "1"
+            two: "2"
+            three: "3"
+        apply: yes
+      register: k8s_configmap_2
+
+    - name: check nothing changed
+      assert:
+        that:
+          - k8s_configmap_2 is not changed
+
+    - name: add same configmap again but using name and namespace args
+      k8s:
+        name: "apply-configmap"
+        namespace: "{{ apply_namespace }}"
+        definition:
+          kind: ConfigMap
+          apiVersion: v1
+          data:
+            one: "1"
+            two: "2"
+            three: "3"
+        apply: yes
+      register: k8s_configmap_2a
+
+    - name: check nothing changed
+      assert:
+        that:
+          - k8s_configmap_2a is not changed
+
+    - name: update configmap
+      k8s:
+        definition:
+          kind: ConfigMap
+          apiVersion: v1
+          metadata:
+            name: "apply-configmap"
+            namespace: "{{ apply_namespace }}"
+          data:
+            one: "1"
+            three: "3"
+            four: "4"
+        apply: yes
+      register: k8s_configmap_3
+
+    - name: ensure that configmap has been correctly updated
+      assert:
+        that:
+          - k8s_configmap_3 is changed
+          - "'four' in k8s_configmap_3.result.data"
+          - "'two' not in k8s_configmap_3.result.data"
+
+    - name: add a service
+      k8s:
+        definition:
+          apiVersion: v1
+          kind: Service
+          metadata:
+            name: apply-svc
+            namespace: "{{ apply_namespace }}"
+          spec:
+            selector:
+              app: whatever
+            ports:
+              - name: http
+                port: 8080
+                targetPort: 8080
+        apply: yes
+      register: k8s_service
+
+    - name: add exactly same service
+      k8s:
+        definition:
+          apiVersion: v1
+          kind: Service
+          metadata:
+            name: apply-svc
+            namespace: "{{ apply_namespace }}"
+          spec:
+            selector:
+              app: whatever
+            ports:
+              - name: http
+                port: 8080
+                targetPort: 8080
+        apply: yes
+      register: k8s_service_2
+
+    - name: check nothing changed
+      assert:
+        that:
+          - k8s_service_2 is not changed
+
+    - name: change service ports
+      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_3
+
+    - name: check ports are correct
+      assert:
+        that:
+          - k8s_service_3 is changed
+          - k8s_service_3.result.spec.ports | length == 1
+          - k8s_service_3.result.spec.ports[0].port == 8081
+
+  always:
+    - name: remove namespace
+      k8s:
+        kind: Namespace
+        name: "{{ apply_namespace }}"
+        state: absent
diff --git a/test/integration/targets/k8s/tasks/crd.yml b/test/integration/targets/k8s/tasks/crd.yml
index d08beb23495..c9e47632f95 100644
--- a/test/integration/targets/k8s/tasks/crd.yml
+++ b/test/integration/targets/k8s/tasks/crd.yml
@@ -8,16 +8,21 @@
     - name: Create a namespace
       k8s:
         name: crd
-        kind: namespace
+        kind: Namespace
 
     - name: install custom resource definitions
       k8s:
         definition: "{{ lookup('file', role_path + '/files/setup-crd.yml') }}"
 
+    - name: pause 5 seconds to avoid race condition
+      pause:
+        seconds: 5
+
     - name: create custom resource definition
       k8s:
         definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}"
         namespace: crd
+        apply: "{{ create_crd_with_apply | default(omit) }}"
       register: create_crd
 
     - name: patch custom resource definition
diff --git a/test/integration/targets/k8s/tasks/delete.yml b/test/integration/targets/k8s/tasks/delete.yml
index 5ee48dca3ef..fef6e5e9f60 100644
--- a/test/integration/targets/k8s/tasks/delete.yml
+++ b/test/integration/targets/k8s/tasks/delete.yml
@@ -1,5 +1,5 @@
 - name: ensure that there are actually some nodes
-  k8s_facts:
+  k8s_info:
     kind: Node
   register: nodes
 
@@ -41,7 +41,7 @@
           - ds.result.status.currentNumberScheduled == ds.result.status.desiredNumberScheduled
 
     - name: check if pods exist
-      k8s_facts:
+      k8s_info:
         namespace: "{{ delete_namespace }}"
         kind: Pod
         label_selectors:
@@ -64,7 +64,7 @@
         wait: yes
 
     - name: show status of pods
-      k8s_facts:
+      k8s_info:
         namespace: "{{ delete_namespace }}"
         kind: Pod
         label_selectors:
@@ -77,7 +77,7 @@
         seconds: 30
 
     - name: check if pods still exist
-      k8s_facts:
+      k8s_info:
         namespace: "{{ delete_namespace }}"
         kind: Pod
         label_selectors:
diff --git a/test/integration/targets/k8s/tasks/full_test.yml b/test/integration/targets/k8s/tasks/full_test.yml
index f8c7aea30d6..08ba9c156e8 100644
--- a/test/integration/targets/k8s/tasks/full_test.yml
+++ b/test/integration/targets/k8s/tasks/full_test.yml
@@ -5,13 +5,14 @@
 # Kubernetes resources
 
 - include_tasks: delete.yml
+- include_tasks: apply.yml
 - include_tasks: waiter.yml
 
 - block:
     - name: Create a namespace
       k8s:
         name: testing
-        kind: namespace
+        kind: Namespace
       register: output
 
     - name: show output
@@ -21,7 +22,7 @@
     - name: Setting validate_certs to true causes a failure
       k8s:
         name: testing
-        kind: namespace
+        kind: Namespace
         validate_certs: yes
       ignore_errors: yes
       register: output
diff --git a/test/integration/targets/k8s/tasks/main.yml b/test/integration/targets/k8s/tasks/main.yml
index 51b8ac482d2..f1fc919b917 100644
--- a/test/integration/targets/k8s/tasks/main.yml
+++ b/test/integration/targets/k8s/tasks/main.yml
@@ -12,7 +12,7 @@
 
 - pip:
     name:
-      - openshift==0.8.8
+      - 'openshift>=0.9.0'
       - coverage
     virtualenv: "{{ virtualenv }}"
     virtualenv_command: "{{ virtualenv_command }}"
@@ -30,8 +30,8 @@
 
 - pip:
     name:
-      - openshift==0.8.8
       - kubernetes-validate==1.12.0
+      - 'openshift>=0.9.0'
       - coverage
     virtualenv: "{{ virtualenv }}"
     virtualenv_command: "{{ virtualenv_command }}"
@@ -71,7 +71,7 @@
 
 - pip:
     name:
-      - openshift==0.8.8
+      - 'openshift>=0.9.0'
       - coverage
     virtualenv: "{{ virtualenv }}"
     virtualenv_command: "{{ virtualenv_command }}"
@@ -80,6 +80,7 @@
 - include_tasks: full_test.yml
   vars:
     ansible_python_interpreter: "{{ virtualenv_interpreter }}"
+    create_crd_with_apply: no
     playbook_namespace: ansible-test-k8s-full
 
 - file:
diff --git a/test/integration/targets/k8s/tasks/older_openshift_fail.yml b/test/integration/targets/k8s/tasks/older_openshift_fail.yml
index 6f91f744b3e..e9006f8597e 100644
--- a/test/integration/targets/k8s/tasks/older_openshift_fail.yml
+++ b/test/integration/targets/k8s/tasks/older_openshift_fail.yml
@@ -46,3 +46,24 @@
         that:
           - k8s_validate is failed
           - "k8s_validate.msg == 'openshift >= 0.8.0 is required for validate'"
+
+    # apply
+    - name: attempt to use apply with older openshift
+      k8s:
+        definition:
+          metadata:
+            name: config-map-test
+            namespace: "{{ playbook_namespace }}"
+          apiVersion: v1
+          kind: ConfigMap
+          data:
+            hello: world
+        apply: yes
+      ignore_errors: yes
+      register: k8s_apply
+
+    - name: assert that apply fails gracefully
+      assert:
+        that:
+          - k8s_apply is failed
+          - "k8s_apply.msg.startswith('Failed to import the required Python library (openshift >= 0.9.0)')"
diff --git a/test/integration/targets/k8s/tasks/openshift.yml b/test/integration/targets/k8s/tasks/openshift.yml
index cb688db150d..f4a9006119f 100644
--- a/test/integration/targets/k8s/tasks/openshift.yml
+++ b/test/integration/targets/k8s/tasks/openshift.yml
@@ -2,8 +2,9 @@
 - name: Create a project
   k8s:
     name: testing
-    kind: project
+    kind: Project
     api_version: v1
+    apply: no
   register: output
 
 - name: show output
diff --git a/test/integration/targets/k8s/tasks/validate_installed.yml b/test/integration/targets/k8s/tasks/validate_installed.yml
index 1f05621f97d..224bd2eb68c 100644
--- a/test/integration/targets/k8s/tasks/validate_installed.yml
+++ b/test/integration/targets/k8s/tasks/validate_installed.yml
@@ -2,7 +2,7 @@
     - name: Create a namespace
       k8s:
         name: "{{ playbook_namespace }}"
-        kind: namespace
+        kind: Namespace
 
     - copy:
         src: files
@@ -82,6 +82,10 @@
       k8s:
         src: "{{ remote_tmp_dir }}/files/setup-crd.yml"
 
+    - name: wait a few seconds
+      pause:
+        seconds: 5
+
     - name: add custom resource definition
       k8s:
         src: "{{ remote_tmp_dir }}/files/crd-resource.yml"
diff --git a/test/integration/targets/k8s/tasks/validate_not_installed.yml b/test/integration/targets/k8s/tasks/validate_not_installed.yml
index 3c80bdde031..ecd17f7ea98 100644
--- a/test/integration/targets/k8s/tasks/validate_not_installed.yml
+++ b/test/integration/targets/k8s/tasks/validate_not_installed.yml
@@ -1,4 +1,4 @@
-  - python_requirements_facts:
+  - python_requirements_info:
       dependencies:
         - openshift
         - kubernetes
diff --git a/test/integration/targets/k8s/tasks/waiter.yml b/test/integration/targets/k8s/tasks/waiter.yml
index 04a2bd09bf5..cc0c1c78afc 100644
--- a/test/integration/targets/k8s/tasks/waiter.yml
+++ b/test/integration/targets/k8s/tasks/waiter.yml
@@ -253,6 +253,7 @@
             namespace: "{{ wait_namespace }}"
           spec:
             paused: True
+        apply: no
         wait: yes
         wait_condition:
           type: Progressing