Provide apply_object method to help check mode (#315)

* Provide apply_object method to help check mode

Split existing apply method into one method that finds the
changes and one method that does the changes

* Add test case

* Fix get_deletions when data is of mixed string type

Somehow the definition argument passed to apply_object can
be mostly str type, whereas the existing resource gets converted
to mostly unicode type. I've only seen this happen with ansible's
check mode!

* Improve code paths through get_deletions

Safeguard the case where `last_applied_value` is a `dict` and
`desired_value` is not a `dict`

This allows us to remove another code path when `desired_value`
is `None`

Author: willthames

openshift/dynamic/apply.py

@@ -5,7 +5,7 @@
 
 LAST_APPLIED_CONFIG_ANNOTATION = 'kubectl.kubernetes.io/last-applied-configuration'
 
-def apply(resource, definition):
+def apply_object(resource, definition):
     desired_annotation = dict(
         metadata=dict(
             annotations={
@@ -16,7 +16,7 @@ def apply(resource, definition):
     try:
         actual = resource.get(name=definition['metadata']['name'], namespace=definition['metadata'].get('namespace'))
     except NotFoundError:
-        return resource.create(body=dict_merge(definition, desired_annotation), namespace=definition['metadata'].get('namespace'))
+        return None, dict_merge(definition, desired_annotation)
     last_applied = actual.metadata.get('annotations',{}).get(LAST_APPLIED_CONFIG_ANNOTATION)
 
     if last_applied:
@@ -25,18 +25,23 @@ def apply(resource, definition):
         del actual_dict['metadata']['annotations'][LAST_APPLIED_CONFIG_ANNOTATION]
         patch = merge(last_applied, definition, actual_dict)
         if patch:
-            return resource.patch(body=dict_merge(patch, desired_annotation),
-                                  name=definition['metadata']['name'],
-                                  namespace=definition['metadata'].get('namespace'),
-                                  content_type='application/merge-patch+json')
+            return actual.to_dict(), dict_merge(patch, desired_annotation)
         else:
-            return actual
+            return actual.to_dict(), actual.to_dict()
     else:
-        return resource.patch(
-            body=definition,
-            name=definition['metadata']['name'],
-            namespace=definition['metadata'].get('namespace'),
-            content_type='application/merge-patch+json')
+        return actual.to_dict(), dict_merge(definition, desired_annotation)
+
+
+def apply(resource, definition):
+    existing, desired = apply_object(resource, definition)
+    if not existing:
+        return resource.create(body=desired, namespace=definition['metadata'].get('namespace'))
+    if existing == desired:
+        return resource.get(name=definition['metadata']['name'], namespace=definition['metadata'].get('namespace'))
+    return resource.patch(body=desired,
+                          name=definition['metadata']['name'],
+                          namespace=definition['metadata'].get('namespace'),
+                          content_type='application/merge-patch+json')
 
 
 # The patch is the difference from actual to desired without deletions, plus deletions
@@ -69,11 +74,7 @@ def get_deletions(last_applied, desired):
     patch = {}
     for k, last_applied_value in last_applied.items():
         desired_value = desired.get(k)
-        if desired_value is None:
-            patch[k] = None
-        elif type(last_applied_value) != type(desired_value):
-            patch[k] = desired_value
-        elif isinstance(last_applied_value, dict):
+        if isinstance(last_applied_value, dict) and isinstance(desired_value, dict):
             p = get_deletions(last_applied_value, desired_value)
             if p:
                 patch[k] = p

test/unit/test_apply.py

@@ -79,6 +79,34 @@
         ),
         expected = dict(spec=dict(ports=[dict(port=8081, name="http")]))
     ),
+    # This last one is based on a real world case where definition was mostly
+    # str type and everything else was mostly unicode type (don't ask me how)
+    dict(
+        last_applied = {
+            u'kind': u'ConfigMap',
+            u'data': {u'one': '1', 'three': '3', 'two': '2'},
+            u'apiVersion': u'v1',
+            u'metadata': {u'namespace': u'apply', u'name': u'apply-configmap'}
+        },
+        actual = {
+            u'kind': u'ConfigMap',
+            u'data': {u'one': '1', 'three': '3', 'two': '2'},
+            u'apiVersion': u'v1',
+            u'metadata': {u'namespace': u'apply', u'name': u'apply-configmap',
+                          u'resourceVersion': '1714994',
+                          u'creationTimestamp': u'2019-08-17T05:08:05Z', u'annotations': {},
+                          u'selfLink': u'/api/v1/namespaces/apply/configmaps/apply-configmap',
+                          u'uid': u'fed45fb0-c0ac-11e9-9d95-025000000001'}
+        },
+        desired = {
+            'kind': u'ConfigMap',
+            'data': {'one': '1', 'three': '3', 'two': '2'},
+            'apiVersion': 'v1',
+            'metadata': {'namespace': 'apply', 'name': 'apply-configmap'}
+        },
+        expected = dict()
+    ),
+
 ]