Merge pull request #50296 from mengqiy/addApplyTestForReplacekeys

Automatic merge from submit-queue (batch tested with PRs 50919, 51410, 50099, 51300, 50296) Add `retainKeys` to patchStrategy for v1 Volumes and extentions/v1beta1 DeploymentStrategy Add `retainKeys` to patchStrategy for v1 Volumes and extentions/v1beta1 DeploymentStrategy. With the new value in `patchStrategy`, the patch will include an optional directive that will tell the apiserver to clear defaulted fields and update. This will resolve issue like #34292 (comment) and similar issue caused by defaulting in volume. The change is [backward compatible](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/add-new-patchStrategy-to-clear-fields-not-present-in-patch.md#version-skew). The proposal for this new patch strategy is in https://github.com/kubernetes/community/blob/master/contributors/design-proposals/add-new-patchStrategy-to-clear-fields-not-present-in-patch.md The implementation to support the new patch strategy's logic is in #44597 and has been merged in 1.7. ```release-note Add `retainKeys` to patchStrategy for v1 Volumes and extentions/v1beta1 DeploymentStrategy. ``` /assign @apelisse /assign @janetkuo for deployment change /assign @saad-ali for volume change
kubernetes · Aug 29, 2017 · 80ea31f · 80ea31f
2 parents 79d0c2d + 9b05e26
commit 80ea31f
Show file tree

Hide file tree

Showing 9 changed files with 74 additions and 6 deletions.
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
@@ -60956,7 +60956,7 @@
        "$ref": "#/definitions/io.k8s.api.core.v1.Volume"
       },
       "x-kubernetes-patch-merge-key": "name",
-      "x-kubernetes-patch-strategy": "merge"
+      "x-kubernetes-patch-strategy": "merge,retainKeys"
      }
     }
    },
@@ -62775,6 +62775,7 @@
      },
      "strategy": {
       "description": "The deployment strategy to use to replace existing pods with new ones.",
+      "x-kubernetes-patch-strategy": "retainKeys",
       "$ref": "#/definitions/io.k8s.api.extensions.v1beta1.DeploymentStrategy"
      },
      "template": {

diff --git a/federation/apis/openapi-spec/swagger.json b/federation/apis/openapi-spec/swagger.json
@@ -11237,7 +11237,7 @@
        "$ref": "#/definitions/io.k8s.api.core.v1.Volume"
       },
       "x-kubernetes-patch-merge-key": "name",
-      "x-kubernetes-patch-strategy": "merge"
+      "x-kubernetes-patch-strategy": "merge,retainKeys"
      }
     }
    },
