added previously failing unit test and fix using pkg/util/merge.go instead of mergo to apply patch.

pkg/kubectl/cmd/expose.go

@@ -85,7 +85,8 @@ Examples:
 
 			inline := util.GetFlagString(cmd, "overrides")
 			if len(inline) > 0 {
-				util.Merge(service, inline, "Service")
+				service, err = util.Merge(service, inline, "Service")
+				checkErr(err)
 			}
 
 			// TODO: extract this flag to a central location, when such a location exists.

pkg/kubectl/cmd/run.go

@@ -74,7 +74,8 @@ Examples:
 
 			inline := util.GetFlagString(cmd, "overrides")
 			if len(inline) > 0 {
-				util.Merge(controller, inline, "ReplicationController")
+				controller, err = util.Merge(controller, inline, "ReplicationController")
+				checkErr(err)
 			}
 
 			// TODO: extract this flag to a central location, when such a location exists.

pkg/kubectl/cmd/update.go

@@ -116,12 +116,13 @@ func updateWithPatch(cmd *cobra.Command, args []string, f *Factory, patch string
 	obj, err := helper.Get(namespace, name)
 	checkErr(err)
 
-	cmdutil.Merge(obj, patch, mapping.Kind)
+	patchedObj, err := cmdutil.Merge(obj, patch, mapping.Kind)
-
-	data, err := helper.Codec.Encode(obj)
 	checkErr(err)
 
-	obj, err = helper.Update(namespace, name, true, data)
+	data, err := helper.Codec.Encode(patchedObj)
+	checkErr(err)
+
+	_, err = helper.Update(namespace, name, true, data)
 	checkErr(err)
 	return name
 }

pkg/kubectl/cmd/util/helpers.go

@@ -29,8 +29,8 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/merge"
 	"github.com/golang/glog"
-	"github.com/imdario/mergo"
 	"github.com/spf13/cobra"
 )
 
@@ -188,36 +188,40 @@ func ReadConfigDataFromLocation(location string) ([]byte, error) {
 	}
 }
 
-func Merge(dst runtime.Object, fragment, kind string) error {
+func Merge(dst runtime.Object, fragment, kind string) (runtime.Object, error) {
 	// Ok, this is a little hairy, we'd rather not force the user to specify a kind for their JSON
 	// So we pull it into a map, add the Kind field, and then reserialize.
 	// We also pull the apiVersion for proper parsing
 	var intermediate interface{}
 	if err := json.Unmarshal([]byte(fragment), &intermediate); err != nil {
-		return err
+		return nil, err
 	}
 	dataMap, ok := intermediate.(map[string]interface{})
 	if !ok {
-		return fmt.Errorf("Expected a map, found something else: %s", fragment)
+		return nil, fmt.Errorf("Expected a map, found something else: %s", fragment)
 	}
 	version, found := dataMap["apiVersion"]
 	if !found {
-		return fmt.Errorf("Inline JSON requires an apiVersion field")
+		return nil, fmt.Errorf("Inline JSON requires an apiVersion field")
 	}
 	versionString, ok := version.(string)
 	if !ok {
-		return fmt.Errorf("apiVersion must be a string")
+		return nil, fmt.Errorf("apiVersion must be a string")
 	}
-	codec := runtime.CodecFor(api.Scheme, versionString)
 
-	dataMap["kind"] = kind
+	codec := runtime.CodecFor(api.Scheme, versionString)
-	data, err := json.Marshal(intermediate)
+	// encode dst into versioned json and apply fragment directly too it
+	target, err := codec.Encode(dst)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	src, err := codec.Decode(data)
+	patched, err := merge.MergeJSON(target, []byte(fragment))
 	if err != nil {
-		return err
+		return nil, err
 	}
-	return mergo.Merge(dst, src)
+	out, err := codec.Decode(patched)
+	if err != nil {
+		return nil, err
+	}
+	return out, nil
 }

pkg/kubectl/cmd/util/helpers_test.go

@@ -30,14 +30,16 @@ func TestMerge(t *testing.T) {
 		fragment  string
 		expected  runtime.Object
 		expectErr bool
+		kind      string
 	}{
 		{
+			kind: "Pod",
 			obj: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Name: "foo",
 				},
 			},
-			fragment: "{ \"apiVersion\": \"v1beta1\" }",
+			fragment: `{ "apiVersion": "v1beta1" }`,
 			expected: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Name: "foo",
@@ -51,12 +53,13 @@ func TestMerge(t *testing.T) {
 			},
 		},
 		{
+			kind: "Pod",
 			obj: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Name: "foo",
 				},
 			},
