diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index fb57f9bf7fe..cbc7a0b89d1 100755 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -1156,14 +1156,26 @@ run_kubectl_apply_deployments_tests() { kubectl apply -f hack/testdata/deployment-label-change1.yaml "${kube_flags[@]}" # check right deployment exists kube::test::get_object_assert 'deployment nginx' "{{${id_field}}}" 'nginx' - # apply deployment with wrong labels mismatch selector throws errors + # apply deployment with new labels and a conflicting resourceVersion output_message=$(! kubectl apply -f hack/testdata/deployment-label-change2.yaml 2>&1 "${kube_flags[@]}") - kube::test::if_has_string "${output_message}" 'Invalid value' - # apply deployment with --force and --overwrite will success + kube::test::if_has_string "${output_message}" 'Error from server (Conflict)' + # apply deployment with --force and --overwrite will succeed kubectl apply -f hack/testdata/deployment-label-change2.yaml --overwrite=true --force=true --grace-period=10 # check the changed deployment output_message=$(kubectl apply view-last-applied deploy/nginx -o json 2>&1 "${kube_flags[@]}" |grep nginx2) kube::test::if_has_string "${output_message}" '"name": "nginx2"' + # applying a resource (with --force) that is both conflicting and invalid will + # cause the server to only return a "Conflict" error when we attempt to patch. + # This means that we will delete the existing resource after receiving 5 conflict + # errors in a row from the server, and will attempt to create the modified + # resource that we are passing to "apply". Since the modified resource is also + # invalid, we will receive an invalid error when we attempt to create it, after + # having deleted the old resource. Ensure that when this case is reached, the + # old resource is restored once again, and the validation error is printed. + output_message=$(! kubectl apply -f hack/testdata/deployment-label-change3.yaml --force 2>&1 "${kube_flags[@]}") + kube::test::if_has_string "${output_message}" 'Invalid value' + # Ensure that the old object has been restored + kube::test::get_object_assert 'deployment nginx' "{{${template_labels}}}" 'nginx2' # cleanup kubectl delete deployments --all --grace-period=10 @@ -4590,6 +4602,7 @@ runTests() { hpa_min_field=".spec.minReplicas" hpa_max_field=".spec.maxReplicas" hpa_cpu_field=".spec.targetCPUUtilizationPercentage" + template_labels=".spec.template.metadata.labels.name" statefulset_replicas_field=".spec.replicas" statefulset_observed_generation=".status.observedGeneration" job_parallelism_field=".spec.parallelism" diff --git a/hack/testdata/deployment-label-change2.yaml b/hack/testdata/deployment-label-change2.yaml index 7f551d32112..9e465bdb6be 100644 --- a/hack/testdata/deployment-label-change2.yaml +++ b/hack/testdata/deployment-label-change2.yaml @@ -2,9 +2,13 @@ apiVersion: extensions/v1beta1 kind: Deployment metadata: name: nginx + resourceVersion: "99" labels: name: nginx spec: + selector: + matchLabels: + name: nginx2 replicas: 3 template: metadata: diff --git a/hack/testdata/deployment-label-change3.yaml b/hack/testdata/deployment-label-change3.yaml new file mode 100644 index 00000000000..15ce288dcdd --- /dev/null +++ b/hack/testdata/deployment-label-change3.yaml @@ -0,0 +1,22 @@ +apiVersion: extensions/v1beta1 +kind: Deployment +metadata: + name: nginx + resourceVersion: "99" + labels: + name: nginx +spec: + selector: + matchLabels: + name: invalid + replicas: 3 + template: + metadata: + labels: + name: nginx3 + spec: + containers: + - name: nginx + image: gcr.io/google-containers/nginx:test-cmd + ports: + - containerPort: 80 diff --git a/pkg/kubectl/cmd/apply.go b/pkg/kubectl/cmd/apply.go index 9bb6d194f81..0c8101266b4 100644 --- a/pkg/kubectl/cmd/apply.go +++ b/pkg/kubectl/cmd/apply.go @@ -675,13 +675,13 @@ func (p *patcher) patch(current runtime.Object, modified []byte, source, namespa } patchBytes, patchObject, err = p.patchSimple(current, modified, source, namespace, name, errOut) } - if err != nil && p.force { - patchBytes, patchObject, err = p.deleteAndCreate(modified, namespace, name) + if err != nil && errors.IsConflict(err) && p.force { + patchBytes, patchObject, err = p.deleteAndCreate(current, modified, namespace, name) } return patchBytes, patchObject, err } -func (p *patcher) deleteAndCreate(modified []byte, namespace, name string) ([]byte, runtime.Object, error) { +func (p *patcher) deleteAndCreate(original runtime.Object, modified []byte, namespace, name string) ([]byte, runtime.Object, error) { err := p.delete(namespace, name) if err != nil { return modified, nil, err @@ -700,5 +700,15 @@ func (p *patcher) deleteAndCreate(modified []byte, namespace, name string) ([]by return modified, nil, err } createdObject, err := p.helper.Create(namespace, true, versionedObject) + if err != nil { + // restore the original object if we fail to create the new one + // but still propagate and advertise error to user + recreated, recreateErr := p.helper.Create(namespace, true, original) + if recreateErr != nil { + err = fmt.Errorf("An error ocurred force-replacing the existing object with the newly provided one:\n\n%v.\n\nAdditionally, an error ocurred attempting to restore the original object:\n\n%v\n", err, recreateErr) + } else { + createdObject = recreated + } + } return modified, createdObject, err }