Merge pull request #60520 from shyamjvs/retry-delete-operations

Automatic merge from submit-queue (batch tested with PRs 60623, 60625, 60520). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add retries to resource deletions in testing framework

Fix https://github.com/kubernetes/kubernetes/issues/55860
(last piece towards fixing it hopefully)

/cc @wojtek-t 

```release-note
NONE
```

test/e2e/framework/util.go

@@ -2923,23 +2923,6 @@ func getRuntimeObjectForKind(c clientset.Interface, kind schema.GroupKind, ns, n
 	}
 }
 
-func deleteResource(c clientset.Interface, kind schema.GroupKind, ns, name string, deleteOption *metav1.DeleteOptions) error {
-	switch kind {
-	case api.Kind("ReplicationController"):
-		return c.CoreV1().ReplicationControllers(ns).Delete(name, deleteOption)
-	case extensionsinternal.Kind("ReplicaSet"), appsinternal.Kind("ReplicaSet"):
-		return c.ExtensionsV1beta1().ReplicaSets(ns).Delete(name, deleteOption)
-	case extensionsinternal.Kind("Deployment"), appsinternal.Kind("Deployment"):
-		return c.ExtensionsV1beta1().Deployments(ns).Delete(name, deleteOption)
-	case extensionsinternal.Kind("DaemonSet"):
-		return c.ExtensionsV1beta1().DaemonSets(ns).Delete(name, deleteOption)
-	case batchinternal.Kind("Job"):
-		return c.BatchV1().Jobs(ns).Delete(name, deleteOption)
-	default:
-		return fmt.Errorf("Unsupported kind when deleting: %v", kind)
-	}
-}
-
 func getSelectorFromRuntimeObject(obj runtime.Object) (labels.Selector, error) {
 	switch typed := obj.(type) {
 	case *v1.ReplicationController:
@@ -2986,10 +2969,6 @@ func getReplicasFromRuntimeObject(obj runtime.Object) (int32, error) {
 	}
 }
 
-func getReaperForKind(internalClientset internalclientset.Interface, kind schema.GroupKind) (kubectl.Reaper, error) {
-	return kubectl.ReaperFor(kind, internalClientset)
-}
-
 // DeleteResourceAndPods deletes a given resource and all pods it spawned
 func DeleteResourceAndPods(clientset clientset.Interface, internalClientset internalclientset.Interface, kind schema.GroupKind, ns, name string) error {
 	By(fmt.Sprintf("deleting %v %s in namespace %s", kind, name, ns))
@@ -3006,23 +2985,13 @@ func DeleteResourceAndPods(clientset clientset.Interface, internalClientset inte
 	if err != nil {
 		return err
 	}
-	reaper, err := getReaperForKind(internalClientset, kind)
-	if err != nil {
-		return err
-	}
-
 	ps, err := podStoreForSelector(clientset, ns, selector)
 	if err != nil {
 		return err
 	}
 	defer ps.Stop()
 	startTime := time.Now()
-	err = reaper.Stop(ns, name, 0, nil)
-	if apierrs.IsNotFound(err) {
-		Logf("%v %s was already deleted: %v", kind, name, err)
-		return nil
-	}
-	if err != nil {
+	if err := testutils.DeleteResourceUsingReaperWithRetries(internalClientset, kind, ns, name, nil); err != nil {
 		return fmt.Errorf("error while stopping %v: %s: %v", kind, name, err)
 	}
 	deleteTime := time.Now().Sub(startTime)
@@ -3071,15 +3040,10 @@ func DeleteResourceAndWaitForGC(c clientset.Interface, kind schema.GroupKind, ns
 	}
 
 	defer ps.Stop()
-	startTime := time.Now()
 	falseVar := false
 	deleteOption := &metav1.DeleteOptions{OrphanDependents: &falseVar}
-	err = deleteResource(c, kind, ns, name, deleteOption)
-	if err != nil && apierrs.IsNotFound(err) {
-		Logf("%v %s was already deleted: %v", kind, name, err)
-		return nil
-	}
-	if err != nil {
+	startTime := time.Now()
+	if err := testutils.DeleteResourceWithRetries(c, kind, ns, name, deleteOption); err != nil {
 		return err
 	}
 	deleteTime := time.Now().Sub(startTime)

test/e2e/scalability/density.go

@@ -538,8 +538,7 @@ var _ = SIGDescribe("Density", func() {
 			namespaces, err := CreateNamespaces(f, numberOfCollections, fmt.Sprintf("density-%v", testArg.podsPerNode), testPhaseDurations.StartPhase(200, "namespace creation"))
 			framework.ExpectNoError(err)
 			if itArg.quotas {
-				err := CreateQuotas(f, namespaces, totalPods+nodeCount, testPhaseDurations.StartPhase(210, "quota creation"))
-				framework.ExpectNoError(err)
+				framework.ExpectNoError(CreateQuotas(f, namespaces, totalPods+nodeCount, testPhaseDurations.StartPhase(210, "quota creation")))
 			}
 
 			configs := make([]testutils.RunObjectConfig, numberOfCollections)

test/e2e/scalability/load.go

@@ -248,7 +248,7 @@ var _ = SIGDescribe("Load capacity", func() {
 					framework.Logf("Starting to delete services...")
 					deleteService := func(i int) {
 						defer GinkgoRecover()
-						framework.ExpectNoError(clientset.CoreV1().Services(services[i].Namespace).Delete(services[i].Name, nil))
+						framework.ExpectNoError(testutils.DeleteResourceWithRetries(clientset, api.Kind("Service"), services[i].Namespace, services[i].Name, nil))
 					}
 					workqueue.Parallelize(serviceOperationsParallelism, len(services), deleteService)
 					framework.Logf("Services deleted")

test/utils/BUILD

@@ -10,6 +10,7 @@ go_library(
     srcs = [
         "conditions.go",
         "create_resources.go",
+        "delete_resources.go",
         "density_utils.go",
         "deployment.go",
         "pod_store.go",
@@ -21,6 +22,7 @@ go_library(
     importpath = "k8s.io/kubernetes/test/utils",
     deps = [
         "//pkg/api/v1/pod:go_default_library",
+        "//pkg/apis/apps:go_default_library",
         "//pkg/apis/batch:go_default_library",
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/extensions:go_default_library",

test/utils/delete_resources.go

@@ -0,0 +1,95 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// TODO: Refactor common part of functions in this file for generic object kinds.
+
+package utils
+
+import (
+	"fmt"
+
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	clientset "k8s.io/client-go/kubernetes"
+	appsinternal "k8s.io/kubernetes/pkg/apis/apps"
+	batchinternal "k8s.io/kubernetes/pkg/apis/batch"
+	api "k8s.io/kubernetes/pkg/apis/core"
+	extensionsinternal "k8s.io/kubernetes/pkg/apis/extensions"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
+	"k8s.io/kubernetes/pkg/kubectl"
+)
+
+func deleteResource(c clientset.Interface, kind schema.GroupKind, namespace, name string, options *metav1.DeleteOptions) error {
+	switch kind {
+	case api.Kind("Pod"):
+		return c.CoreV1().Pods(namespace).Delete(name, options)
+	case api.Kind("ReplicationController"):
+		return c.CoreV1().ReplicationControllers(namespace).Delete(name, options)
+	case extensionsinternal.Kind("ReplicaSet"), appsinternal.Kind("ReplicaSet"):
+		return c.ExtensionsV1beta1().ReplicaSets(namespace).Delete(name, options)
+	case extensionsinternal.Kind("Deployment"), appsinternal.Kind("Deployment"):
+		return c.ExtensionsV1beta1().Deployments(namespace).Delete(name, options)
+	case extensionsinternal.Kind("DaemonSet"):
+		return c.ExtensionsV1beta1().DaemonSets(namespace).Delete(name, options)
+	case batchinternal.Kind("Job"):
+		return c.BatchV1().Jobs(namespace).Delete(name, options)
+	case api.Kind("Secret"):
+		return c.CoreV1().Secrets(namespace).Delete(name, options)
+	case api.Kind("ConfigMap"):
+		return c.CoreV1().ConfigMaps(namespace).Delete(name, options)
+	case api.Kind("Service"):
+		return c.CoreV1().Services(namespace).Delete(name, options)
+	default:
+		return fmt.Errorf("Unsupported kind when deleting: %v", kind)
+	}
+}
+
+func getReaperForKind(c internalclientset.Interface, kind schema.GroupKind) (kubectl.Reaper, error) {
+	return kubectl.ReaperFor(kind, c)
+}
+
+func DeleteResourceWithRetries(c clientset.Interface, kind schema.GroupKind, namespace, name string, options *metav1.DeleteOptions) error {
+	deleteFunc := func() (bool, error) {
+		err := deleteResource(c, kind, namespace, name, options)
+		if err == nil || apierrs.IsNotFound(err) {
+			return true, nil
+		}
+		if IsRetryableAPIError(err) {
+			return false, nil
+		}
+		return false, fmt.Errorf("Failed to delete object with non-retriable error: %v", err)
+	}
+	return RetryWithExponentialBackOff(deleteFunc)
+}
+
+func DeleteResourceUsingReaperWithRetries(c internalclientset.Interface, kind schema.GroupKind, namespace, name string, options *metav1.DeleteOptions) error {
+	reaper, err := getReaperForKind(c, kind)
+	if err != nil {
+		return err
+	}
+	deleteFunc := func() (bool, error) {
+		err := reaper.Stop(namespace, name, 0, options)
+		if err == nil || apierrs.IsNotFound(err) {
+			return true, nil
+		}
+		if IsRetryableAPIError(err) {
+			return false, nil
+		}
+		return false, fmt.Errorf("Failed to delete object with non-retriable error: %v", err)
+	}
+	return RetryWithExponentialBackOff(deleteFunc)
+}

test/utils/runners.go

@@ -1113,7 +1113,7 @@ func (config *SecretConfig) Run() error {
 }
 
 func (config *SecretConfig) Stop() error {
-	if err := config.Client.CoreV1().Secrets(config.Namespace).Delete(config.Name, &metav1.DeleteOptions{}); err != nil {
+	if err := DeleteResourceWithRetries(config.Client, api.Kind("Secret"), config.Namespace, config.Name, &metav1.DeleteOptions{}); err != nil {
 		return fmt.Errorf("Error deleting secret: %v", err)
 	}
 	config.LogFunc("Deleted secret %v/%v", config.Namespace, config.Name)
@@ -1171,7 +1171,7 @@ func (config *ConfigMapConfig) Run() error {
 }
 
 func (config *ConfigMapConfig) Stop() error {
-	if err := config.Client.CoreV1().ConfigMaps(config.Namespace).Delete(config.Name, &metav1.DeleteOptions{}); err != nil {
+	if err := DeleteResourceWithRetries(config.Client, api.Kind("ConfigMap"), config.Namespace, config.Name, &metav1.DeleteOptions{}); err != nil {
 		return fmt.Errorf("Error deleting configmap: %v", err)
 	}
 	config.LogFunc("Deleted configmap %v/%v", config.Namespace, config.Name)