-			fragment: "{ \"apiVersion\": \"v1beta1\", \"id\": \"baz\", \"desiredState\": { \"host\": \"bar\" } }",
+			fragment: `{ "apiVersion": "v1beta1", "id": "baz", "desiredState": { "host": "bar" } }`,
 			expected: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Name: "baz",
@@ -71,12 +74,13 @@ func TestMerge(t *testing.T) {
 			},
 		},
 		{
+			kind: "Pod",
 			obj: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Name: "foo",
 				},
 			},
-			fragment: "{ \"apiVersion\": \"v1beta3\", \"spec\": { \"volumes\": [ {\"name\": \"v1\"}, {\"name\": \"v2\"} ] } }",
+			fragment: `{ "apiVersion": "v1beta3", "spec": { "volumes": [ {"name": "v1"}, {"name": "v2"} ] } }`,
 			expected: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Name: "foo",
@@ -100,24 +104,81 @@ func TestMerge(t *testing.T) {
 			},
 		},
 		{
+			kind:      "Pod",
 			obj:       &api.Pod{},
 			fragment:  "invalid json",
 			expected:  &api.Pod{},
 			expectErr: true,
 		},
+		{
+			kind: "Pod",
+			obj: &api.Pod{
+				ObjectMeta: api.ObjectMeta{
+					Name: "foo",
+				},
+			},
+			fragment: `{ "apiVersion": "v1beta1", "id": null}`,
+			expected: &api.Pod{
+				ObjectMeta: api.ObjectMeta{
+					Name: "",
+				},
+				Spec: api.PodSpec{
+					RestartPolicy: api.RestartPolicy{
+						Always: &api.RestartPolicyAlways{},
+					},
+					DNSPolicy: api.DNSClusterFirst,
+				},
+			},
+		},
+		{
+			kind: "Service",
+			obj: &api.Service{
+				Spec: api.ServiceSpec{
+					Port: 10,
+				},
+			},
+			fragment: `{ "apiVersion": "v1beta1", "port": 0 }`,
+			expected: &api.Service{
+				Spec: api.ServiceSpec{
+					Port:            0,
+					Protocol:        "TCP",
+					SessionAffinity: "None",
+				},
+			},
+		},
+		{
+			kind: "Service",
+			obj: &api.Service{
+				Spec: api.ServiceSpec{
+					Selector: map[string]string{
+						"version": "v1",
+					},
+				},
+			},
+			fragment: `{ "apiVersion": "v1beta1", "selector": { "version": "v2" } }`,
+			expected: &api.Service{
+				Spec: api.ServiceSpec{
+					Protocol:        "TCP",
+					SessionAffinity: "None",
+					Selector: map[string]string{
+						"version": "v2",
+					},
+				},
+			},
+		},
 	}
 
 	for i, test := range tests {
-		err := Merge(test.obj, test.fragment, "Pod")
+		out, err := Merge(test.obj, test.fragment, test.kind)
 		if !test.expectErr {
 			if err != nil {
-				t.Errorf("unexpected error: %v", err)
+				t.Errorf("testcase[%d], unexpected error: %v", i, err)
-			} else if !reflect.DeepEqual(test.obj, test.expected) {
+			} else if !reflect.DeepEqual(out, test.expected) {
-				t.Errorf("\n\ntestcase[%d]\nexpected:\n%v\nsaw:\n%v", i, test.expected, test.obj)
+				t.Errorf("\n\ntestcase[%d]\nexpected:\n%+v\nsaw:\n%+v", i, test.expected, out)
 			}
 		}
 		if test.expectErr && err == nil {
-			t.Errorf("unexpected non-error")
+			t.Errorf("testcase[%d], unexpected non-error", i)
 		}
 	}