Merge pull request #41705 from liggitt/patch-output-result

Automatic merge from submit-queue (batch tested with PRs 41597, 42185, 42075, 42178, 41705)

Honor output formats in kubectl patch

Currently, output formats other than `-o name` are only honored when in `--local` mode.

This PR also prints the result from the server when in regular mode

pkg/kubectl/cmd/patch.go

@@ -191,13 +191,19 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin
 				infoCopy.Object = patchedObj
 				infoCopy.VersionedObject = patchedObj
 				if patch, patchType, err := cmdutil.ChangeResourcePatch(&infoCopy, f.Command(cmd, true)); err == nil {
-					if _, err = helper.Patch(info.Namespace, info.Name, patchType, patch); err != nil {
+					if recordedObj, err := helper.Patch(info.Namespace, info.Name, patchType, patch); err != nil {
 						glog.V(4).Infof("error recording reason: %v", err)
+					} else {
+						patchedObj = recordedObj
 					}
 				}
 			}
 			count++
 
+			if err := info.Refresh(patchedObj, true); err != nil {
+				return err
+			}
+
 			oldData, err := json.Marshal(info.Object)
 			if err != nil {
 				return err
@@ -210,9 +216,10 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin
 				dataChangedMsg = "patched"
 			}
 
-			if options.OutputFormat == "name" || len(options.OutputFormat) == 0 {
-				cmdutil.PrintSuccess(mapper, options.OutputFormat == "name", out, info.Mapping.Resource, info.Name, false, dataChangedMsg)
+			if len(options.OutputFormat) > 0 && options.OutputFormat != "name" {
+				return cmdutil.PrintResourceInfoForCommand(cmd, info, f, out)
 			}
+			cmdutil.PrintSuccess(mapper, options.OutputFormat == "name", out, info.Mapping.Resource, info.Name, false, dataChangedMsg)
 			return nil
 		}
 
@@ -234,16 +241,10 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin
 		// rawExtension := &runtime.Unknown{
 		//	Raw: originalPatchedObjJS,
 		// }
-
-		printer, err := f.PrinterForMapping(cmd, mapping, false)
-		if err != nil {
+		if err := info.Refresh(targetObj, true); err != nil {
 			return err
 		}
-		if err := printer.PrintObj(targetObj, out); err != nil {
-			return err
-		}
-
-		return nil
+		return cmdutil.PrintResourceInfoForCommand(cmd, info, f, out)
 	})
 	if err != nil {
 		return err

pkg/kubectl/cmd/patch_test.go

@@ -19,18 +19,19 @@ package cmd
 import (
 	"bytes"
 	"net/http"
+	"strings"
 	"testing"
 
 	"k8s.io/client-go/rest/fake"
 	"k8s.io/kubernetes/pkg/api"
 	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
+	"k8s.io/kubernetes/pkg/printers"
 )
 
 func TestPatchObject(t *testing.T) {
 	_, svc, _ := testData()
 
 	f, tf, codec, _ := cmdtesting.NewAPIFactory()
-	tf.Printer = &testPrinter{}
 	tf.UnstructuredClient = &fake.RESTClient{
 		APIRegistry:          api.Registry,
 		NegotiatedSerializer: unstructuredSerializer,
@@ -53,8 +54,8 @@ func TestPatchObject(t *testing.T) {
 	cmd.Flags().Set("output", "name")
 	cmd.Run(cmd, []string{"services/frontend"})
 
-	// uses the name from the file, not the response
-	if buf.String() != "service/frontend\n" {
+	// uses the name from the response
+	if buf.String() != "service/baz\n" {
 		t.Errorf("unexpected output: %s", buf.String())
 	}
 }
@@ -63,7 +64,6 @@ func TestPatchObjectFromFile(t *testing.T) {
 	_, svc, _ := testData()
 
 	f, tf, codec, _ := cmdtesting.NewAPIFactory()
-	tf.Printer = &testPrinter{}
 	tf.UnstructuredClient = &fake.RESTClient{
 		APIRegistry:          api.Registry,
 		NegotiatedSerializer: unstructuredSerializer,
@@ -87,8 +87,56 @@ func TestPatchObjectFromFile(t *testing.T) {
 	cmd.Flags().Set("filename", "../../../examples/guestbook/frontend-service.yaml")
 	cmd.Run(cmd, []string{})
 
-	// uses the name from the file, not the response
-	if buf.String() != "service/frontend\n" {
+	// uses the name from the response
+	if buf.String() != "service/baz\n" {
+		t.Errorf("unexpected output: %s", buf.String())
+	}
+}
+
+func TestPatchObjectFromFileOutput(t *testing.T) {
+	_, svc, _ := testData()
+
+	svcCopyObj, err := api.Scheme.DeepCopy(&svc.Items[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	svcCopy := svcCopyObj.(*api.Service)
+	if svcCopy.Labels == nil {
+		svcCopy.Labels = map[string]string{}
+	}
+	svcCopy.Labels["post-patch"] = "post-patch-value"
+
+	f, tf, codec, _ := cmdtesting.NewAPIFactory()
+	tf.CommandPrinter = &printers.YAMLPrinter{}
+	tf.GenericPrinter = true
+	tf.UnstructuredClient = &fake.RESTClient{
+		APIRegistry:          api.Registry,
+		NegotiatedSerializer: unstructuredSerializer,
+		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			switch p, m := req.URL.Path, req.Method; {
+			case p == "/namespaces/test/services/frontend" && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &svc.Items[0])}, nil
+			case p == "/namespaces/test/services/frontend" && m == "PATCH":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, svcCopy)}, nil
+			default:
+				t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
+				return nil, nil
+			}
+		}),
+	}
+	tf.Namespace = "test"
+	buf := bytes.NewBuffer([]byte{})
+
+	cmd := NewCmdPatch(f, buf)
+	cmd.Flags().Set("namespace", "test")
+	cmd.Flags().Set("patch", `{"spec":{"type":"NodePort"}}`)
+	cmd.Flags().Set("output", "yaml")
+	cmd.Flags().Set("filename", "../../../examples/guestbook/frontend-service.yaml")
+	cmd.Run(cmd, []string{})
+
+	t.Log(buf.String())
+	// make sure the value returned by the server is used
+	if !strings.Contains(buf.String(), "post-patch: post-patch-value") {
 		t.Errorf("unexpected output: %s", buf.String())
 	}
 }