Support retainkeys strategy in new apply merge code

pkg/kubectl/apply/strategy/BUILD

@@ -25,6 +25,7 @@ go_test(
         "replace_map_list_test.go",
         "replace_map_test.go",
         "replace_primitive_list_test.go",
+        "retain_keys_test.go",
         "suite_test.go",
         "utils_test.go",
     ],

pkg/kubectl/apply/strategy/retain_keys_test.go

@@ -0,0 +1,195 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package strategy_test
+
+import (
+	. "github.com/onsi/ginkgo"
+
+	"k8s.io/kubernetes/pkg/kubectl/apply/strategy"
+)
+
+var _ = Describe("Merging fields with the retainkeys strategy", func() {
+	Context("where some fields are only defined remotely", func() {
+		It("should drop those fields ", func() {
+			recorded := create(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+spec:
+  strategy:
+`)
+			local := create(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+spec:
+  strategy:
+    type: Recreate
+`)
+			remote := create(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+spec:
+  strategy:
+    type: RollingUpdate
+    rollingUpdate:
+      maxUnavailable: 1
+      maxSurge: 1
+`)
+			expected := create(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+spec:
+  strategy:
+    type: Recreate
+`)
+			run(strategy.Create(strategy.Options{}), recorded, local, remote, expected)
+		})
+	})
+
+	Context("where some fields are defined both locally and remotely", func() {
+		It("should merge those fields", func() {
+			recorded := create(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+spec:
+  strategy:
+`)
+			local := create(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+spec:
+  strategy:
+    type: RollingUpdate
+    rollingUpdate:
+      maxUnavailable: 2
+`)
+			remote := create(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+spec:
+  strategy:
+    type: RollingUpdate
+    rollingUpdate:
+      maxSurge: 1
+`)
+			expected := create(`
+apiVersion: extensions/v1beta1
+kind: Deployment
+spec:
+  strategy:
+    type: RollingUpdate
+    rollingUpdate:
+      maxUnavailable: 2
+      maxSurge: 1
+`)
+			run(strategy.Create(strategy.Options{}), recorded, local, remote, expected)
+		})
+	})
+
+	Context("where the elements are in a list and some fields are only defined remotely", func() {
+		It("should drop those fields ", func() {
+			recorded := create(`
+apiVersion: apps/v1beta1
+kind: Deployment
+spec:
+  template:
+    spec:
+`)
+			local := create(`
+apiVersion: apps/v1beta1
+kind: Deployment
+spec:
+  template:
+    spec:
+      volumes:
+      - name: cache-volume
+        emptyDir:
+`)
+			remote := create(`
+apiVersion: apps/v1beta1
+kind: Deployment
+spec:
+  template:
+    spec:
+      volumes:
+      - name: cache-volume
+        hostPath:
+          path: /tmp/cache-volume
+`)
+			expected := create(`
+apiVersion: apps/v1beta1
+kind: Deployment
+spec:
+  template:
+    spec:
+      volumes:
+      - name: cache-volume
+        emptyDir:
+`)
+			run(strategy.Create(strategy.Options{}), recorded, local, remote, expected)
+		})
+	})
+
+	Context("where the elements are in a list", func() {
+		It("the fields defined both locally and remotely should be merged", func() {
+			recorded := create(`
+apiVersion: apps/v1beta1
+kind: Deployment
+spec:
+  template:
+    spec:
+`)
+			local := create(`
+apiVersion: apps/v1beta1
+kind: Deployment
+spec:
+  template:
+    spec:
+      volumes:
+      - name: cache-volume
+        hostPath:
+          path: /tmp/cache-volume
+        emptyDir:
+`)
+			remote := create(`
+apiVersion: apps/v1beta1
+kind: Deployment
+spec:
+  template:
+    spec:
+      volumes:
+      - name: cache-volume
+        hostPath:
+          path: /tmp/cache-volume
+          type: Directory
+`)
+			expected := create(`
+apiVersion: apps/v1beta1
+kind: Deployment
+spec:
+  template:
+    spec:
+      volumes:
+      - name: cache-volume
+        hostPath:
+          path: /tmp/cache-volume
+          type: Directory
+        emptyDir:
+`)
+			run(strategy.Create(strategy.Options{}), recorded, local, remote, expected)
+		})
+	})
+})

pkg/kubectl/apply/strategy/retain_keys_visitor.go

@@ -16,4 +16,62 @@ limitations under the License.
 
 package strategy
 
-// TODO: Write this
+import (
+	"fmt"
+	"k8s.io/kubernetes/pkg/kubectl/apply"
+)
+
+func createRetainKeysStrategy(options Options, strategic *delegatingStrategy) retainKeysStrategy {
+	return retainKeysStrategy{
+		&mergeStrategy{strategic, options},
+		strategic,
+		options,
+	}
+}
+
+// retainKeysStrategy merges the values in an Element into a single Result,
+// dropping any fields omitted from the local copy. (but merging values when
+// defined locally and remotely)
+type retainKeysStrategy struct {
+	merge     *mergeStrategy
+	strategic *delegatingStrategy
+	options   Options
+}
+
+// MergeMap merges the type instances in a TypeElement into a single Result
+// keeping only the fields defined locally, but merging their values with
+// the remote values.
+func (v retainKeysStrategy) MergeType(e apply.TypeElement) (apply.Result, error) {
+	// No merge logic if adding or deleting a field
+	if result, done := v.merge.doAddOrDelete(&e); done {
+		return result, nil
+	}
+
+	elem := map[string]apply.Element{}
+	for key := range e.GetLocalMap() {
+		elem[key] = e.GetValues()[key]
+	}
+	return v.merge.doMergeMap(elem)
+}
+
+// MergeMap returns an error.  Only TypeElements can have retainKeys.
+func (v retainKeysStrategy) MergeMap(e apply.MapElement) (apply.Result, error) {
+	return apply.Result{}, fmt.Errorf("Cannot use retainkeys with map element %v", e.Name)
+}
+
+// MergeList returns an error.  Only TypeElements can have retainKeys.
+func (v retainKeysStrategy) MergeList(e apply.ListElement) (apply.Result, error) {
+	return apply.Result{}, fmt.Errorf("Cannot use retainkeys with list element %v", e.Name)
+}
+
+// MergePrimitive returns an error.  Only TypeElements can have retainKeys.
+func (v retainKeysStrategy) MergePrimitive(diff apply.PrimitiveElement) (apply.Result, error) {
+	return apply.Result{}, fmt.Errorf("Cannot use retainkeys with primitive element %v", diff.Name)
+}
+
+// MergeEmpty returns an empty result
+func (v retainKeysStrategy) MergeEmpty(diff apply.EmptyElement) (apply.Result, error) {
+	return v.merge.MergeEmpty(diff)
+}
+
+var _ apply.Strategy = &retainKeysStrategy{}

pkg/kubectl/apply/strategy/strategic_visitor.go

@@ -23,9 +23,10 @@ import (
 // delegatingStrategy delegates merging fields to other visitor implementations
 // based on the merge strategy preferred by the field.
 type delegatingStrategy struct {
-	options Options
+	options    Options
-	merge   mergeStrategy
+	merge      mergeStrategy
-	replace replaceStrategy
+	replace    replaceStrategy
+	retainKeys retainKeysStrategy
 }
 
 // createDelegatingStrategy returns a new delegatingStrategy
@@ -35,18 +36,20 @@ func createDelegatingStrategy(options Options) *delegatingStrategy {
 	}
 	v.replace = createReplaceStrategy(options, v)
 	v.merge = createMergeStrategy(options, v)
+	v.retainKeys = createRetainKeysStrategy(options, v)
 	return v
 }
 
 // MergeList delegates visiting a list based on the field patch strategy.
 // Defaults to "replace"
 func (v delegatingStrategy) MergeList(diff apply.ListElement) (apply.Result, error) {
-	// TODO: Support retainkeys
 	switch diff.GetFieldMergeType() {
 	case apply.MergeStrategy:
 		return v.merge.MergeList(diff)
 	case apply.ReplaceStrategy:
 		return v.replace.MergeList(diff)
+	case apply.RetainKeysStrategy:
+		return v.retainKeys.MergeList(diff)
 	default:
 		return v.replace.MergeList(diff)
 	}
@@ -55,12 +58,13 @@ func (v delegatingStrategy) MergeList(diff apply.ListElement) (apply.Result, err
 // MergeMap delegates visiting a map based on the field patch strategy.
 // Defaults to "merge"
 func (v delegatingStrategy) MergeMap(diff apply.MapElement) (apply.Result, error) {
-	// TODO: Support retainkeys
 	switch diff.GetFieldMergeType() {
 	case apply.MergeStrategy:
 		return v.merge.MergeMap(diff)
 	case apply.ReplaceStrategy:
 		return v.replace.MergeMap(diff)
+	case apply.RetainKeysStrategy:
+		return v.retainKeys.MergeMap(diff)
 	default:
 		return v.merge.MergeMap(diff)
 	}
@@ -69,12 +73,13 @@ func (v delegatingStrategy) MergeMap(diff apply.MapElement) (apply.Result, error
 // MergeType delegates visiting a map based on the field patch strategy.
 // Defaults to "merge"
 func (v delegatingStrategy) MergeType(diff apply.TypeElement) (apply.Result, error) {
-	// TODO: Support retainkeys
 	switch diff.GetFieldMergeType() {
 	case apply.MergeStrategy:
 		return v.merge.MergeType(diff)
 	case apply.ReplaceStrategy:
 		return v.replace.MergeType(diff)
+	case apply.RetainKeysStrategy:
+		return v.retainKeys.MergeType(diff)
 	default:
 		return v.merge.MergeType(diff)
 	}