Alternate Get retry approach to avoid copy-and-paste

kaovilai · Oct 13, 2023 · 181156f · 181156f
1 parent 9de7131
commit 181156f
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 23 deletions.
diff --git a/pkg/client/retry.go b/pkg/client/retry.go
@@ -18,10 +18,10 @@ package client
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -60,22 +60,33 @@ func CreateRetryGenerateNameWithFunc(obj kbclient.Object, createFn func() error)
 	}
 }
 
-func GetRetriableWithCacheLister(lister cache.GenericNamespaceLister, name string, retriable func(error) bool) (runtime.Object, error) {
-	var clusterObj runtime.Object
-	getFunc := func() error {
-		var err error
-		clusterObj, err = lister.Get(name)
-		return err
+type GetFunc func(name string) (*unstructured.Unstructured, error)
+
+func GetFuncForCacheLister(lister cache.GenericNamespaceLister) GetFunc {
+	return func(name string) (*unstructured.Unstructured, error) {
+		runtimeObj, err := lister.Get(name)
+		if err != nil {
+			return nil, err
+		}
+		u, ok := runtimeObj.(*unstructured.Unstructured)
+		if !ok {
+			return nil, fmt.Errorf("expected *unstructured.Unstructured but got %T", u)
+		}
+		return u, nil
+	}
+}
+
+func GetFuncForDynamicClient(client Dynamic, getOptions metav1.GetOptions) GetFunc {
+	return func(name string) (*unstructured.Unstructured, error) {
+		return client.Get(name, getOptions)
 	}
-	err := retry.OnError(MinuteBackoff, retriable, getFunc)
-	return clusterObj, err
 }
 
-func GetRetriableWithDynamicClient(client Dynamic, name string, getOptions metav1.GetOptions, retriable func(error) bool) (*unstructured.Unstructured, error) {
+func GetRetriable(getFuncIn GetFunc, name string, retriable func(error) bool) (*unstructured.Unstructured, error) {
 	var clusterObj *unstructured.Unstructured
 	getFunc := func() error {
 		var err error
-		clusterObj, err = client.Get(name, getOptions)
+		clusterObj, err = getFuncIn(name)
 		return err
 	}
 	err := retry.OnError(MinuteBackoff, retriable, getFunc)

diff --git a/pkg/client/retry_test.go b/pkg/client/retry_test.go
@@ -67,7 +67,7 @@ func TestGetRetriableWithDynamicClient(t *testing.T) {
 				return tt.want, nil
 			})
 
-			got, err := GetRetriableWithDynamicClient(dc, tt.args.name, tt.args.getOptions, tt.args.retriable)
+			got, err := GetRetriable(GetFuncForDynamicClient(dc, tt.args.getOptions), tt.args.name, tt.args.retriable)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("GetRetriableWithDynamicClient() error = %v, wantErr %v", err, tt.wantErr)
 				return
@@ -114,7 +114,7 @@ func TestGetRetriableWithCacheLister(t *testing.T) {
 				}
 				return tt.want, nil
 			})
-			got, err := GetRetriableWithCacheLister(lister, tt.args.name, tt.args.retriable)
+			got, err := GetRetriable(GetFuncForCacheLister(lister), tt.args.name, tt.args.retriable)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("GetRetriableWithCacheLister() error = %v, wantErr %v", err, tt.wantErr)
 				return

diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go
@@ -1073,22 +1073,17 @@ func (ctx *restoreContext) getResource(groupResource schema.GroupResource, obj *
 	lister := ctx.getResourceLister(groupResource, obj, namespace)
 	var (
 		err        error
-		clusterObj runtime.Object
+		clusterObj *unstructured.Unstructured
 	)
 	if retriable == nil {
-		clusterObj, err = lister.Get(name)
+		clusterObj, err = client.GetFuncForCacheLister(lister)(name)
 	} else {
-		clusterObj, err = client.GetRetriableWithCacheLister(lister, name, retriable)
+		clusterObj, err = client.GetRetriable(client.GetFuncForCacheLister(lister), name, retriable)
 	}
 	if err != nil {
 		return nil, errors.Wrapf(err, "error getting resource from lister for %s, %s/%s", groupResource, namespace, name)
 	}
-	u, ok := clusterObj.(*unstructured.Unstructured)
-	if !ok {
-		ctx.log.WithError(errors.WithStack(fmt.Errorf("expected *unstructured.Unstructured but got %T", u))).Error("unable to understand entry returned from client")
-		return nil, fmt.Errorf("expected *unstructured.Unstructured but got %T", u)
-	}
-	return u, nil
+	return clusterObj, nil
 }
 
 func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupResource schema.GroupResource, namespace string) (results.Result, results.Result, bool) {
@@ -1540,7 +1535,7 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
 		if !ctx.disableInformerCache {
 			fromCluster, err = ctx.getResource(groupResource, obj, namespace, name, apierrors.IsNotFound)
 		} else {
-			fromCluster, err = client.GetRetriableWithDynamicClient(resourceClient, name, metav1.GetOptions{}, apierrors.IsNotFound)
+			fromCluster, err = client.GetRetriable(client.GetFuncForDynamicClient(resourceClient, metav1.GetOptions{}), name, apierrors.IsNotFound)
 		}
 		if err != nil && isAlreadyExistsError {
 			ctx.log.Errorf("Error retrieving in-cluster version of %s: %v", kube.NamespaceAndName(obj), err)