@@ -12530,6 +12530,7 @@
      },
      "strategy": {
       "description": "The deployment strategy to use to replace existing pods with new ones.",
+      "x-kubernetes-patch-strategy": "retainKeys",
       "$ref": "#/definitions/io.k8s.api.extensions.v1beta1.DeploymentStrategy"
      },
      "template": {

diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh
@@ -932,6 +932,30 @@ run_kubectl_apply_tests() {
   kubectl delete pods test-pod "${kube_flags[@]}"
 
 
+  ## kubectl apply should be able to clear defaulted fields.
+  # Pre-Condition: no deployment exists
+  kube::test::get_object_assert deployments "{{range.items}}{{$id_field}}:{{end}}" ''
+  # Command: apply a deployment "test-deployment-retainkeys" (doesn't exist) should create this deployment
+  kubectl apply -f hack/testdata/retainKeys/deployment/deployment-before.yaml "${kube_flags[@]}"
+  # Post-Condition: deployment "test-deployment-retainkeys" created
+  kube::test::get_object_assert deployments "{{range.items}}{{$id_field}}{{end}}" 'test-deployment-retainkeys'
+  # Post-Condition: deployment "test-deployment-retainkeys" has defaulted fields
+  [[ "$(kubectl get deployments test-deployment-retainkeys -o yaml "${kube_flags[@]}" | grep RollingUpdate)" ]]
+  [[ "$(kubectl get deployments test-deployment-retainkeys -o yaml "${kube_flags[@]}" | grep maxSurge)" ]]
+  [[ "$(kubectl get deployments test-deployment-retainkeys -o yaml "${kube_flags[@]}" | grep maxUnavailable)" ]]
+  [[ "$(kubectl get deployments test-deployment-retainkeys -o yaml "${kube_flags[@]}" | grep emptyDir)" ]]
+  # Command: apply a deployment "test-deployment-retainkeys" should clear
+  # defaulted fields and successfully update the deployment
+  [[ "$(kubectl apply -f hack/testdata/retainKeys/deployment/deployment-after.yaml "${kube_flags[@]}")" ]]
+  # Post-Condition: deployment "test-deployment-retainkeys" has updated fields
+  [[ "$(kubectl get deployments test-deployment-retainkeys -o yaml "${kube_flags[@]}" | grep Recreate)" ]]
+  ! [[ "$(kubectl get deployments test-deployment-retainkeys -o yaml "${kube_flags[@]}" | grep RollingUpdate)" ]]
+  [[ "$(kubectl get deployments test-deployment-retainkeys -o yaml "${kube_flags[@]}" | grep hostPath)" ]]
+  ! [[ "$(kubectl get deployments test-deployment-retainkeys -o yaml "${kube_flags[@]}" | grep emptyDir)" ]]
+  # Clean up
+  kubectl delete deployments test-deployment-retainkeys "${kube_flags[@]}"
+
+
   ## kubectl apply -f with label selector should only apply matching objects
   # Pre-Condition: no POD exists
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''

diff --git a/hack/testdata/retainKeys/deployment/deployment-after.yaml b/hack/testdata/retainKeys/deployment/deployment-after.yaml
@@ -0,0 +1,22 @@
+apiVersion: extensions/v1beta1
+kind: Deployment
+metadata:
+  name: test-deployment-retainkeys
+spec:
+  strategy:
+    type: Recreate
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - name: nginx
+        image: nginx
+        ports:
+        - containerPort: 80
+      volumes:
+      - name: test-volume
+        hostPath:
+          path: /data
diff --git a/hack/testdata/retainKeys/deployment/deployment-before.yaml b/hack/testdata/retainKeys/deployment/deployment-before.yaml
@@ -0,0 +1,18 @@
+apiVersion: extensions/v1beta1
+kind: Deployment
+metadata:
+  name: test-deployment-retainkeys
+spec:
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - name: nginx
+        image: nginx
+        ports:
+        - containerPort: 80
+      volumes:
+      - name: test-volume
diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto
diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go
@@ -2494,8 +2494,8 @@ type PodSpec struct {
 	// More info: https://kubernetes.io/docs/concepts/storage/volumes
 	// +optional
 	// +patchMergeKey=name
-	// +patchStrategy=merge
-	Volumes []Volume `json:"volumes,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,1,rep,name=volumes"`
+	// +patchStrategy=merge,retainKeys
+	Volumes []Volume `json:"volumes,omitempty" patchStrategy:"merge,retainKeys" patchMergeKey:"name" protobuf:"bytes,1,rep,name=volumes"`
 	// List of initialization containers belonging to the pod.
 	// Init containers are executed in order prior to containers being started. If any
 	// init container fails, the pod is considered to have failed and is handled according

diff --git a/staging/src/k8s.io/api/extensions/v1beta1/generated.proto b/staging/src/k8s.io/api/extensions/v1beta1/generated.proto
diff --git a/staging/src/k8s.io/api/extensions/v1beta1/types.go b/staging/src/k8s.io/api/extensions/v1beta1/types.go
@@ -193,7 +193,8 @@ type DeploymentSpec struct {
 
 	// The deployment strategy to use to replace existing pods with new ones.
 	// +optional
-	Strategy DeploymentStrategy `json:"strategy,omitempty" protobuf:"bytes,4,opt,name=strategy"`
+	// +patchStrategy=retainKeys
+	Strategy DeploymentStrategy `json:"strategy,omitempty" patchStrategy:"retainKeys" protobuf:"bytes,4,opt,name=strategy"`
 
 	// Minimum number of seconds for which a newly created pod should be ready
 	// without any of its container crashing, for it to be considered available.