Recursive diff (#366)

* Fix strategic patch merge of env variables

The key is `env`, not `envVars`

* Add a Kubernetes specific recursive dict diff

Understands strategic patch keys and can therefore
better diff list elements

* Remove unused dictdiffer dependency

Author: willthames

openshift/dynamic/apply.py

@@ -15,15 +15,15 @@
     'imagePullSecrets': 'name',
     'containers.volumeMounts': 'mountPath',
     'containers.volumeDevices': 'devicePath',
-    'containers.envVars': 'name',
+    'containers.env': 'name',
     'containers.ports': 'containerPort',
     'initContainers.volumeMounts': 'mountPath',
     'initContainers.volumeDevices': 'devicePath',
-    'initContainers.envVars': 'name',
+    'initContainers.env': 'name',
     'initContainers.ports': 'containerPort',
     'ephemeralContainers.volumeMounts': 'mountPath',
     'ephemeralContainers.volumeDevices': 'devicePath',
-    'ephemeralContainers.envVars': 'name',
+    'ephemeralContainers.env': 'name',
     'ephemeralContainers.ports': 'containerPort',
 }
 
@@ -183,6 +183,62 @@ def list_merge(last_applied, actual, desired, position):
         return desired
 
 
+def recursive_list_diff(list1, list2, position=None):
+    result = (list(), list())
+    if position in STRATEGIC_MERGE_PATCH_KEYS:
+        patch_merge_key = STRATEGIC_MERGE_PATCH_KEYS[position]
+        dict1 = list_to_dict(list1, patch_merge_key, position)
+        dict2 = list_to_dict(list2, patch_merge_key, position)
+        dict1_keys = set(dict1.keys())
+        dict2_keys = set(dict2.keys())
+        for key in dict1_keys - dict2_keys:
+            result[0].append(dict1[key])
+        for key in dict2_keys - dict1_keys:
+            result[1].append(dict2[key])
+        for key in dict1_keys & dict2_keys:
+            diff = recursive_diff(dict1[key], dict2[key], position)
+            if diff:
+                # reinsert patch merge key to relate changes in other keys to
+                # a specific list element
+                diff[0].update({patch_merge_key: dict1[key][patch_merge_key]})
+                diff[1].update({patch_merge_key: dict2[key][patch_merge_key]})
+                result[0].append(diff[0])
+                result[1].append(diff[1])
+        if result[0] or result[1]:
+            return result
+    elif list1 != list2:
+        return (list1, list2)
+    return None
+
+
+def recursive_diff(dict1, dict2, position=None):
+    if not position:
+        if 'kind' in dict1 and dict1.get('kind') == dict2.get('kind'):
+            position = dict1['kind']
+    left = dict((k, v) for (k, v) in dict1.items() if k not in dict2)
+    right = dict((k, v) for (k, v) in dict2.items() if k not in dict1)
+    for k in (set(dict1.keys()) & set(dict2.keys())):
+        if position:
+            this_position = "%s.%s" % (position, k)
+        if isinstance(dict1[k], dict) and isinstance(dict2[k], dict):
+            result = recursive_diff(dict1[k], dict2[k], this_position)
+            if result:
+                left[k] = result[0]
+                right[k] = result[1]
+        elif isinstance(dict1[k], list) and isinstance(dict2[k], list):
+            result = recursive_list_diff(dict1[k], dict2[k], this_position)
+            if result:
+                left[k] = result[0]
+                right[k] = result[1]
+        elif dict1[k] != dict2[k]:
+            left[k] = dict1[k]
+            right[k] = dict2[k]
+    if left or right:
+        return left, right
+    else:
+        return None
+
+
 def get_deletions(last_applied, desired):
     patch = {}
     for k, last_applied_value in last_applied.items():

python-openshift.spec

@@ -35,7 +35,6 @@ BuildRequires: python-setuptools
 BuildRequires: git
 
 Requires: python2
-Requires: python2-dictdiffer
 Requires: python2-kubernetes
 Requires: python2-string_utils
 Requires: python-requests
