fix apply --force w/ invalid AND conflicting resource

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"

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:

hack/testdata/deployment-label-change3.yaml

+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

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
 }