diff --git a/util/diff/diff.go b/util/diff/diff.go index 7d96a0b1777a6..a93db7f168a92 100644 --- a/util/diff/diff.go +++ b/util/diff/diff.go @@ -30,15 +30,26 @@ type DiffResultList struct { // Diff performs a diff on two unstructured objects. If the live object happens to have a // "kubectl.kubernetes.io/last-applied-configuration", then perform a three way diff. func Diff(config, live *unstructured.Unstructured) *DiffResult { + if config != nil { + config = stripTypeInformation(config) + } + if live != nil { + live = stripTypeInformation(live) + } orig := getLastAppliedConfigAnnotation(live) if orig != nil && config != nil { - return ThreeWayDiff(orig, config, live) + dr, err := ThreeWayDiff(orig, config, live) + if err == nil { + return dr + } + log.Warnf("three-way diff calculation failed: %v. Falling back to two-way diff", err) } return TwoWayDiff(config, live) } // TwoWayDiff performs a normal two-way diff between two unstructured objects. Ignores extra fields // in the live object. +// Inputs are assumed to be stripped of type information func TwoWayDiff(config, live *unstructured.Unstructured) *DiffResult { var configObj, liveObj map[string]interface{} if config != nil { @@ -57,39 +68,70 @@ func TwoWayDiff(config, live *unstructured.Unstructured) *DiffResult { // ThreeWayDiff performs a diff with the understanding of how to incorporate the // last-applied-configuration annotation in the diff. -func ThreeWayDiff(orig, config, live *unstructured.Unstructured) *DiffResult { +// Inputs are assumed to be stripped of type information +func ThreeWayDiff(orig, config, live *unstructured.Unstructured) (*DiffResult, error) { orig = removeNamespaceAnnotation(orig) - // remove extra fields in the live, that were not in the original object - liveObj := jsonutil.RemoveMapFields(orig.Object, live.Object) - // now we have a pruned live object - gjDiff := gojsondiff.New().CompareObjects(config.Object, liveObj) + // Remove defaulted fields from the live object. + // This subtracts any extra fields in the live object which are not present in last-applied-configuration. + // This is needed to perform a fair comparison when we send the objects to gojsondiff + live = &unstructured.Unstructured{Object: jsonutil.RemoveMapFields(orig.Object, live.Object)} + + // 1. calculate a 3-way merge patch + patchBytes, err := threeWayMergePatch(orig, config, live) + if err != nil { + return nil, err + } + + // 2. apply the patch against the live object + liveBytes, err := json.Marshal(live) + if err != nil { + return nil, err + } + versionedObject, err := scheme.Scheme.New(orig.GroupVersionKind()) + if err != nil { + return nil, err + } + patchedLiveBytes, err := strategicpatch.StrategicMergePatch(liveBytes, patchBytes, versionedObject) + if err != nil { + return nil, err + } + var patchedLive unstructured.Unstructured + err = json.Unmarshal(patchedLiveBytes, &patchedLive) + if err != nil { + return nil, err + } + + // 3. diff the live object vs. the patched live object + gjDiff := gojsondiff.New().CompareObjects(patchedLive.Object, live.Object) dr := DiffResult{ Diff: gjDiff, Modified: gjDiff.Modified(), } - // Theoretically, we should be able to return the diff result a this point. Just to be safe, - // calculate a kubernetes 3-way merge patch to see if kubernetes will also agree with what we - // just calculated. - patch, err := threeWayMergePatch(orig, config, live) + return &dr, nil +} + +// stripTypeInformation strips any type information (e.g. float64 vs. int) from the unstructured +// object by remarshalling the object. This is important for diffing since it will cause godiff +// to report a false difference. +func stripTypeInformation(un *unstructured.Unstructured) *unstructured.Unstructured { + unBytes, err := json.Marshal(un) if err != nil { - log.Warnf("Failed to calculate three way merge patch: %v", err) - return &dr - } - patchStr := string(patch) - modified := bool(patchStr != "{}") - if dr.Modified != modified { - // We theoretically should not get here. If we do, it is a issue with our diff calculation - // We should honor what kubernetes thinks. If we *do* get here, it means what we will be - // reporting OutOfSync, but do not have a good way to visualize the diff to the user. - log.Warnf("Disagreement in three way diff calculation: %s", patchStr) - dr.Modified = modified + panic(err) } - return &dr + var newUn unstructured.Unstructured + err = json.Unmarshal(unBytes, &newUn) + if err != nil { + panic(err) + } + return &newUn } +// removeNamespaceAnnotation remove the namespace and an empty annotation map from the metadata. +// The namespace field is *always* present in live objects, but not necessarily present in config +// or last-applied. This results in a diff which we don't care about. We delete the two so that +// the diff is more relevant. func removeNamespaceAnnotation(orig *unstructured.Unstructured) *unstructured.Unstructured { orig = orig.DeepCopy() - // remove the namespace an annotation from the if metadataIf, ok := orig.Object["metadata"]; ok { metadata := metadataIf.(map[string]interface{}) delete(metadata, "namespace") diff --git a/util/diff/diff_test.go b/util/diff/diff_test.go index 7a400c4d3ed4b..a6a6db92d0011 100644 --- a/util/diff/diff_test.go +++ b/util/diff/diff_test.go @@ -2,8 +2,10 @@ package diff import ( "encoding/json" + "io/ioutil" "log" "os" + "strings" "testing" "github.com/argoproj/argo-cd/test" @@ -193,8 +195,11 @@ var demoLive = ` ` // Tests a real world example -func TestDiffActualExample(t *testing.T) { +func TestThreeWayDiffExample1(t *testing.T) { var configUn, liveUn unstructured.Unstructured + // NOTE: it is intentional to unmarshal to Unstructured.Object instead of just Unstructured + // since it catches a case when we comparison fails due to subtle differences in types + // (e.g. float vs. int) err := json.Unmarshal([]byte(demoConfig), &configUn.Object) assert.Nil(t, err) err = json.Unmarshal([]byte(demoLive), &liveUn.Object) @@ -208,3 +213,70 @@ func TestDiffActualExample(t *testing.T) { } } + +func TestThreeWayDiffExample2(t *testing.T) { + configData, err := ioutil.ReadFile("testdata/elasticsearch-config.json") + assert.NoError(t, err) + liveData, err := ioutil.ReadFile("testdata/elasticsearch-live.json") + assert.NoError(t, err) + var configUn, liveUn unstructured.Unstructured + err = json.Unmarshal(configData, &configUn.Object) + assert.NoError(t, err) + err = json.Unmarshal(liveData, &liveUn.Object) + assert.NoError(t, err) + dr := Diff(&configUn, &liveUn) + assert.False(t, dr.Modified) + ascii, err := dr.ASCIIFormat(&configUn, formatOpts) + assert.Nil(t, err) + log.Println(ascii) +} + +// TestThreeWayDiffExample2WithDifference is same as TestThreeWayDiffExample2 but with differences +func TestThreeWayDiffExample2WithDifference(t *testing.T) { + configData, err := ioutil.ReadFile("testdata/elasticsearch-config.json") + assert.NoError(t, err) + liveData, err := ioutil.ReadFile("testdata/elasticsearch-live.json") + assert.NoError(t, err) + var configUn, liveUn unstructured.Unstructured + err = json.Unmarshal(configData, &configUn.Object) + assert.NoError(t, err) + err = json.Unmarshal(liveData, &liveUn.Object) + assert.NoError(t, err) + + labels := configUn.GetLabels() + // add a new label + labels["foo"] = "bar" + // modify a label + labels["chart"] = "elasticsearch-1.7.1" + // remove an existing label + delete(labels, "release") + configUn.SetLabels(labels) + + dr := Diff(&configUn, &liveUn) + assert.True(t, dr.Modified) + ascii, err := dr.ASCIIFormat(&configUn, formatOpts) + assert.Nil(t, err) + log.Println(ascii) + + // Check that we indicate missing/extra/changed correctly + showsMissing := false + showsExtra := false + showsChanged := 0 + for _, line := range strings.Split(ascii, "\n") { + if strings.HasPrefix(line, `- "foo": "bar"`) { + showsMissing = true + } + if strings.HasPrefix(line, `+ "release": "elasticsearch4"`) { + showsExtra = true + } + if strings.HasPrefix(line, `- "chart": "elasticsearch-1.7.1"`) { + showsChanged++ + } + if strings.HasPrefix(line, `+ "chart": "elasticsearch-1.7.0"`) { + showsChanged++ + } + } + assert.True(t, showsMissing) + assert.True(t, showsExtra) + assert.Equal(t, 2, showsChanged) +} diff --git a/util/diff/testdata/elasticsearch-config.json b/util/diff/testdata/elasticsearch-config.json new file mode 100644 index 0000000000000..cd0e4da28042e --- /dev/null +++ b/util/diff/testdata/elasticsearch-config.json @@ -0,0 +1,217 @@ +{ + "apiVersion": "apps/v1beta1", + "kind": "StatefulSet", + "metadata": { + "labels": { + "app": "elasticsearch", + "applications.argoproj.io/app-name": "elasticsearch4", + "chart": "elasticsearch-1.7.0", + "component": "data", + "heritage": "Tiller", + "release": "elasticsearch4" + }, + "name": "elasticsearch4-data" + }, + "spec": { + "replicas": 2, + "selector": { + "matchLabels": { + "app": "elasticsearch", + "component": "data", + "release": "elasticsearch4" + } + }, + "serviceName": "elasticsearch4-data", + "template": { + "metadata": { + "labels": { + "app": "elasticsearch", + "applications.argoproj.io/app-name": "elasticsearch4", + "component": "data", + "release": "elasticsearch4" + } + }, + "spec": { + "affinity": { + "podAntiAffinity": { + "preferredDuringSchedulingIgnoredDuringExecution": [ + { + "podAffinityTerm": { + "labelSelector": { + "matchLabels": { + "app": "elasticsearch", + "component": "data", + "release": "elasticsearch4" + } + }, + "topologyKey": "kubernetes.io/hostname" + }, + "weight": 1 + } + ] + } + }, + "containers": [ + { + "env": [ + { + "name": "DISCOVERY_SERVICE", + "value": "elasticsearch4-discovery" + }, + { + "name": "NODE_MASTER", + "value": "false" + }, + { + "name": "PROCESSORS", + "valueFrom": { + "resourceFieldRef": { + "resource": "limits.cpu" + } + } + }, + { + "name": "ES_JAVA_OPTS", + "value": "-Djava.net.preferIPv4Stack=true -Xms1536m -Xmx1536m" + }, + { + "name": "MINIMUM_MASTER_NODES", + "value": "2" + } + ], + "image": "docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.0", + "imagePullPolicy": "IfNotPresent", + "lifecycle": { + "postStart": { + "exec": { + "command": [ + "/bin/bash", + "/post-start-hook.sh" + ] + } + }, + "preStop": { + "exec": { + "command": [ + "/bin/bash", + "/pre-stop-hook.sh" + ] + } + } + }, + "name": "elasticsearch", + "ports": [ + { + "containerPort": 9300, + "name": "transport" + } + ], + "readinessProbe": { + "httpGet": { + "path": "/_cluster/health?local=true", + "port": 9200 + }, + "initialDelaySeconds": 5 + }, + "resources": { + "limits": { + "cpu": "1" + }, + "requests": { + "cpu": "25m", + "memory": "1536Mi" + } + }, + "volumeMounts": [ + { + "mountPath": "/usr/share/elasticsearch/data", + "name": "data" + }, + { + "mountPath": "/usr/share/elasticsearch/config/elasticsearch.yml", + "name": "config", + "subPath": "elasticsearch.yml" + }, + { + "mountPath": "/pre-stop-hook.sh", + "name": "config", + "subPath": "pre-stop-hook.sh" + }, + { + "mountPath": "/post-start-hook.sh", + "name": "config", + "subPath": "post-start-hook.sh" + } + ] + } + ], + "initContainers": [ + { + "command": [ + "sysctl", + "-w", + "vm.max_map_count=262144" + ], + "image": "busybox", + "imagePullPolicy": "Always", + "name": "sysctl", + "securityContext": { + "privileged": true + } + }, + { + "command": [ + "/bin/bash", + "-c", + "chown -R elasticsearch:elasticsearch /usr/share/elasticsearch/data && chown -R elasticsearch:elasticsearch /usr/share/elasticsearch/logs" + ], + "image": "docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.0", + "imagePullPolicy": "IfNotPresent", + "name": "chown", + "securityContext": { + "runAsUser": 0 + }, + "volumeMounts": [ + { + "mountPath": "/usr/share/elasticsearch/data", + "name": "data" + } + ] + } + ], + "securityContext": { + "fsGroup": 1000 + }, + "terminationGracePeriodSeconds": 3600, + "volumes": [ + { + "configMap": { + "name": "elasticsearch4" + }, + "name": "config" + } + ] + } + }, + "updateStrategy": { + "type": "OnDelete" + }, + "volumeClaimTemplates": [ + { + "metadata": { + "name": "data" + }, + "spec": { + "accessModes": [ + "ReadWriteOnce" + ], + "resources": { + "requests": { + "storage": "30Gi" + } + } + } + } + ] + } +} \ No newline at end of file diff --git a/util/diff/testdata/elasticsearch-live.json b/util/diff/testdata/elasticsearch-live.json new file mode 100644 index 0000000000000..a071fde5e97b7 --- /dev/null +++ b/util/diff/testdata/elasticsearch-live.json @@ -0,0 +1,261 @@ +{ + "apiVersion": "apps/v1beta1", + "kind": "StatefulSet", + "metadata": { + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"apps/v1beta1\",\"kind\":\"StatefulSet\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"elasticsearch\",\"applications.argoproj.io/app-name\":\"elasticsearch4\",\"chart\":\"elasticsearch-1.7.0\",\"component\" :\"data\",\"heritage\":\"Tiller\",\"release\":\"elasticsearch4\"},\"name\":\"elasticsearch4-data\",\"namespace\":\"elasticsearch4\"},\"spec\":{\"replicas\":2,\"selector\":{\"matchLabels\":{\"app\":\"elasticsearch\",\"component\":\"data\",\"release\":\"elasticsearch4\"}},\"serviceName\":\"elasticsearch4-data\",\"template\":{\"metadata\":{\"labels\":{\"app\":\"elasticsearch\",\"applications.argoproj.io/app-name\":\"elasticsearch4\",\"component\":\"data\",\"release\":\"elasticsearch4\"}},\"spec\":{\"affinity\":{\"podAntiAffinity\":{\"preferredDuringSchedulingIgnoredDuringExecution\":[{\"podAffinityTerm\":{\"labelSelector\":{\"matchLabels\":{\"app\":\"elasticsearch\",\"component\":\"data\",\"release\":\"elasticsearch4\"}},\"topologyKey\":\"kubernetes.io/hostname\"},\"weight\":1}]}},\"containers\":[{\"env\":[{\"name\":\"DISCOVERY_SERVICE\",\"value\":\"elasticsearch4-discovery\"},{\"name\":\"NODE_MASTER\",\"value\":\"false\"},{\"name\":\"PROCESSORS\",\"valueFrom\":{\"resourceFieldRef\":{\"resource\":\"limits.cpu\"}}},{\"name\":\"ES_JAVA_OPTS\",\"value\":\"-Djava.net.preferIPv4Stack=true -Xms1536m -Xmx1536m\"},{\"name\":\"MINIMUM_MASTER_NODES\",\"value\":\"2\"}],\"image\":\"docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.0\",\"imagePullPolicy\":\"IfNotPresent\",\"lifecycle\":{\"postStart\":{\"exec\":{\"command\":[\"/bin/bash\",\"/post-start-hook.sh\"]}},\"preStop\":{\"exec\":{\"command\":[\"/bin/bash\",\"/pre-stop-hook.sh\"]}}},\"name\":\"elasticsearch\",\"ports\":[{\"containerPort\":9300,\"name\":\"transport\"}],\"readinessProbe\":{\"httpGet\":{\"path\":\"/_cluster/health?local=true\",\"port\":9200},\"initialDelaySeconds\":5},\"resources\":{\"limits\":{\"cpu\":\"1\"},\"requests\":{\"cpu\":\"25m\",\"memory\":\"1536Mi\"}},\"volumeMounts\":[{\"mountPath\":\"/usr/share/elasticsearch/data\",\"name\":\"data\"},{\"mountPath\":\"/usr/share/elasticsearch/config/elasticsearch.yml\",\"name\":\"config\",\"subPath\":\"elasticsearch.yml\"},{\"mountPath\":\"/pre-stop-hook.sh\",\"name\":\"config\",\"subPath\":\"pre-stop-hook.sh\"},{\"mountPath\":\"/post-start-hook.sh\",\"name\":\"config\",\"subPath\":\"post-start-hook.sh\"}]}],\"initContainers\":[{\"command\":[\"sysctl\",\"-w\",\"vm.max_map_count=262144\"],\"image\":\"busybox\",\"imagePullPolicy\":\"Always\",\"name\":\"sysctl\",\"securityContext\":{\"privileged\":true}},{\"command\":[\"/bin/bash\",\"-c\",\"chown -R elasticsearch:elasticsearch /usr/share/elasticsearch/data \\u0026\\u0026 chown -R elasticsearch:elasticsearch /usr/share/elasticsearch/logs\"],\"image\":\"docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.0\",\"imagePullPolicy\":\"IfNotPresent\",\"name\":\"chown\",\"securityContext\":{\"runAsUser\":0},\"volumeMounts\":[{\"mountPath\":\"/usr/share/elasticsearch/data\",\"name\":\"data\"}]}],\"securityContext\":{\"fsGroup\":1000},\"terminationGracePeriodSeconds\":3600,\"volumes\":[{\"configMap\":{\"name\":\"elasticsearch4\"},\"name\":\"config\"}]}},\"updateStrategy\":{\"type\":\"OnDelete\"},\"volumeClaimTemplates\":[{\"metadata\":{\"name\":\"data\"},\"spec\":{\"accessModes\":[\"ReadWriteOnce\"],\"resources\":{\"requests\":{\"storage\":\"30Gi\"}}}}]}}\n" + }, + "creationTimestamp": "2018-09-14T22:20:41Z", + "generation": 1, + "labels": { + "app": "elasticsearch", + "applications.argoproj.io/app-name": "elasticsearch4", + "chart": "elasticsearch-1.7.0", + "component": "data", + "heritage": "Tiller", + "release": "elasticsearch4" + }, + "name": "elasticsearch4-data", + "namespace": "elasticsearch4", + "resourceVersion": "10689842", + "selfLink": "/apis/apps/v1beta1/namespaces/elasticsearch4/statefulsets/elasticsearch4-data", + "uid": "6a850cf9-b86c-11e8-bbd2-42010a8a00bb" + }, + "spec": { + "podManagementPolicy": "OrderedReady", + "replicas": 2, + "revisionHistoryLimit": 10, + "selector": { + "matchLabels": { + "app": "elasticsearch", + "component": "data", + "release": "elasticsearch4" + } + }, + "serviceName": "elasticsearch4-data", + "template": { + "metadata": { + "creationTimestamp": null, + "labels": { + "app": "elasticsearch", + "applications.argoproj.io/app-name": "elasticsearch4", + "component": "data", + "release": "elasticsearch4" + } + }, + "spec": { + "affinity": { + "podAntiAffinity": { + "preferredDuringSchedulingIgnoredDuringExecution": [ + { + "podAffinityTerm": { + "labelSelector": { + "matchLabels": { + "app": "elasticsearch", + "component": "data", + "release": "elasticsearch4" + } + }, + "topologyKey": "kubernetes.io/hostname" + }, + "weight": 1 + } + ] + } + }, + "containers": [ + { + "env": [ + { + "name": "DISCOVERY_SERVICE", + "value": "elasticsearch4-discovery" + }, + { + "name": "NODE_MASTER", + "value": "false" + }, + { + "name": "PROCESSORS", + "valueFrom": { + "resourceFieldRef": { + "divisor": "0", + "resource": "limits.cpu" + } + } + }, + { + "name": "ES_JAVA_OPTS", + "value": "-Djava.net.preferIPv4Stack=true -Xms1536m -Xmx1536m" + }, + { + "name": "MINIMUM_MASTER_NODES", + "value": "2" + } + ], + "image": "docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.0", + "imagePullPolicy": "IfNotPresent", + "lifecycle": { + "postStart": { + "exec": { + "command": [ + "/bin/bash", + "/post-start-hook.sh" + ] + } + }, + "preStop": { + "exec": { + "command": [ + "/bin/bash", + "/pre-stop-hook.sh" + ] + } + } + }, + "name": "elasticsearch", + "ports": [ + { + "containerPort": 9300, + "name": "transport", + "protocol": "TCP" + } + ], + "readinessProbe": { + "failureThreshold": 3, + "httpGet": { + "path": "/_cluster/health?local=true", + "port": 9200, + "scheme": "HTTP" + }, + "initialDelaySeconds": 5, + "periodSeconds": 10, + "successThreshold": 1, + "timeoutSeconds": 1 + }, + "resources": { + "limits": { + "cpu": "1" + }, + "requests": { + "cpu": "25m", + "memory": "1536Mi" + } + }, + "terminationMessagePath": "/dev/termination-log", + "terminationMessagePolicy": "File", + "volumeMounts": [ + { + "mountPath": "/usr/share/elasticsearch/data", + "name": "data" + }, + { + "mountPath": "/usr/share/elasticsearch/config/elasticsearch.yml", + "name": "config", + "subPath": "elasticsearch.yml" + }, + { + "mountPath": "/pre-stop-hook.sh", + "name": "config", + "subPath": "pre-stop-hook.sh" + }, + { + "mountPath": "/post-start-hook.sh", + "name": "config", + "subPath": "post-start-hook.sh" + } + ] + } + ], + "dnsPolicy": "ClusterFirst", + "initContainers": [ + { + "command": [ + "sysctl", + "-w", + "vm.max_map_count=262144" + ], + "image": "busybox", + "imagePullPolicy": "Always", + "name": "sysctl", + "resources": {}, + "securityContext": { + "privileged": true + }, + "terminationMessagePath": "/dev/termination-log", + "terminationMessagePolicy": "File" + }, + { + "command": [ + "/bin/bash", + "-c", + "chown -R elasticsearch:elasticsearch /usr/share/elasticsearch/data && chown -R elasticsearch:elasticsearch /usr/share/elasticsearch/logs" + ], + "image": "docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.0", + "imagePullPolicy": "IfNotPresent", + "name": "chown", + "resources": {}, + "securityContext": { + "runAsUser": 0 + }, + "terminationMessagePath": "/dev/termination-log", + "terminationMessagePolicy": "File", + "volumeMounts": [ + { + "mountPath": "/usr/share/elasticsearch/data", + "name": "data" + } + ] + } + ], + "restartPolicy": "Always", + "schedulerName": "default-scheduler", + "securityContext": { + "fsGroup": 1000 + }, + "terminationGracePeriodSeconds": 3600, + "volumes": [ + { + "configMap": { + "defaultMode": 420, + "name": "elasticsearch4" + }, + "name": "config" + } + ] + } + }, + "updateStrategy": { + "type": "OnDelete" + }, + "volumeClaimTemplates": [ + { + "metadata": { + "creationTimestamp": null, + "name": "data" + }, + "spec": { + "accessModes": [ + "ReadWriteOnce" + ], + "resources": { + "requests": { + "storage": "30Gi" + } + } + }, + "status": { + "phase": "Pending" + } + } + ] + }, + "status": { + "collisionCount": 0, + "currentReplicas": 2, + "currentRevision": "elasticsearch4-data-74f6d9c899", + "observedGeneration": 1, + "readyReplicas": 2, + "replicas": 2, + "updateRevision": "elasticsearch4-data-74f6d9c899" + } +} \ No newline at end of file diff --git a/util/json/json.go b/util/json/json.go index 5150ef7230e55..95e5407a666df 100644 --- a/util/json/json.go +++ b/util/json/json.go @@ -73,3 +73,12 @@ func removeListFields(config, live []interface{}) []interface{} { } return result } + +// MustMarshal is a convenience function to marshal an object successfully or panic +func MustMarshal(v interface{}) []byte { + bytes, err := json.Marshal(v) + if err != nil { + panic(err) + } + return bytes +}