Take last applied configuration into account when patching (#341)

If, as previously, we ignore the last-applied annotation when
comparing desired and actual, we end up with a result where,
when deleting all annotations in a change, no annotations are
deleted(!)

Going from
```
metadata:
  name: my-pod
  annotations:
    x: y
```

to:
```
metadata:
  name: my-pod
```

used to result in a patch that updated the last applied configuration
value only.

Now it correctly sets the old annotations to None too.

Break out apply_patch as a separate method to allow it be tested in
isolation.

Ensure that the last-applied annotation is sorted.

Ensure that the last-applied annotation is loaded consistently (i.e.
without unnecessary conversion to u'value's)

Author: willthames

openshift/dynamic/apply.py

@@ -1,6 +1,7 @@
 from collections import OrderedDict
 from copy import deepcopy
 import json
+import sys
 
 from openshift.dynamic.exceptions import NotFoundError, ApplyException
 
@@ -49,31 +50,67 @@
 )
 
 
-def apply_object(resource, definition):
-    desired_annotation = dict(
+if sys.version_info.major >= 3:
+    json_loads_byteified = json.loads
+else:
+    # https://stackoverflow.com/a/33571117
+    def json_loads_byteified(json_text):
+        return _byteify(
+            json.loads(json_text, object_hook=_byteify),
+            ignore_dicts=True
+        )
+
+    def _byteify(data, ignore_dicts = False):
+        # if this is a unicode string, return its string representation
+        if isinstance(data, unicode):  # noqa: F821
+            return data.encode('utf-8')
+        # if this is a list of values, return list of byteified values
+        if isinstance(data, list):
+            return [ _byteify(item, ignore_dicts=True) for item in data ]
+        # if this is a dictionary, return dictionary of byteified keys and values
+        # but only if we haven't already byteified it
+        if isinstance(data, dict) and not ignore_dicts:
+            return {
+                _byteify(key, ignore_dicts=True): _byteify(value, ignore_dicts=True)
+                for key, value in data.items()
+            }
+        # if it's anything else, return it in its original form
+        return data
+
+
+def annotate(desired):
+    return dict(
         metadata=dict(
             annotations={
-                LAST_APPLIED_CONFIG_ANNOTATION: json.dumps(definition, separators=(',', ':'), indent=None)
+                LAST_APPLIED_CONFIG_ANNOTATION: json.dumps(desired, separators=(',', ':'), indent=None, sort_keys=True)
             }
         )
     )
-    try:
-        actual = resource.get(name=definition['metadata']['name'], namespace=definition['metadata'].get('namespace'))
-    except NotFoundError:
-        return None, dict_merge(definition, desired_annotation)
-    last_applied = actual.metadata.get('annotations',{}).get(LAST_APPLIED_CONFIG_ANNOTATION)
+
+
+def apply_patch(actual, desired):
+    last_applied = actual['metadata'].get('annotations',{}).get(LAST_APPLIED_CONFIG_ANNOTATION)
 
     if last_applied:
-        last_applied = json.loads(last_applied)
-        actual_dict = actual.to_dict()
-        del actual_dict['metadata']['annotations'][LAST_APPLIED_CONFIG_ANNOTATION]
-        patch = merge(last_applied, definition, actual_dict)
+        # ensure that last_applied doesn't come back as a dict of unicode key/value pairs
+        # json.loads can be used if we stop supporting python 2
+        last_applied = json_loads_byteified(last_applied)
+        patch = merge(dict_merge(last_applied, annotate(last_applied)),
+                      dict_merge(desired, annotate(desired)), actual)
         if patch:
-            return actual.to_dict(), dict_merge(patch, desired_annotation)
+            return actual, patch
         else:
-            return actual.to_dict(), actual.to_dict()
+            return actual, actual
     else:
-        return actual.to_dict(), dict_merge(definition, desired_annotation)
+        return actual, dict_merge(desired, annotate(desired))
+
+
+def apply_object(resource, definition):
+    try:
+        actual = resource.get(name=definition['metadata']['name'], namespace=definition['metadata'].get('namespace'))
+    except NotFoundError:
+        return None, dict_merge(definition, annotate(definition))
+    return apply_patch(actual.to_dict(), definition)
 
 
 def apply(resource, definition):

test/unit/test_apply.py

@@ -1,7 +1,6 @@
-from openshift.dynamic.apply import merge
+from openshift.dynamic.apply import merge, apply_patch
 
 tests = [
-
     dict(
         last_applied = dict(
             kind="ConfigMap",
@@ -41,6 +40,19 @@
         ),
         expected = dict(data=dict(two=None, three="3"))
     ),
+    dict(
+        last_applied = dict(
+            kind="ConfigMap",
+            metadata=dict(name="foo", annotations=dict(this="one", hello="world")),
+            data=dict(one="1", two="2")
+        ),
+        desired = dict(
+            kind="ConfigMap",
+            metadata=dict(name="foo"),
+            data=dict(one="1", three="3")
+        ),
+        expected = dict(metadata=dict(annotations=None), data=dict(two=None, three="3"))
+    ),
     dict(
         last_applied = dict(
             kind="Service",
@@ -278,3 +290,26 @@
 def test_merges():
     for test in tests:
         assert(merge(test['last_applied'], test['desired'], test.get('actual', test['last_applied'])) == test['expected'])
+
+
+def test_apply_patch():
+        actual = dict(
+            kind="ConfigMap",
+            metadata=dict(name="foo",
+                          annotations={'kubectl.kubernetes.io/last-applied-configuration':
+                                       '{"data":{"one":"1","two":"2"},"kind":"ConfigMap",'
+                                       '"metadata":{"annotations":{"hello":"world","this":"one"},"name":"foo"}}',
+                                       'this': 'one', 'hello': 'world'}),
+            data=dict(one="1", two="2")
+        )
+        desired = dict(
+            kind="ConfigMap",
+            metadata=dict(name="foo"),
+            data=dict(one="1", three="3")
+        )
+        expected = dict(
+            metadata=dict(
+                annotations={'kubectl.kubernetes.io/last-applied-configuration': '{"data":{"one":"1","three":"3"},"kind":"ConfigMap","metadata":{"name":"foo"}}',
+                             'this': None, 'hello': None}),
+                data=dict(two=None, three="3"))
+        assert(apply_patch(actual, desired) == (actual, expected))