@@ -55,7 +54,6 @@ BuildRequires: %{py3}-setuptools
 BuildRequires: git
 
 Requires: %{py3}
-Requires: %{py3}-dictdiffer
 Requires: %{py3}-kubernetes
 Requires: %{py3}-string_utils
 Requires: %{py3}-requests
@@ -92,7 +90,7 @@ Python client for the OpenShift API
 #the requirements are also done in an non-backwards compatible way
 %if 0%{?rhel}
 sed -i -e "s/find_packages(include='openshift.*')/['openshift', 'openshift.dynamic', 'openshift.helper']/g" setup.py
-sed -i -e '30s/^/REQUIRES = [\n    "dictdiffer",\n    "jinja2",\n    "kubernetes",\n    "setuptools",\n    "six",\n    "ruamel.yaml",\n    "python-string-utils",\n]\n/g' setup.py
+sed -i -e '30s/^/REQUIRES = [\n    "jinja2",\n    "kubernetes",\n    "setuptools",\n    "six",\n    "ruamel.yaml",\n    "python-string-utils",\n]\n/g' setup.py
 sed -i -e "s/extract_requirements('requirements.txt')/REQUIRES/g" setup.py
 #sed -i -e '14,21d' setup.py
 %endif

requirements.txt

@@ -1,4 +1,3 @@
-dictdiffer
 jinja2
 kubernetes
 python-string-utils

test-requirements.txt

@@ -5,4 +5,3 @@ pytest
 pytest-bdd
 pytest-cov
 PyYAML
-dictdiffer

test/unit/test_diff.py

@@ -0,0 +1,69 @@
+from openshift.dynamic.apply import recursive_diff
+
+tests = [
+    dict(
+        before = dict(
+            kind="Service",
+            metadata=dict(name="foo"),
+            spec=dict(ports=[dict(port=8080, name="http")])
+        ),
+        after = dict(
+            kind="Service",
+            metadata=dict(name="foo"),
+            spec=dict(ports=[dict(port=8080, name="http")])
+        ),
+        expected = None
+    ),
+    dict(
+        before = dict(
+            kind="Service",
+            metadata=dict(name="foo"),
+            spec=dict(ports=[dict(port=8080, name="http")])
+        ),
+        after = dict(
+            kind="Service",
+            metadata=dict(name="foo"),
+            spec=dict(ports=[dict(port=8081, name="http")])
+        ),
+        expected = (
+            dict(spec=dict(ports=[dict(port=8080, name="http")])),
+            dict(spec=dict(ports=[dict(port=8081, name="http")]))
+        )
+    ),
+    dict(
+        before = dict(
+            kind="Service",
+            metadata=dict(name="foo"),
+            spec=dict(ports=[dict(port=8080, name="http"), dict(port=8081, name="https")])
+        ),
+        after = dict(
+            kind="Service",
+            metadata=dict(name="foo"),
+            spec=dict(ports=[dict(port=8081, name="https"), dict(port=8080, name="http")])
+        ),
+        expected = None
+    ),
+
+    dict(
+        before = dict(
+            kind="Pod",
+            metadata=dict(name="foo"),
+            spec=dict(containers=[dict(name="busybox", image="busybox",
+                                       env=[dict(name="hello", value="world"),
+                                            dict(name="another", value="next")])])
+        ),
+        after = dict(
+            kind="Pod",
+            metadata=dict(name="foo"),
+            spec=dict(containers=[dict(name="busybox", image="busybox",
+                                       env=[dict(name="hello", value="everyone")])])
+        ),
+        expected=(dict(spec=dict(containers=[dict(name="busybox", env=[dict(name="another", value="next"), dict(name="hello", value="world")])])),
+                  dict(spec=dict(containers=[dict(name="busybox", env=[dict(name="hello", value="everyone")])])))
+    ),
+    ]
+
+
+def test_diff():
+    for test in tests:
+        assert(recursive_diff(test['before'], test['after']) == test['expected'])