fixup! feat: Add waiter for object

- Retry get only if object is not found, fail immediately otherwise.
nutanix-cloud-native · Jul 3, 2024 · e57042b · e57042b
1 parent 3c78f08
commit e57042b
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 64 deletions.
diff --git a/pkg/wait/wait.go b/pkg/wait/wait.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"time"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -51,8 +52,10 @@ func ForObject[T client.Object](
 		true,
 		func(checkCtx context.Context) (bool, error) {
 			if getErr = input.Reader.Get(checkCtx, key, input.Target); getErr != nil {
-				// Retry if get fails.
-				return false, nil
+				if apierrors.IsNotFound(getErr) {
+					return false, nil
+				}
+				return false, getErr
 			}
 
 			if ok, err := input.Check(checkCtx, input.Target); err != nil {

diff --git a/pkg/wait/wait_test.go b/pkg/wait/wait_test.go
@@ -13,39 +13,58 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
+var errBrokenReader = errors.New("broken")
+
+type brokenReader struct{}
+
+func (r *brokenReader) Get(
+	ctx context.Context,
+	key client.ObjectKey,
+	obj client.Object,
+	opts ...client.GetOption,
+) error {
+	return errBrokenReader
+}
+
+func (r *brokenReader) List(
+	ctx context.Context,
+	list client.ObjectList,
+	opts ...client.ListOption,
+) error {
+	return errBrokenReader
+}
+
+var _ client.Reader = &brokenReader{}
+
 func TestWait(t *testing.T) {
-	// We use the corev1.Namespace concrete type for the test, because we want to
-	// verify behavior for a concrete type, and because the Wait function is
-	// generic, and will behave identically for all concrete types.
-	type args struct {
-		input ForObjectInput[*corev1.Namespace]
-	}
 	tests := []struct {
-		name     string
-		args     args
+		name string
+		// We use the corev1.Namespace concrete type for the test, because we want to
+		// verify behavior for a concrete type, and because the Wait function is
+		// generic, and will behave identically for all concrete types.
+		input    ForObjectInput[*corev1.Namespace]
 		errCheck func(error) bool
 	}{
 		{
-			name: "time out while get fails; report get error",
-			args: args{
-				input: ForObjectInput[*corev1.Namespace]{
-					Reader: fake.NewFakeClient(),
-					Check: func(_ context.Context, _ *corev1.Namespace) (bool, error) {
-						return true, nil
+			name: "time out while get does not find object; report get error",
+			input: ForObjectInput[*corev1.Namespace]{
+				Reader: fake.NewFakeClient(),
+				Check: func(_ context.Context, _ *corev1.Namespace) (bool, error) {
+					return true, nil
+				},
+				Interval: time.Nanosecond,
+				Timeout:  time.Millisecond,
+				Target: &corev1.Namespace{
+					TypeMeta: v1.TypeMeta{
+						Kind:       "Namespace",
+						APIVersion: "v1",
 					},
-					Interval: time.Nanosecond,
-					Timeout:  time.Millisecond,
-					Target: &corev1.Namespace{
-						TypeMeta: v1.TypeMeta{
-							Kind:       "Namespace",
-							APIVersion: "v1",
-						},
-						ObjectMeta: v1.ObjectMeta{
-							Name: "example",
-						},
+					ObjectMeta: v1.ObjectMeta{
+						Name: "example",
 					},
 				},
 			},
@@ -55,26 +74,35 @@ func TestWait(t *testing.T) {
 			},
 		},
 		{
-			name: "time out while check returns false; no check error to report",
-			args: args{
-				input: ForObjectInput[*corev1.Namespace]{
-					Reader: fake.NewFakeClient(
-						&corev1.Namespace{
-							TypeMeta: v1.TypeMeta{
-								Kind:       "Namespace",
-								APIVersion: "v1",
-							},
-							ObjectMeta: v1.ObjectMeta{
-								Name: "example",
-							},
-						},
-					),
-					Check: func(_ context.Context, _ *corev1.Namespace) (bool, error) {
-						return false, nil
+			name: "return immediately when get fails; report get error",
+			input: ForObjectInput[*corev1.Namespace]{
+				Reader: &brokenReader{},
+				Check: func(_ context.Context, _ *corev1.Namespace) (bool, error) {
+					return true, nil
+				},
+				Interval: time.Nanosecond,
+				Timeout:  time.Millisecond,
+				Target: &corev1.Namespace{
+					TypeMeta: v1.TypeMeta{
+						Kind:       "Namespace",
+						APIVersion: "v1",
+					},
+					ObjectMeta: v1.ObjectMeta{
+						Name: "example",
 					},
-					Interval: time.Nanosecond,
-					Timeout:  time.Millisecond,
-					Target: &corev1.Namespace{
+				},
+			},
+			errCheck: func(err error) bool {
+				return !wait.Interrupted(err) &&
+					!apierrors.IsNotFound(err) &&
+					errors.Is(err, errBrokenReader)
+			},
+		},
+		{
+			name: "time out while check returns false; no check error to report",
+			input: ForObjectInput[*corev1.Namespace]{
+				Reader: fake.NewFakeClient(
+					&corev1.Namespace{
 						TypeMeta: v1.TypeMeta{
 							Kind:       "Namespace",
 							APIVersion: "v1",
@@ -83,30 +111,29 @@ func TestWait(t *testing.T) {
 							Name: "example",
 						},
 					},
+				),
+				Check: func(_ context.Context, _ *corev1.Namespace) (bool, error) {
+					return false, nil
+				},
+				Interval: time.Nanosecond,
+				Timeout:  time.Millisecond,
+				Target: &corev1.Namespace{
+					TypeMeta: v1.TypeMeta{
+						Kind:       "Namespace",
+						APIVersion: "v1",
+					},
+					ObjectMeta: v1.ObjectMeta{
+						Name: "example",
+					},
 				},
 			},
 			errCheck: wait.Interrupted,
 		},
 		{
 			name: "return immediately when check returns an error; report the error",
-			args: args{
-				input: ForObjectInput[*corev1.Namespace]{
-					Reader: fake.NewFakeClient(
-						&corev1.Namespace{
-							TypeMeta: v1.TypeMeta{
-								Kind:       "Namespace",
-								APIVersion: "v1",
-							},
-							ObjectMeta: v1.ObjectMeta{
-								Name: "example",
-							},
-						},
-					),
-					Check: func(_ context.Context, _ *corev1.Namespace) (bool, error) {
-						return false, fmt.Errorf("condition failed")
-					},
-					Interval: time.Nanosecond,
-					Timeout:  time.Millisecond, Target: &corev1.Namespace{
+			input: ForObjectInput[*corev1.Namespace]{
+				Reader: fake.NewFakeClient(
+					&corev1.Namespace{
 						TypeMeta: v1.TypeMeta{
 							Kind:       "Namespace",
 							APIVersion: "v1",
@@ -115,6 +142,19 @@ func TestWait(t *testing.T) {
 							Name: "example",
 						},
 					},
+				),
+				Check: func(_ context.Context, _ *corev1.Namespace) (bool, error) {
+					return false, fmt.Errorf("condition failed")
+				},
+				Interval: time.Nanosecond,
+				Timeout:  time.Millisecond, Target: &corev1.Namespace{
+					TypeMeta: v1.TypeMeta{
+						Kind:       "Namespace",
+						APIVersion: "v1",
+					},
+					ObjectMeta: v1.ObjectMeta{
+						Name: "example",
+					},
 				},
 			},
 			errCheck: func(err error) bool {
@@ -127,7 +167,7 @@ func TestWait(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			err := ForObject(
 				context.Background(),
-				tt.args.input,
+				tt.input,
 			)
 			if !tt.errCheck(err) {
 				t.Errorf("error did not pass check: %s", err)