Commit 80ea31fc authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

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 https://github.com/kubernetes/kubernetes/issues/34292#issue-181572469 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
parents 79d0c2d5 9b05e264
...@@ -60956,7 +60956,7 @@ ...@@ -60956,7 +60956,7 @@
"$ref": "#/definitions/io.k8s.api.core.v1.Volume" "$ref": "#/definitions/io.k8s.api.core.v1.Volume"
}, },
"x-kubernetes-patch-merge-key": "name", "x-kubernetes-patch-merge-key": "name",
"x-kubernetes-patch-strategy": "merge" "x-kubernetes-patch-strategy": "merge,retainKeys"
} }
} }
}, },
...@@ -62775,6 +62775,7 @@ ...@@ -62775,6 +62775,7 @@
}, },
"strategy": { "strategy": {
"description": "The deployment strategy to use to replace existing pods with new ones.", "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" "$ref": "#/definitions/io.k8s.api.extensions.v1beta1.DeploymentStrategy"
}, },
"template": { "template": {
...@@ -11237,7 +11237,7 @@ ...@@ -11237,7 +11237,7 @@
"$ref": "#/definitions/io.k8s.api.core.v1.Volume" "$ref": "#/definitions/io.k8s.api.core.v1.Volume"
}, },
"x-kubernetes-patch-merge-key": "name", "x-kubernetes-patch-merge-key": "name",
"x-kubernetes-patch-strategy": "merge" "x-kubernetes-patch-strategy": "merge,retainKeys"
} }
} }
}, },
...@@ -12530,6 +12530,7 @@ ...@@ -12530,6 +12530,7 @@
}, },
"strategy": { "strategy": {
"description": "The deployment strategy to use to replace existing pods with new ones.", "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" "$ref": "#/definitions/io.k8s.api.extensions.v1beta1.DeploymentStrategy"
}, },
"template": { "template": {
......
...@@ -932,6 +932,30 @@ run_kubectl_apply_tests() { ...@@ -932,6 +932,30 @@ run_kubectl_apply_tests() {
kubectl delete pods test-pod "${kube_flags[@]}" 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 ## kubectl apply -f with label selector should only apply matching objects
# Pre-Condition: no POD exists # Pre-Condition: no POD exists
kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
......
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
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
...@@ -2720,7 +2720,7 @@ message PodSpec { ...@@ -2720,7 +2720,7 @@ message PodSpec {
// More info: https://kubernetes.io/docs/concepts/storage/volumes // More info: https://kubernetes.io/docs/concepts/storage/volumes
// +optional // +optional
// +patchMergeKey=name // +patchMergeKey=name
// +patchStrategy=merge // +patchStrategy=merge,retainKeys
repeated Volume volumes = 1; repeated Volume volumes = 1;
// List of initialization containers belonging to the pod. // List of initialization containers belonging to the pod.
......
...@@ -2494,8 +2494,8 @@ type PodSpec struct { ...@@ -2494,8 +2494,8 @@ type PodSpec struct {
// More info: https://kubernetes.io/docs/concepts/storage/volumes // More info: https://kubernetes.io/docs/concepts/storage/volumes
// +optional // +optional
// +patchMergeKey=name // +patchMergeKey=name
// +patchStrategy=merge // +patchStrategy=merge,retainKeys
Volumes []Volume `json:"volumes,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,1,rep,name=volumes"` Volumes []Volume `json:"volumes,omitempty" patchStrategy:"merge,retainKeys" patchMergeKey:"name" protobuf:"bytes,1,rep,name=volumes"`
// List of initialization containers belonging to the pod. // List of initialization containers belonging to the pod.
// Init containers are executed in order prior to containers being started. If any // 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 // init container fails, the pod is considered to have failed and is handled according
......
...@@ -276,6 +276,7 @@ message DeploymentSpec { ...@@ -276,6 +276,7 @@ message DeploymentSpec {
// The deployment strategy to use to replace existing pods with new ones. // The deployment strategy to use to replace existing pods with new ones.
// +optional // +optional
// +patchStrategy=retainKeys
optional DeploymentStrategy strategy = 4; optional DeploymentStrategy strategy = 4;
// Minimum number of seconds for which a newly created pod should be ready // Minimum number of seconds for which a newly created pod should be ready
......
...@@ -193,7 +193,8 @@ type DeploymentSpec struct { ...@@ -193,7 +193,8 @@ type DeploymentSpec struct {
// The deployment strategy to use to replace existing pods with new ones. // The deployment strategy to use to replace existing pods with new ones.
// +optional // +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 // 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. // without any of its container crashing, for it to be considered available.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment