Introduce Ready status condition for CFServiceBinding

* The CFServiceBinding becomes ready when the underlying servicebinding.io bindings become ready * The rest of the CFServiceBinding status conditions have been removed (as other controllers and API should only care whether the binding is ready or not) * Introduce a reusable `ReadyConditionBuilder` utility used to set the object status issue: #3217 Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
cloudfoundry · Apr 25, 2024 · f78f69c · f78f69c
1 parent f60426c
commit f78f69c
Show file tree

Hide file tree

Showing 34 changed files with 570 additions and 274 deletions.
diff --git a/api/repositories/app_repository.go b/api/repositories/app_repository.go
@@ -11,7 +11,6 @@ import (
 	"code.cloudfoundry.org/korifi/api/authorization"
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/controllers/controllers/workloads/env"
 	"code.cloudfoundry.org/korifi/controllers/webhooks"
 	"code.cloudfoundry.org/korifi/tools/k8s"
@@ -431,7 +430,7 @@ func (f *AppRepo) SetCurrentDroplet(ctx context.Context, authInfo authorization.
 		return CurrentDropletRecord{}, fmt.Errorf("failed to set app droplet: %w", apierrors.FromK8sError(err, AppResourceType))
 	}
 
-	_, err = f.appAwaiter.AwaitCondition(ctx, userClient, cfApp, shared.StatusConditionReady)
+	_, err = f.appAwaiter.AwaitCondition(ctx, userClient, cfApp, korifiv1alpha1.StatusConditionReady)
 	if err != nil {
 		return CurrentDropletRecord{}, fmt.Errorf("failed to await the app staged condition: %w", apierrors.FromK8sError(err, AppResourceType))
 	}
@@ -674,7 +673,7 @@ func cfAppToAppRecord(cfApp korifiv1alpha1.CFApp) AppRecord {
 		CreatedAt:             cfApp.CreationTimestamp.Time,
 		UpdatedAt:             getLastUpdatedTime(&cfApp),
 		DeletedAt:             golangTime(cfApp.DeletionTimestamp),
-		IsStaged:              meta.IsStatusConditionTrue(cfApp.Status.Conditions, shared.StatusConditionReady),
+		IsStaged:              meta.IsStatusConditionTrue(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady),
 		envSecretName:         cfApp.Spec.EnvSecretName,
 		vcapServiceSecretName: cfApp.Status.VCAPServicesSecretName,
 		vcapAppSecretName:     cfApp.Status.VCAPApplicationSecretName,

diff --git a/api/repositories/app_repository_test.go b/api/repositories/app_repository_test.go
@@ -13,7 +13,6 @@ import (
 	. "code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/api/repositories/fakeawaiter"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/controllers/controllers/workloads/env"
 	"code.cloudfoundry.org/korifi/controllers/controllers/workloads/testutils"
 	"code.cloudfoundry.org/korifi/tests/matchers"
@@ -108,7 +107,7 @@ var _ = Describe("AppRepository", func() {
 			When("the app has staged condition true", func() {
 				BeforeEach(func() {
 					cfApp.Status.Conditions = []metav1.Condition{{
-						Type:               shared.StatusConditionReady,
+						Type:               korifiv1alpha1.StatusConditionReady,
 						Status:             metav1.ConditionTrue,
 						LastTransitionTime: metav1.Now(),
 						Reason:             "staged",
@@ -131,7 +130,7 @@ var _ = Describe("AppRepository", func() {
 			When("the app has staged condition false", func() {
 				BeforeEach(func() {
 					meta.SetStatusCondition(&cfApp.Status.Conditions, metav1.Condition{
-						Type:    shared.StatusConditionReady,
+						Type:    korifiv1alpha1.StatusConditionReady,
 						Status:  metav1.ConditionFalse,
 						Reason:  "appStaged",
 						Message: "",
@@ -140,7 +139,7 @@ var _ = Describe("AppRepository", func() {
 					Eventually(func(g Gomega) {
 						app := korifiv1alpha1.CFApp{}
 						g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfApp), &app)).To(Succeed())
-						g.Expect(meta.IsStatusConditionFalse(app.Status.Conditions, shared.StatusConditionReady)).To(BeTrue())
+						g.Expect(meta.IsStatusConditionFalse(app.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue())
 					}).Should(Succeed())
 				})
 
@@ -1120,7 +1119,7 @@ var _ = Describe("AppRepository", func() {
 				obj, conditionType := appAwaiter.AwaitConditionArgsForCall(0)
 				Expect(obj.GetName()).To(Equal(appGUID))
 				Expect(obj.GetNamespace()).To(Equal(cfSpace.Name))
-				Expect(conditionType).To(Equal(shared.StatusConditionReady))
+				Expect(conditionType).To(Equal(korifiv1alpha1.StatusConditionReady))
 			})
 
 			It("returns a CurrentDroplet record", func() {

diff --git a/api/repositories/buildpack_repository.go b/api/repositories/buildpack_repository.go
@@ -7,7 +7,7 @@ import (
 
 	"code.cloudfoundry.org/korifi/api/authorization"
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
-	"code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
+	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/types"
@@ -41,7 +41,7 @@ func NewBuildpackRepository(builderName string, userClientFactory authorization.
 }
 
 func (r *BuildpackRepository) ListBuildpacks(ctx context.Context, authInfo authorization.Info) ([]BuildpackRecord, error) {
-	var builderInfo v1alpha1.BuilderInfo
+	var builderInfo korifiv1alpha1.BuilderInfo
 
 	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
@@ -64,10 +64,10 @@ func (r *BuildpackRepository) ListBuildpacks(ctx context.Context, authInfo autho
 		return nil, apierrors.FromK8sError(err, BuildpackResourceType)
 	}
 
-	if !meta.IsStatusConditionTrue(builderInfo.Status.Conditions, StatusConditionReady) {
+	if !meta.IsStatusConditionTrue(builderInfo.Status.Conditions, korifiv1alpha1.StatusConditionReady) {
 		var conditionNotReadyMessage string
 
-		readyCondition := meta.FindStatusCondition(builderInfo.Status.Conditions, StatusConditionReady)
+		readyCondition := meta.FindStatusCondition(builderInfo.Status.Conditions, korifiv1alpha1.StatusConditionReady)
 		if readyCondition != nil {
 			conditionNotReadyMessage = readyCondition.Message
 		}
@@ -82,7 +82,7 @@ func (r *BuildpackRepository) ListBuildpacks(ctx context.Context, authInfo autho
 	return builderInfoToBuildpackRecords(builderInfo), nil
 }
 
-func builderInfoToBuildpackRecords(info v1alpha1.BuilderInfo) []BuildpackRecord {
+func builderInfoToBuildpackRecords(info korifiv1alpha1.BuilderInfo) []BuildpackRecord {
 	buildpackRecords := make([]BuildpackRecord, 0, len(info.Status.Buildpacks))
 
 	for i := range info.Status.Buildpacks {

diff --git a/api/repositories/deployment_repository.go b/api/repositories/deployment_repository.go
@@ -9,7 +9,6 @@ import (
 	"code.cloudfoundry.org/korifi/api/authorization"
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/tools/k8s"
 	"code.cloudfoundry.org/korifi/version"
 	"github.com/go-logr/logr"
@@ -155,7 +154,7 @@ func appToDeploymentRecord(cfApp *korifiv1alpha1.CFApp) DeploymentRecord {
 		},
 	}
 
-	if meta.IsStatusConditionTrue(cfApp.Status.Conditions, shared.StatusConditionReady) {
+	if meta.IsStatusConditionTrue(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady) {
 		deploymentRecord.Status = DeploymentStatus{
 			Value:  DeploymentStatusValueFinalized,
 			Reason: DeploymentStatusReasonDeployed,

diff --git a/api/repositories/deployment_repository_test.go b/api/repositories/deployment_repository_test.go
@@ -6,7 +6,6 @@ import (
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	"code.cloudfoundry.org/korifi/api/repositories"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/tests/matchers"
 	"code.cloudfoundry.org/korifi/tools/k8s"
 	"code.cloudfoundry.org/korifi/version"
@@ -88,7 +87,7 @@ var _ = Describe("DeploymentRepository", func() {
 				BeforeEach(func() {
 					Expect(k8s.Patch(ctx, k8sClient, cfApp, func() {
 						meta.SetStatusCondition(&cfApp.Status.Conditions, metav1.Condition{
-							Type:   shared.StatusConditionReady,
+							Type:   korifiv1alpha1.StatusConditionReady,
 							Status: metav1.ConditionTrue,
 							Reason: "ready",
 						})
@@ -179,7 +178,7 @@ var _ = Describe("DeploymentRepository", func() {
 				BeforeEach(func() {
 					Expect(k8s.Patch(ctx, k8sClient, cfApp, func() {
 						meta.SetStatusCondition(&cfApp.Status.Conditions, metav1.Condition{
-							Type:   shared.StatusConditionReady,
+							Type:   korifiv1alpha1.StatusConditionReady,
 							Status: metav1.ConditionTrue,
 							Reason: "ready",
 						})

diff --git a/api/repositories/org_repository.go b/api/repositories/org_repository.go
@@ -100,7 +100,7 @@ func (r *OrgRepo) CreateOrg(ctx context.Context, info authorization.Info, messag
 		return OrgRecord{}, fmt.Errorf("failed to create cf org: %w", apierrors.FromK8sError(err, OrgResourceType))
 	}
 
-	cfOrg, err = r.conditionAwaiter.AwaitCondition(ctx, userClient, cfOrg, StatusConditionReady)
+	cfOrg, err = r.conditionAwaiter.AwaitCondition(ctx, userClient, cfOrg, korifiv1alpha1.StatusConditionReady)
 	if err != nil {
 		return OrgRecord{}, apierrors.FromK8sError(err, OrgResourceType)
 	}
@@ -130,7 +130,7 @@ func (r *OrgRepo) ListOrgs(ctx context.Context, info authorization.Info, filter
 			return authorizedNamespaces[o.Name]
 		},
 		func(o korifiv1alpha1.CFOrg) bool {
-			return meta.IsStatusConditionTrue(o.Status.Conditions, StatusConditionReady)
+			return meta.IsStatusConditionTrue(o.Status.Conditions, korifiv1alpha1.StatusConditionReady)
 		},
 		SetPredicate(filter.GUIDs, func(s korifiv1alpha1.CFOrg) string { return s.Name }),
 		SetPredicate(filter.Names, func(s korifiv1alpha1.CFOrg) string { return s.Spec.DisplayName }),

diff --git a/api/repositories/org_repository_test.go b/api/repositories/org_repository_test.go
@@ -10,7 +10,6 @@ import (
 	"code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/api/repositories/fakeawaiter"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/tests/matchers"
 	"code.cloudfoundry.org/korifi/tools"
 	"code.cloudfoundry.org/korifi/tools/k8s"
@@ -137,7 +136,7 @@ var _ = Describe("OrgRepository", func() {
 				obj, conditionType := conditionAwaiter.AwaitConditionArgsForCall(0)
 				Expect(obj.GetName()).To(Equal(cfOrg.Name))
 				Expect(obj.GetNamespace()).To(Equal(rootNamespace))
-				Expect(conditionType).To(Equal(shared.StatusConditionReady))
+				Expect(conditionType).To(Equal(korifiv1alpha1.StatusConditionReady))
 			})
 
 			When("the org does not become ready", func() {

diff --git a/api/repositories/package_repository.go b/api/repositories/package_repository.go
@@ -8,7 +8,6 @@ import (
 	"code.cloudfoundry.org/korifi/api/authorization"
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/controllers/controllers/workloads"
 	"code.cloudfoundry.org/korifi/tools/dockercfg"
 	"code.cloudfoundry.org/korifi/tools/k8s"
@@ -305,8 +304,8 @@ func (r *PackageRepo) ListPackages(ctx context.Context, authInfo authorization.I
 	if len(message.States) > 0 {
 		stateSet := NewSet(message.States...)
 		preds = append(preds, func(p korifiv1alpha1.CFPackage) bool {
-			return (stateSet.Includes(PackageStateReady) && meta.IsStatusConditionTrue(p.Status.Conditions, shared.StatusConditionReady)) ||
-				(stateSet.Includes(PackageStateAwaitingUpload) && !meta.IsStatusConditionTrue(p.Status.Conditions, shared.StatusConditionReady))
+			return (stateSet.Includes(PackageStateReady) && meta.IsStatusConditionTrue(p.Status.Conditions, korifiv1alpha1.StatusConditionReady)) ||
+				(stateSet.Includes(PackageStateAwaitingUpload) && !meta.IsStatusConditionTrue(p.Status.Conditions, korifiv1alpha1.StatusConditionReady))
 		})
 	}
 
@@ -347,7 +346,7 @@ func (r *PackageRepo) UpdatePackageSource(ctx context.Context, authInfo authoriz
 		return PackageRecord{}, fmt.Errorf("failed to update package source: %w", apierrors.FromK8sError(err, PackageResourceType))
 	}
 
-	cfPackage, err = r.awaiter.AwaitCondition(ctx, userClient, cfPackage, shared.StatusConditionReady)
+	cfPackage, err = r.awaiter.AwaitCondition(ctx, userClient, cfPackage, korifiv1alpha1.StatusConditionReady)
 	if err != nil {
 		return PackageRecord{}, fmt.Errorf("failed awaiting Ready status condition: %w", err)
 	}
@@ -358,7 +357,7 @@ func (r *PackageRepo) UpdatePackageSource(ctx context.Context, authInfo authoriz
 
 func (r *PackageRepo) cfPackageToPackageRecord(cfPackage *korifiv1alpha1.CFPackage) PackageRecord {
 	state := PackageStateAwaitingUpload
-	if meta.IsStatusConditionTrue(cfPackage.Status.Conditions, shared.StatusConditionReady) {
+	if meta.IsStatusConditionTrue(cfPackage.Status.Conditions, korifiv1alpha1.StatusConditionReady) {
 		state = PackageStateReady
 	}
 	return PackageRecord{

diff --git a/api/repositories/service_binding_repository.go b/api/repositories/service_binding_repository.go
@@ -144,7 +144,7 @@ func (r *ServiceBindingRepo) CreateServiceBinding(ctx context.Context, authInfo
 		return ServiceBindingRecord{}, apierrors.FromK8sError(err, ServiceBindingResourceType)
 	}
 
-	cfServiceBinding, err = r.bindingConditionAwaiter.AwaitCondition(ctx, userClient, cfServiceBinding, VCAPServicesSecretAvailableCondition)
+	cfServiceBinding, err = r.bindingConditionAwaiter.AwaitCondition(ctx, userClient, cfServiceBinding, korifiv1alpha1.StatusConditionReady)
 	if err != nil {
 		return ServiceBindingRecord{}, err
 	}

diff --git a/api/repositories/service_binding_repository_test.go b/api/repositories/service_binding_repository_test.go
@@ -89,7 +89,7 @@ var _ = Describe("ServiceBindingRepo", func() {
 				Expect(k8s.Patch(ctx, k8sClient, cfServiceBinding, func() {
 					cfServiceBinding.Status.Binding.Name = "service-secret-name"
 					meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{
-						Type:    repositories.VCAPServicesSecretAvailableCondition,
+						Type:    korifiv1alpha1.StatusConditionReady,
 						Status:  metav1.ConditionTrue,
 						Reason:  "blah",
 						Message: "blah",
@@ -169,7 +169,7 @@ var _ = Describe("ServiceBindingRepo", func() {
 				obj, conditionType := conditionAwaiter.AwaitConditionArgsForCall(0)
 				Expect(obj.GetName()).To(Equal(cfServiceBinding.Name))
 				Expect(obj.GetNamespace()).To(Equal(space.Name))
-				Expect(conditionType).To(Equal(repositories.VCAPServicesSecretAvailableCondition))
+				Expect(conditionType).To(Equal(korifiv1alpha1.StatusConditionReady))
 			})
 
 			When("the vcap services secret available condition is never met", func() {

diff --git a/api/repositories/shared.go b/api/repositories/shared.go
@@ -10,11 +10,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-const (
-	StatusConditionReady                 = "Ready"
-	VCAPServicesSecretAvailableCondition = "VCAPServicesSecretAvailable"
-)
-
 //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate
 
 //counterfeiter:generate -o fake -fake-name RepositoryCreator . RepositoryCreator

diff --git a/api/repositories/space_repository.go b/api/repositories/space_repository.go
@@ -104,7 +104,7 @@ func (r *SpaceRepo) CreateSpace(ctx context.Context, info authorization.Info, me
 		return SpaceRecord{}, apierrors.FromK8sError(err, SpaceResourceType)
 	}
 
-	cfSpace, err = r.conditionAwaiter.AwaitCondition(ctx, userClient, cfSpace, StatusConditionReady)
+	cfSpace, err = r.conditionAwaiter.AwaitCondition(ctx, userClient, cfSpace, korifiv1alpha1.StatusConditionReady)
 	if err != nil {
 		return SpaceRecord{}, apierrors.FromK8sError(err, SpaceResourceType)
 	}
@@ -133,7 +133,7 @@ func (r *SpaceRepo) ListSpaces(ctx context.Context, info authorization.Info, mes
 	preds := []func(korifiv1alpha1.CFSpace) bool{
 		func(s korifiv1alpha1.CFSpace) bool { return authorizedSpaceNamespaces[s.Name] },
 		func(s korifiv1alpha1.CFSpace) bool {
-			return meta.IsStatusConditionTrue(s.Status.Conditions, StatusConditionReady)
+			return meta.IsStatusConditionTrue(s.Status.Conditions, korifiv1alpha1.StatusConditionReady)
 		},
 		SetPredicate(message.GUIDs, func(s korifiv1alpha1.CFSpace) string { return s.Name }),
 		SetPredicate(message.Names, func(s korifiv1alpha1.CFSpace) string { return s.Spec.DisplayName }),

diff --git a/api/repositories/space_repository_test.go b/api/repositories/space_repository_test.go
@@ -9,7 +9,6 @@ import (
 	"code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/api/repositories/fakeawaiter"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/tests/matchers"
 	"code.cloudfoundry.org/korifi/tools"
 	"code.cloudfoundry.org/korifi/tools/k8s"
@@ -124,7 +123,7 @@ var _ = Describe("SpaceRepository", func() {
 				obj, conditionType := conditionAwaiter.AwaitConditionArgsForCall(0)
 				Expect(obj.GetName()).To(Equal(cfSpace.Name))
 				Expect(obj.GetNamespace()).To(Equal(orgGUID))
-				Expect(conditionType).To(Equal(shared.StatusConditionReady))
+				Expect(conditionType).To(Equal(korifiv1alpha1.StatusConditionReady))
 			})
 
 			It("creates a CFSpace resource in the org namespace", func() {

diff --git a/controllers/api/v1alpha1/constants.go b/controllers/api/v1alpha1/constants.go
@@ -10,4 +10,6 @@ const (
 	HTTPHealthCheckType    HealthCheckType = "http"
 	PortHealthCheckType    HealthCheckType = "port"
 	ProcessHealthCheckType HealthCheckType = "process"
+
+	StatusConditionReady = "Ready"
 )
diff --git a/controllers/cleanup/package_cleaner.go b/controllers/cleanup/package_cleaner.go
@@ -5,7 +5,6 @@ import (
 	"sort"
 
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/statefulset-runner/controllers"
 
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -58,7 +57,7 @@ func (c PackageCleaner) Clean(ctx context.Context, app types.NamespacedName) err
 		if cfPackage.Name == currentPackage {
 			continue
 		}
-		if !meta.IsStatusConditionTrue(cfPackage.Status.Conditions, shared.StatusConditionReady) {
+		if !meta.IsStatusConditionTrue(cfPackage.Status.Conditions, korifiv1alpha1.StatusConditionReady) {
 			continue
 		}
 		deletablePackages = append(deletablePackages, cfPackage)

diff --git a/controllers/cleanup/package_cleaner_test.go b/controllers/cleanup/package_cleaner_test.go
@@ -5,7 +5,6 @@ import (
 
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"code.cloudfoundry.org/korifi/controllers/cleanup"
-	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	. "code.cloudfoundry.org/korifi/controllers/controllers/workloads/testutils"
 	"code.cloudfoundry.org/korifi/statefulset-runner/controllers"
 
@@ -132,7 +131,7 @@ func createPackage(namespace, appGUID, name string) *korifiv1alpha1.CFPackage {
 func createReadyPackage(namespace, appGUID, name string) *korifiv1alpha1.CFPackage {
 	pkg := createPackage(namespace, appGUID, name)
 	meta.SetStatusCondition(&pkg.Status.Conditions, metav1.Condition{
-		Type:   shared.StatusConditionReady,
+		Type:   korifiv1alpha1.StatusConditionReady,
 		Status: metav1.ConditionTrue,
 		Reason: "SourceImageSet",
 	})