Merge pull request #20948 from ghodss/versioned-apply

Auto commit by PR queue bot

pkg/kubectl/cmd/apply.go

@@ -107,6 +107,7 @@ func RunApply(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *Ap
 	}
 
 	encoder := f.JSONEncoder()
+	decoder := f.Decoder(false)
 
 	count := 0
 	err = r.Visit(func(info *resource.Info, err error) error {
@@ -161,8 +162,18 @@ func RunApply(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *Ap
 			return cmdutil.AddSourceToErr(fmt.Sprintf("retrieving original configuration from:\n%v\nfor:", info), info.Source, err)
 		}
 
+		// Create the versioned struct from the original from the server for
+		// strategic patch.
+		// TODO: Move all structs in apply to use raw data. Can be done once
+		// builder has a RawResult method which delivers raw data instead of
+		// internal objects.
+		versionedObject, _, err := decoder.Decode(current, nil, nil)
+		if err != nil {
+			return cmdutil.AddSourceToErr(fmt.Sprintf("converting encoded server-side object back to versioned struct:\n%v\nfor:", info), info.Source, err)
+		}
+
 		// Compute a three way strategic merge patch to send to server.
-		patch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, info.VersionedObject, true)
+		patch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, versionedObject, true)
 		if err != nil {
 			format := "creating patch with:\noriginal:\n%s\nmodified:\n%s\ncurrent:\n%s\nfrom:\n%v\nfor:"
 			return cmdutil.AddSourceToErr(fmt.Sprintf(format, original, modified, current, info), info.Source, err)

pkg/kubectl/cmd/apply_test.go

@@ -52,8 +52,9 @@ func validateApplyArgs(cmd *cobra.Command, args []string) error {
 }
 
 const (
-	filenameRC  = "../../../examples/guestbook/redis-master-controller.yaml"
-	filenameSVC = "../../../examples/guestbook/frontend-service.yaml"
+	filenameRC    = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc.yaml"
+	filenameSVC   = "../../../test/fixtures/pkg/kubectl/cmd/apply/service.yaml"
+	filenameRCSVC = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-service.yaml"
 )
 
 func readBytesFromFile(t *testing.T, filename string) []byte {
@@ -245,7 +246,15 @@ func TestApplyNonExistObject(t *testing.T) {
 	}
 }
 
-func TestApplyMultipleObject(t *testing.T) {
+func TestApplyMultipleObjectsAsList(t *testing.T) {
+	testApplyMultipleObjects(t, true)
+}
+
+func TestApplyMultipleObjectsAsFiles(t *testing.T) {
+	testApplyMultipleObjects(t, false)
+}
+
+func testApplyMultipleObjects(t *testing.T, asList bool) {
 	nameRC, currentRC := readAndAnnotateReplicationController(t, filenameRC)
 	pathRC := "/namespaces/test/replicationcontrollers/" + nameRC
 
@@ -282,8 +291,12 @@ func TestApplyMultipleObject(t *testing.T) {
 	buf := bytes.NewBuffer([]byte{})
 
 	cmd := NewCmdApply(f, buf)
+	if asList {
+		cmd.Flags().Set("filename", filenameRCSVC)
+	} else {
 		cmd.Flags().Set("filename", filenameRC)
 		cmd.Flags().Set("filename", filenameSVC)
+	}
 	cmd.Flags().Set("output", "name")
 
 	cmd.Run(cmd, []string{})
@@ -291,8 +304,10 @@ func TestApplyMultipleObject(t *testing.T) {
 	// Names should come from the REST response, NOT the files
 	expectRC := "replicationcontroller/" + nameRC + "\n"
 	expectSVC := "service/" + nameSVC + "\n"
-	expect := expectRC + expectSVC
-	if buf.String() != expect {
-		t.Fatalf("unexpected output: %s\nexpected: %s", buf.String(), expect)
+	// Test both possible orders since output is non-deterministic.
+	expectOne := expectRC + expectSVC
+	expectTwo := expectSVC + expectRC
+	if buf.String() != expectOne && buf.String() != expectTwo {
+		t.Fatalf("unexpected output: %s\nexpected: %s OR %s", buf.String(), expectOne, expectTwo)
 	}
 }

test/fixtures/pkg/kubectl/cmd/apply/rc-service.yaml

@@ -0,0 +1,32 @@
+apiVersion: v1
+kind: List
+items:
+  - apiVersion: v1
+    kind: Service
+    metadata:
+      name: test-service
+      labels:
+        name: test-service
+    spec:
+      ports:
+      - port: 80
+      selector:
+        name: test-rc
+  - apiVersion: v1
+    kind: ReplicationController
+    metadata:
+      name: test-rc
+      labels:
+        name: test-rc
+    spec:
+      replicas: 1
+      template:
+        metadata:
+          labels:
+            name: test-rc
+        spec:
+          containers:
+            - name: test-rc
+              image: nginx
+              ports:
+              - containerPort: 80

test/fixtures/pkg/kubectl/cmd/apply/rc.yaml

@@ -0,0 +1,18 @@
+apiVersion: v1
+kind: ReplicationController
+metadata:
+  name: test-rc
+  labels:
+    name: test-rc
+spec:
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        name: test-rc
+    spec:
+      containers:
+        - name: test-rc
+          image: nginx
+          ports:
+          - containerPort: 80

test/fixtures/pkg/kubectl/cmd/apply/service.yaml

@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: test-service
+  labels:
+    name: test-service
+spec:
+  ports:
+  - port: 80
+  selector:
+    name: test-rc