Merge pull request #41924 from ymqytw/change_saveConfig

Automatic merge from submit-queue (batch tested with PRs 41984, 41682, 41924, 41928)

make edit respect --save-config

New behavior:

edit:
1) the flag is set to true: it will create or update the last-applied-config annotation
2) the flag is set to false or is unspecified: NOP

replace:
1) If saveConfig is true, create or update the annotation.
2) If saveConfig is false and the local config file doesn't have the annotation, we save the annotation from the live object if there is one (Try to not surprise the users).
3) If saveConfig is false and the local config file has the annotation, we use the annotation in the config file.
4) Same behavior for force replacing

fixes #40626

```release-note
stop kubectl edit from updating the last-applied-configuration annotation when --save-config is unspecified or false.
```

@pwittrock @liggitt 

This is a bug fix that prevented `edit` from being respected by `apply`

pkg/kubectl/cmd/edit.go

@@ -85,7 +85,10 @@ var (
 		KUBE_EDITOR="nano" kubectl edit svc/docker-registry
 
 		# Edit the job 'myjob' in JSON using the v1 API format:
-		kubectl edit job.v1.batch/myjob -o json`)
+		kubectl edit job.v1.batch/myjob -o json
+
+		# Edit the deployment 'mydeployment' in YAML and save the modified config in its annotation:
+		kubectl edit deployment/mydeployment -o yaml --save-config`)
 )
 
 func NewCmdEdit(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command {
@@ -532,9 +535,11 @@ func visitToCreate(createVisitor resource.Visitor, mapper meta.RESTMapper, out,
 func visitAnnotation(cmd *cobra.Command, f cmdutil.Factory, annotationVisitor resource.Visitor, encoder runtime.Encoder) error {
 	// iterate through all items to apply annotations
 	err := annotationVisitor.Visit(func(info *resource.Info, incomingErr error) error {
-		// put configuration annotation in "updates"
-		if err := kubectl.CreateOrUpdateAnnotation(cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag), info, encoder); err != nil {
-			return err
+		// If the flag is true, create or update the annotation. Otherwise, NOP
+		if cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag) {
+			if err := kubectl.CreateOrUpdateAnnotation(true, info, encoder); err != nil {
+				return err
+			}
 		}
 		if cmdutil.ShouldRecord(cmd, info) {
 			if err := cmdutil.RecordChangeCause(info.Object, f.Command(cmd, false)); err != nil {

pkg/kubectl/cmd/edit_test.go

@@ -49,6 +49,7 @@ type EditTestCase struct {
 	Args             []string `yaml:"args"`
 	Filename         string   `yaml:"filename"`
 	Output           string   `yaml:"outputFormat"`
+	SaveConfig       string   `yaml:"saveConfig"`
 	Namespace        string   `yaml:"namespace"`
 	ExpectedStdout   []string `yaml:"expectedStdout"`
 	ExpectedStderr   []string `yaml:"expectedStderr"`
@@ -247,6 +248,9 @@ func TestEdit(t *testing.T) {
 		if len(testcase.Output) > 0 {
 			cmd.Flags().Set("output", testcase.Output)
 		}
+		if len(testcase.SaveConfig) > 0 {
+			cmd.Flags().Set("save-config", testcase.SaveConfig)
+		}
 
 		cmdutil.BehaviorOnFatal(func(str string, code int) {
 			errBuf.WriteString(str)

pkg/kubectl/cmd/testdata/edit/testcase-not-update-annotation/0.response

@@ -0,0 +1,38 @@
+{
+  "apiVersion": "v1",
+  "kind": "Service",
+  "metadata": {
+    "annotations": {
+      "kubectl.kubernetes.io/last-applied-configuration": "{\"kind\":\"Service\",\"apiVersion\":\"v1\",\"metadata\":{\"name\":\"svc1\",\"creationTimestamp\":null,\"labels\":{\"app\":\"svc1\"}},\"spec\":{\"ports\":[{\"name\":\"80\",\"protocol\":\"TCP\",\"port\":80,\"targetPort\":80}],\"selector\":{\"app\":\"svc1\"},\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
+    },
+    "creationTimestamp": "2017-02-27T19:40:53Z",
+    "labels": {
+      "app": "svc1"
+    },
+    "name": "svc1",
+    "namespace": "edit-test",
+    "resourceVersion": "670",
+    "selfLink": "/api/v1/namespaces/edit-test/services/svc1",
+    "uid": "a6c11186-fd24-11e6-b53c-480fcf4a5275"
+  },
+  "spec": {
+    "clusterIP": "10.0.0.204",
+    "ports": [
+      {
+        "name": "80",
+        "port": 80,
+        "protocol": "TCP",
+        "targetPort": 80
+      }
+    ],
+    "selector": {
+      "app": "svc1"
+    },
+    "sessionAffinity": "None",
+    "type": "ClusterIP"
+  },
+  "status": {
+    "loadBalancer": {}
+  }
+}
+

pkg/kubectl/cmd/testdata/edit/testcase-not-update-annotation/1.edited

@@ -0,0 +1,32 @@
+# Please edit the object below. Lines beginning with a '#' will be ignored,
+# and an empty file will abort the edit. If an error occurs while saving this file will be
+# reopened with the relevant failures.
+#
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    kubectl.kubernetes.io/last-applied-configuration: |
+      {"kind":"Service","apiVersion":"v1","metadata":{"name":"svc1","creationTimestamp":null,"labels":{"app":"svc1"}},"spec":{"ports":[{"name":"80","protocol":"TCP","port":80,"targetPort":80}],"selector":{"app":"svc1"},"type":"ClusterIP"},"status":{"loadBalancer":{}}}
+  creationTimestamp: 2017-02-27T19:40:53Z
+  labels:
+    app: svc1
+    new-label: new-value
+  name: svc1
+  namespace: edit-test
+  resourceVersion: "670"
+  selfLink: /api/v1/namespaces/edit-test/services/svc1
+  uid: a6c11186-fd24-11e6-b53c-480fcf4a5275
+spec:
+  clusterIP: 10.0.0.204
+  ports:
+  - name: "80"
+    port: 80
+    protocol: TCP
+    targetPort: 80
+  selector:
+    app: svc1
+  sessionAffinity: None
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-not-update-annotation/1.original

@@ -0,0 +1,31 @@
+# Please edit the object below. Lines beginning with a '#' will be ignored,
+# and an empty file will abort the edit. If an error occurs while saving this file will be
+# reopened with the relevant failures.
+#
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    kubectl.kubernetes.io/last-applied-configuration: |
+      {"kind":"Service","apiVersion":"v1","metadata":{"name":"svc1","creationTimestamp":null,"labels":{"app":"svc1"}},"spec":{"ports":[{"name":"80","protocol":"TCP","port":80,"targetPort":80}],"selector":{"app":"svc1"},"type":"ClusterIP"},"status":{"loadBalancer":{}}}
+  creationTimestamp: 2017-02-27T19:40:53Z
+  labels:
+    app: svc1
+  name: svc1
+  namespace: edit-test
+  resourceVersion: "670"
+  selfLink: /api/v1/namespaces/edit-test/services/svc1
+  uid: a6c11186-fd24-11e6-b53c-480fcf4a5275
+spec:
+  clusterIP: 10.0.0.204
+  ports:
+  - name: "80"
+    port: 80
+    protocol: TCP
+    targetPort: 80
+  selector:
+    app: svc1
+  sessionAffinity: None
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-not-update-annotation/2.request

@@ -0,0 +1,7 @@
+{
+  "metadata": {
+    "labels": {
+      "new-label": "new-value"
+    }
+  }
+}

pkg/kubectl/cmd/testdata/edit/testcase-not-update-annotation/2.response

@@ -0,0 +1,38 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+	  "annotations": {
+      "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"creationTimestamp\":\"2017-02-27T19:40:53Z\",\"labels\":{\"app\":\"svc1\"},\"name\":\"svc1\",\"namespace\":\"edit-test\",\"resourceVersion\":\"670\",\"selfLink\":\"/api/v1/namespaces/edit-test/services/svc1\",\"uid\":\"a6c11186-fd24-11e6-b53c-480fcf4a5275\"},\"spec\":{\"clusterIP\":\"10.0.0.204\",\"ports\":[{\"name\":\"80\",\"port\":80,\"protocol\":\"TCP\",\"targetPort\":80}],\"selector\":{\"app\":\"svc1\"},\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
+    },
+		"name": "svc1",
+		"namespace": "edit-test",
+		"selfLink": "/api/v1/namespaces/edit-test/services/svc1",
+		"uid": "a6c11186-fd24-11e6-b53c-480fcf4a5275",
+		"resourceVersion":"1045",
+		"creationTimestamp":"2017-02-27T19:40:53Z",
+		"labels": {
+			"app": "svc1",
+			"new-label": "new-value"
+		}
+	},
+  "spec": {
+    "clusterIP": "10.0.0.204",
+    "ports": [
+      {
+        "name": "80",
+        "port": 80,
+        "protocol": "TCP",
+        "targetPort": 80
+      }
+    ],
+    "selector": {
+      "app": "svc1"
+    },
+    "sessionAffinity": "None",
+    "type": "ClusterIP"
+  },
+  "status": {
+    "loadBalancer": {}
+  }
+}

pkg/kubectl/cmd/testdata/edit/testcase-not-update-annotation/test.yaml

@@ -0,0 +1,30 @@
+# kubectl create namespace edit-test
+# kubectl create service clusterip svc1 --tcp 80 --namespace=edit-test --save-config
+# kubectl edit service svc1 --namespace=edit-test --save-config=false
+description: edit with flag --save-config=false should not update the annotation
+mode: edit
+args:
+- service
+- svc1
+saveConfig: "false"
+namespace: edit-test
+expectedStdout:
+- service "svc1" edited
+expectedExitCode: 0
+steps:
+- type: request
+  expectedMethod: GET
+  expectedPath: /api/v1/namespaces/edit-test/services/svc1
+  expectedInput: 0.request
+  resultingStatusCode: 200
+  resultingOutput: 0.response
+- type: edit
+  expectedInput: 1.original
+  resultingOutput: 1.edited
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /api/v1/namespaces/edit-test/services/svc1
+  expectedContentType: application/strategic-merge-patch+json
+  expectedInput: 2.request
+  resultingStatusCode: 200
+  resultingOutput: 2.response

pkg/kubectl/cmd/testdata/edit/testcase-update-annotation/0.response

@@ -0,0 +1,38 @@
+{
+  "apiVersion": "v1",
+  "kind": "Service",
+  "metadata": {
+    "annotations": {
+      "kubectl.kubernetes.io/last-applied-configuration": "{\"kind\":\"Service\",\"apiVersion\":\"v1\",\"metadata\":{\"name\":\"svc1\",\"creationTimestamp\":null,\"labels\":{\"app\":\"svc1\"}},\"spec\":{\"ports\":[{\"name\":\"80\",\"protocol\":\"TCP\",\"port\":80,\"targetPort\":80}],\"selector\":{\"app\":\"svc1\"},\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
+    },
+    "creationTimestamp": "2017-02-27T19:40:53Z",
+    "labels": {
+      "app": "svc1"
+    },
+    "name": "svc1",
+    "namespace": "edit-test",
+    "resourceVersion": "670",
+    "selfLink": "/api/v1/namespaces/edit-test/services/svc1",
+    "uid": "a6c11186-fd24-11e6-b53c-480fcf4a5275"
+  },
+  "spec": {
+    "clusterIP": "10.0.0.204",
+    "ports": [
+      {
+        "name": "80",
+        "port": 80,
+        "protocol": "TCP",
+        "targetPort": 80
+      }
+    ],
+    "selector": {
+      "app": "svc1"
+    },
+    "sessionAffinity": "None",
+    "type": "ClusterIP"
+  },
+  "status": {
+    "loadBalancer": {}
+  }
+}
+

pkg/kubectl/cmd/testdata/edit/testcase-update-annotation/1.edited

@@ -0,0 +1,32 @@
+# Please edit the object below. Lines beginning with a '#' will be ignored,
+# and an empty file will abort the edit. If an error occurs while saving this file will be
+# reopened with the relevant failures.
+#
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    kubectl.kubernetes.io/last-applied-configuration: |
+      {"kind":"Service","apiVersion":"v1","metadata":{"name":"svc1","creationTimestamp":null,"labels":{"app":"svc1"}},"spec":{"ports":[{"name":"80","protocol":"TCP","port":80,"targetPort":80}],"selector":{"app":"svc1"},"type":"ClusterIP"},"status":{"loadBalancer":{}}}
+  creationTimestamp: 2017-02-27T19:40:53Z
+  labels:
+    app: svc1
+    new-label: new-value
+  name: svc1
+  namespace: edit-test
+  resourceVersion: "670"
+  selfLink: /api/v1/namespaces/edit-test/services/svc1
+  uid: a6c11186-fd24-11e6-b53c-480fcf4a5275
+spec:
+  clusterIP: 10.0.0.204
+  ports:
+  - name: "80"
+    port: 80
+    protocol: TCP
+    targetPort: 80
+  selector:
+    app: svc1
+  sessionAffinity: None
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-update-annotation/1.original

@@ -0,0 +1,31 @@
+# Please edit the object below. Lines beginning with a '#' will be ignored,
+# and an empty file will abort the edit. If an error occurs while saving this file will be
+# reopened with the relevant failures.
+#
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    kubectl.kubernetes.io/last-applied-configuration: |
+      {"kind":"Service","apiVersion":"v1","metadata":{"name":"svc1","creationTimestamp":null,"labels":{"app":"svc1"}},"spec":{"ports":[{"name":"80","protocol":"TCP","port":80,"targetPort":80}],"selector":{"app":"svc1"},"type":"ClusterIP"},"status":{"loadBalancer":{}}}
+  creationTimestamp: 2017-02-27T19:40:53Z
+  labels:
+    app: svc1
+  name: svc1
+  namespace: edit-test
+  resourceVersion: "670"
+  selfLink: /api/v1/namespaces/edit-test/services/svc1
+  uid: a6c11186-fd24-11e6-b53c-480fcf4a5275
+spec:
+  clusterIP: 10.0.0.204
+  ports:
+  - name: "80"
+    port: 80
+    protocol: TCP
+    targetPort: 80
+  selector:
+    app: svc1
+  sessionAffinity: None
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-update-annotation/2.request

@@ -0,0 +1,10 @@
+{
+  "metadata": {
+    "annotations": {
+      "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"creationTimestamp\":\"2017-02-27T19:40:53Z\",\"labels\":{\"app\":\"svc1\",\"new-label\":\"new-value\"},\"name\":\"svc1\",\"namespace\":\"edit-test\",\"resourceVersion\":\"670\",\"selfLink\":\"/api/v1/namespaces/edit-test/services/svc1\",\"uid\":\"a6c11186-fd24-11e6-b53c-480fcf4a5275\"},\"spec\":{\"clusterIP\":\"10.0.0.204\",\"ports\":[{\"name\":\"80\",\"port\":80,\"protocol\":\"TCP\",\"targetPort\":80}],\"selector\":{\"app\":\"svc1\"},\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
+    },
+    "labels": {
+      "new-label": "new-value"
+    }
+  }
+}

pkg/kubectl/cmd/testdata/edit/testcase-update-annotation/2.response

@@ -0,0 +1,38 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+	  "annotations": {
+      "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"creationTimestamp\":\"2017-02-27T19:40:53Z\",\"labels\":{\"app\":\"svc1\",\"new-label\":\"new-value\"},\"name\":\"svc1\",\"namespace\":\"edit-test\",\"resourceVersion\":\"670\",\"selfLink\":\"/api/v1/namespaces/edit-test/services/svc1\",\"uid\":\"a6c11186-fd24-11e6-b53c-480fcf4a5275\"},\"spec\":{\"clusterIP\":\"10.0.0.204\",\"ports\":[{\"name\":\"80\",\"port\":80,\"protocol\":\"TCP\",\"targetPort\":80}],\"selector\":{\"app\":\"svc1\"},\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
+    },
+		"name": "svc1",
+		"namespace": "edit-test",
+		"selfLink": "/api/v1/namespaces/edit-test/services/svc1",
+		"uid": "a6c11186-fd24-11e6-b53c-480fcf4a5275",
+		"resourceVersion":"1045",
+		"creationTimestamp":"2017-02-27T19:40:53Z",
+		"labels": {
+			"app": "svc1",
+			"new-label": "new-value"
+		}
+	},
+  "spec": {
+    "clusterIP": "10.0.0.204",
+    "ports": [
+      {
+        "name": "80",
+        "port": 80,
+        "protocol": "TCP",
+        "targetPort": 80
+      }
+    ],
+    "selector": {
+      "app": "svc1"
+    },
+    "sessionAffinity": "None",
+    "type": "ClusterIP"
+  },
+  "status": {
+    "loadBalancer": {}
+  }
+}

pkg/kubectl/cmd/testdata/edit/testcase-update-annotation/test.yaml

@@ -0,0 +1,30 @@
+# kubectl create namespace edit-test
+# kubectl create service clusterip svc1 --tcp 80 --namespace=edit-test --save-config
+# kubectl edit service svc1 --namespace=edit-test --save-config=true
+description: edit with flag --save-config=true should update the annotation
+mode: edit
+args:
+- service
+- svc1
+saveConfig: "true"
+namespace: edit-test
+expectedStdout:
+- service "svc1" edited
+expectedExitCode: 0
+steps:
+- type: request
+  expectedMethod: GET
+  expectedPath: /api/v1/namespaces/edit-test/services/svc1
+  expectedInput: 0.request
+  resultingStatusCode: 200
+  resultingOutput: 0.response
+- type: edit
+  expectedInput: 1.original
+  resultingOutput: 1.edited
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /api/v1/namespaces/edit-test/services/svc1
+  expectedContentType: application/strategic-merge-patch+json
+  expectedInput: 2.request
+  resultingStatusCode: 200
+  resultingOutput: 2.response

pkg/kubectl/cmd/util/helpers.go

@@ -404,7 +404,7 @@ func AddDryRunFlag(cmd *cobra.Command) {
 }
 
 func AddApplyAnnotationFlags(cmd *cobra.Command) {
-	cmd.Flags().Bool(ApplyAnnotationsFlag, false, "If true, the configuration of current object will be saved in its annotation. This is useful when you want to perform kubectl apply on this object in the future.")
+	cmd.Flags().Bool(ApplyAnnotationsFlag, false, "If true, the configuration of current object will be saved in its annotation. Otherwise, the annotation will be unchanged. This flag is useful when you want to perform kubectl apply on this object in the future.")
 }
 
 // AddGeneratorFlags adds flags common to resource generation commands