Use EnsurePatch in OSBAPI client factory tests

The client factory is an utility and its tests are not executed in the
context of the controllers retry loop. Therefore, in order to avoid
flakes from k8s eventual consistency, we use `EnsurePatch` in tests
instead of `k8sClient.Patch` to ensure that the factory sees the state
of the objects the test setup creates.

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>

controllers/controllers/services/instances/managed/controller_test.go

@@ -419,6 +419,11 @@ var _ = Describe("CFServiceInstance", func() {
 					Status: metav1.ConditionTrue,
 					Reason: "ProvisionFailed",
 				})
+				meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
+					Type:   korifiv1alpha1.StatusConditionReady,
+					Status: metav1.ConditionFalse,
+					Reason: "ProvisionFailed",
+				})
 			})).To(Succeed())
 		})
 

controllers/controllers/services/osbapi/clientfactory_test.go

@@ -6,9 +6,9 @@ import (
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi"
 	"code.cloudfoundry.org/korifi/model/services"
+	"code.cloudfoundry.org/korifi/tests/helpers"
 	"code.cloudfoundry.org/korifi/tests/helpers/broker"
 	"code.cloudfoundry.org/korifi/tools"
-	"code.cloudfoundry.org/korifi/tools/k8s"
 	"github.com/google/uuid"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -48,7 +48,7 @@ var _ = Describe("ClientFactory", func() {
 				tools.CredentialsSecretKey: []byte(`{"username": "broker-user", "password": "broker-password"}`),
 			},
 		}
-		Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed())
+		Expect(k8sClient.Create(ctx, credentialsSecret)).To(Succeed())
 
 		cfServiceBroker = &korifiv1alpha1.CFServiceBroker{
 			ObjectMeta: metav1.ObjectMeta{
@@ -65,9 +65,9 @@ var _ = Describe("ClientFactory", func() {
 				},
 			},
 		}
-		Expect(adminClient.Create(ctx, cfServiceBroker)).To(Succeed())
+		Expect(k8sClient.Create(ctx, cfServiceBroker)).To(Succeed())
 
-		factory = osbapi.NewClientFactory(adminClient, true)
+		factory = osbapi.NewClientFactory(k8sClient, true)
 	})
 
 	JustBeforeEach(func() {
@@ -84,9 +84,9 @@ var _ = Describe("ClientFactory", func() {
 
 	When("the credentials secret cannot be found", func() {
 		BeforeEach(func() {
-			Expect(k8s.PatchResource(ctx, adminClient, cfServiceBroker, func() {
-				cfServiceBroker.Spec.Credentials.Name = "i-do-not-exist"
-			})).To(Succeed())
+			helpers.EnsurePatch(k8sClient, cfServiceBroker, func(b *korifiv1alpha1.CFServiceBroker) {
+				b.Spec.Credentials.Name = "i-do-not-exist"
+			})
 		})
 
 		It("returns an error", func() {
@@ -104,12 +104,12 @@ var _ = Describe("ClientFactory", func() {
 					Name:      cfServiceBroker.Spec.Credentials.Name,
 				},
 			}
-			Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed())
-			Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() {
-				credentialsSecret.Data = map[string][]byte{
+			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed())
+			helpers.EnsurePatch(k8sClient, credentialsSecret, func(s *corev1.Secret) {
+				s.Data = map[string][]byte{
 					"foo": []byte("bar"),
 				}
-			})).To(Succeed())
+			})
 		})
 
 		It("returns an error", func() {
@@ -118,11 +118,11 @@ var _ = Describe("ClientFactory", func() {
 
 		When("username is not set", func() {
 			BeforeEach(func() {
-				Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() {
-					credentialsSecret.Data = map[string][]byte{
+				helpers.EnsurePatch(k8sClient, credentialsSecret, func(s *corev1.Secret) {
+					s.Data = map[string][]byte{
 						tools.CredentialsSecretKey: []byte(`{"password": "my-password"}`),
 					}
-				})).To(Succeed())
+				})
 			})
 
 			It("returns an error", func() {
@@ -132,11 +132,11 @@ var _ = Describe("ClientFactory", func() {
 
 		When("password is not set", func() {
 			BeforeEach(func() {
-				Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() {
-					credentialsSecret.Data = map[string][]byte{
+				helpers.EnsurePatch(k8sClient, credentialsSecret, func(s *corev1.Secret) {
+					s.Data = map[string][]byte{
 						tools.CredentialsSecretKey: []byte(`{"username": "my-user"}`),
 					}
-				})).To(Succeed())
+				})
 			})
 
 			It("returns an error", func() {
@@ -147,7 +147,7 @@ var _ = Describe("ClientFactory", func() {
 
 	When("the client does not trust insecure brokers", func() {
 		BeforeEach(func() {
-			factory = osbapi.NewClientFactory(adminClient, false)
+			factory = osbapi.NewClientFactory(k8sClient, false)
 		})
 
 		It("creates a client that does not trust insecure brokers", func() {

controllers/controllers/services/osbapi/suite_test.go

@@ -22,12 +22,11 @@ import (
 )
 
 var (
-	ctx             context.Context
-	stopManager     context.CancelFunc
-	stopClientCache context.CancelFunc
-	testEnv         *envtest.Environment
-	adminClient     client.Client
-	rootNamespace   string
+	ctx           context.Context
+	stopManager   context.CancelFunc
+	testEnv       *envtest.Environment
+	k8sClient     client.Client
+	rootNamespace string
 )
 
 func TestOSBAPI(t *testing.T) {
@@ -55,20 +54,19 @@ var _ = BeforeSuite(func() {
 	k8sManager := helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml"))
 	Expect(shared.SetupIndexWithManager(k8sManager)).To(Succeed())
 
-	adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config)
+	k8sClient = helpers.NewSyncClient(k8sManager.GetClient())
 
 	stopManager = helpers.StartK8sManager(k8sManager)
 
 	rootNamespace = uuid.NewString()
-	Expect(adminClient.Create(ctx, &corev1.Namespace{
+	Expect(k8sClient.Create(ctx, &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: rootNamespace,
 		},
 	})).To(Succeed())
 })
 
 var _ = AfterSuite(func() {
-	stopClientCache()
 	stopManager()
 	Expect(testEnv.Stop()).To(Succeed())
 })