From 18ca0dd6c4c9bd4c0c95f741c92376ba8cc3bc57 Mon Sep 17 00:00:00 2001
From: Matthew Fisher <matt.fisher@microsoft.com>
Date: Tue, 30 Jul 2019 16:08:31 -0700
Subject: [PATCH] ref(client): use three-way merge patch strategy

Co-Signed-by: Taylor Thomas <taylor.thomas@microsoft.com>
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
---
 pkg/kube/client.go      | 26 ++++++++++++++++++--------
 pkg/kube/client_test.go | 13 +++++++++++++
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/pkg/kube/client.go b/pkg/kube/client.go
index 6176ad0d0..a9e9a6222 100644
--- a/pkg/kube/client.go
+++ b/pkg/kube/client.go
@@ -29,7 +29,6 @@ import (
 	"github.com/pkg/errors"
 	batch "k8s.io/api/batch/v1"
 	v1 "k8s.io/api/core/v1"
-	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -281,12 +280,17 @@ func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.P
 		return nil, types.StrategicMergePatchType, errors.Wrap(err, "serializing target configuration")
 	}
 
-	// While different objects need different merge types, the parent function
-	// that calls this does not try to create a patch when the data (first
-	// returned object) is nil. We can skip calculating the merge type as
-	// the returned merge type is ignored.
-	if apiequality.Semantic.DeepEqual(oldData, newData) {
-		return nil, types.StrategicMergePatchType, nil
+	// Fetch the current object for the three way merge
+	helper := resource.NewHelper(target.Client, target.Mapping)
+	currentObj, err := helper.Get(target.Namespace, target.Name, target.Export)
+	if err != nil && !apierrors.IsNotFound(err) {
+		return nil, types.StrategicMergePatchType, errors.Wrapf(err, "unable to get data for current object %s/%s", target.Namespace, target.Name)
+	}
+
+	// Even if currentObj is nil (because it was not found), it will marshal just fine
+	currentData, err := json.Marshal(currentObj)
+	if err != nil {
+		return nil, types.StrategicMergePatchType, errors.Wrap(err, "serializing live configuration")
 	}
 
 	// Get a versioned object
@@ -301,7 +305,13 @@ func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.P
 		patch, err := jsonpatch.CreateMergePatch(oldData, newData)
 		return patch, types.MergePatchType, err
 	}
-	patch, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, versionedObject)
+
+	patchMeta, err := strategicpatch.NewPatchMetaFromStruct(versionedObject)
+	if err != nil {
+		return nil, types.StrategicMergePatchType, errors.Wrap(err, "unable to create patch metadata from object")
+	}
+
+	patch, err := strategicpatch.CreateThreeWayMergePatch(oldData, newData, currentData, patchMeta, true)
 	return patch, types.StrategicMergePatchType, err
 }
 
diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go
index c9da5cc57..b097c4782 100644
--- a/pkg/kube/client_test.go
+++ b/pkg/kube/client_test.go
@@ -119,6 +119,17 @@ func TestUpdate(t *testing.T) {
 				return newResponse(200, &listA.Items[0])
 			case p == "/namespaces/default/pods/otter" && m == "GET":
 				return newResponse(200, &listA.Items[1])
+			case p == "/namespaces/default/pods/otter" && m == "PATCH":
+				data, err := ioutil.ReadAll(req.Body)
+				if err != nil {
+					t.Fatalf("could not dump request: %s", err)
+				}
+				req.Body.Close()
+				expected := `{}`
+				if string(data) != expected {
+					t.Errorf("expected patch\n%s\ngot\n%s", expected, string(data))
+				}
+				return newResponse(200, &listB.Items[0])
 			case p == "/namespaces/default/pods/dolphin" && m == "GET":
 				return newResponse(404, notFoundBody())
 			case p == "/namespaces/default/pods/starfish" && m == "PATCH":
@@ -165,10 +176,12 @@ func TestUpdate(t *testing.T) {
 	// 	t.Fatal(err)
 	// }
 	expectedActions := []string{
+		"/namespaces/default/pods/starfish:GET",
 		"/namespaces/default/pods/starfish:GET",
 		"/namespaces/default/pods/starfish:PATCH",
 		"/namespaces/default/pods/otter:GET",
 		"/namespaces/default/pods/otter:GET",
+		"/namespaces/default/pods/otter:PATCH",
 		"/namespaces/default/pods/dolphin:GET",
 		"/namespaces/default/pods:POST",
 		"/namespaces/default/pods/squid:DELETE",