From f78f69c2ea7d32d645e6b4eb988c91da4ef25423 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Thu, 25 Apr 2024 11:09:29 +0000 Subject: [PATCH] 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 --- api/repositories/app_repository.go | 5 +- api/repositories/app_repository_test.go | 9 +- api/repositories/buildpack_repository.go | 10 +- api/repositories/deployment_repository.go | 3 +- .../deployment_repository_test.go | 5 +- api/repositories/org_repository.go | 4 +- api/repositories/org_repository_test.go | 3 +- api/repositories/package_repository.go | 9 +- .../service_binding_repository.go | 2 +- .../service_binding_repository_test.go | 4 +- api/repositories/shared.go | 5 - api/repositories/space_repository.go | 4 +- api/repositories/space_repository_test.go | 3 +- controllers/api/v1alpha1/constants.go | 2 + controllers/cleanup/package_cleaner.go | 3 +- controllers/cleanup/package_cleaner_test.go | 3 +- .../services/bindings/controller.go | 217 ++++++++-------- .../services/bindings/controller_test.go | 233 +++++++++++++++--- .../services/bindings/suite_test.go | 2 +- controllers/controllers/shared/index.go | 2 - .../controllers/workloads/cfapp_controller.go | 35 +-- .../workloads/cfapp_controller_test.go | 73 ++---- .../controllers/workloads/cforg_controller.go | 5 +- .../workloads/cfpackage_controller.go | 3 +- .../workloads/cfpackage_controller_test.go | 7 +- .../workloads/cfprocess_controller.go | 4 +- .../workloads/cfspace_controller.go | 6 +- .../workloads/cftask_controller.go | 3 +- .../controllers/workloads/k8sns/reconciler.go | 2 +- .../controllers/workloads/suite_test.go | 4 +- .../controllers/appworkload_controller.go | 4 +- tests/crds/crds_suite_test.go | 5 +- tools/k8s/condition_builder.go | 63 +++++ tools/k8s/condition_builder_test.go | 102 ++++++++ 34 files changed, 570 insertions(+), 274 deletions(-) create mode 100644 tools/k8s/condition_builder.go create mode 100644 tools/k8s/condition_builder_test.go diff --git a/api/repositories/app_repository.go b/api/repositories/app_repository.go index 7b5cc0c49..1da967da2 100644 --- 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 index d9b990b31..a7d66d5fa 100644 --- 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 index c9c68bfec..fa31ca9ae 100644 --- 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 index d26a58a8b..a2c0eefd9 100644 --- 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 index d0f20c149..e6795d123 100644 --- 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 index 449c9422a..38da26776 100644 --- 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 index a3fd81202..2eba7a7d3 100644 --- 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 index e9af01b5a..9a35bcb79 100644 --- 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 index 3443bb865..84a7682fe 100644 --- 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 index b24348842..9e39f43ab 100644 --- 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 index 95e5ba9a1..9848383ea 100644 --- 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 index 6f21699b4..257b26485 100644 --- 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 index db64ad119..be2e3e971 100644 --- 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 index 14b69a293..38cc8d9e2 100644 --- 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 index bdd0d8d72..e0976ae73 100644 --- 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 index f57333462..a898ad479 100644 --- 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", }) diff --git a/controllers/controllers/services/bindings/controller.go b/controllers/controllers/services/bindings/controller.go index 76a97981c..88df50da5 100644 --- a/controllers/controllers/services/bindings/controller.go +++ b/controllers/controllers/services/bindings/controller.go @@ -44,11 +44,9 @@ import ( ) const ( - BindingSecretAvailableCondition = "BindingSecretAvailable" - VCAPServicesSecretAvailableCondition = "VCAPServicesSecretAvailable" - ServiceBindingGUIDLabel = "korifi.cloudfoundry.org/service-binding-guid" - ServiceCredentialBindingTypeLabel = "korifi.cloudfoundry.org/service-credential-binding-type" - ServiceBindingSecretTypePrefix = "servicebinding.io/" + ServiceBindingGUIDLabel = "korifi.cloudfoundry.org/service-binding-guid" + ServiceCredentialBindingTypeLabel = "korifi.cloudfoundry.org/service-credential-binding-type" + ServiceBindingSecretTypePrefix = "servicebinding.io/" ) // CFServiceBindingReconciler reconciles a CFServiceBinding object @@ -70,9 +68,14 @@ func NewCFServiceBindingReconciler( func (r *CFServiceBindingReconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder { return ctrl.NewControllerManagedBy(mgr). For(&korifiv1alpha1.CFServiceBinding{}). + Owns(&servicebindingv1beta1.ServiceBinding{}). Watches( &korifiv1alpha1.CFServiceInstance{}, handler.EnqueueRequestsFromMapFunc(r.serviceInstanceToServiceBindings), + ). + Watches( + &korifiv1alpha1.CFApp{}, + handler.EnqueueRequestsFromMapFunc(r.appToServiceBindings), ) } @@ -100,6 +103,31 @@ func (r *CFServiceBindingReconciler) serviceInstanceToServiceBindings(ctx contex return requests } +func (r *CFServiceBindingReconciler) appToServiceBindings(ctx context.Context, o client.Object) []reconcile.Request { + cfApp := o.(*korifiv1alpha1.CFApp) + + serviceBindings := &korifiv1alpha1.CFServiceBindingList{} + + if err := r.k8sClient.List(ctx, serviceBindings, + client.InNamespace(cfApp.Namespace), + client.MatchingFields{shared.IndexServiceBindingAppGUID: cfApp.Name}, + ); err != nil { + return []reconcile.Request{} + } + + requests := []reconcile.Request{} + for _, sb := range serviceBindings.Items { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: sb.Name, + Namespace: sb.Namespace, + }, + }) + } + + return requests +} + //+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebindings,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebindings/status,verbs=get;update;patch //+kubebuilder:rbac:groups=servicebinding.io,resources=servicebindings,verbs=get;list;create;update;patch;watch @@ -110,9 +138,16 @@ func (r *CFServiceBindingReconciler) ReconcileResource(ctx context.Context, cfSe cfServiceBinding.Status.ObservedGeneration = cfServiceBinding.Generation log.V(1).Info("set observed generation", "generation", cfServiceBinding.Status.ObservedGeneration) + var err error + readyConditionBuilder := k8s.NewReadyConditionBuilder(cfServiceBinding) + defer func() { + meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, readyConditionBuilder.WithError(err).Build()) + }() + cfServiceInstance := new(korifiv1alpha1.CFServiceInstance) - err := r.k8sClient.Get(ctx, types.NamespacedName{Name: cfServiceBinding.Spec.Service.Name, Namespace: cfServiceBinding.Namespace}, cfServiceInstance) + err = r.k8sClient.Get(ctx, types.NamespacedName{Name: cfServiceBinding.Spec.Service.Name, Namespace: cfServiceBinding.Namespace}, cfServiceInstance) if err != nil { + log.Info("service instance not found", "service-instance", cfServiceBinding.Spec.Service.Name, "error", err) return ctrl.Result{}, err } @@ -123,13 +158,9 @@ func (r *CFServiceBindingReconciler) ReconcileResource(ctx context.Context, cfSe } if cfServiceInstance.Status.Credentials.Name == "" { - meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ - Type: BindingSecretAvailableCondition, - Status: metav1.ConditionFalse, - Reason: "CredentialsSecretNotAvailable", - Message: "Service instance credentials not available yet", - ObservedGeneration: cfServiceBinding.Generation, - }) + readyConditionBuilder. + WithReason("CredentialsSecretNotAvailable"). + WithMessage("Service instance credentials not available yet") return ctrl.Result{RequeueAfter: time.Second}, nil } @@ -146,13 +177,6 @@ func (r *CFServiceBindingReconciler) ReconcileResource(ctx context.Context, cfSe } log.Error(err, "failed to reconcile credentials secret") - meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ - Type: BindingSecretAvailableCondition, - Status: metav1.ConditionFalse, - Reason: "FailedReconcilingCredentialsSecret", - Message: err.Error(), - ObservedGeneration: cfServiceBinding.Generation, - }) return ctrl.Result{}, err } @@ -165,41 +189,37 @@ func (r *CFServiceBindingReconciler) ReconcileResource(ctx context.Context, cfSe if cfApp.Status.VCAPServicesSecretName == "" { log.V(1).Info("did not find VCAPServiceSecret name on status of CFApp", "CFServiceBinding", cfServiceBinding.Name) - meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ - Type: VCAPServicesSecretAvailableCondition, - Status: metav1.ConditionFalse, - Reason: "SecretNotFound", - Message: "VCAPServicesSecret name absent from status of CFApp", - ObservedGeneration: cfServiceBinding.Generation, - }) + readyConditionBuilder.WithReason("VcapServicesSecretNotAvailable") return ctrl.Result{RequeueAfter: 2 * time.Second}, nil } - meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ - Type: VCAPServicesSecretAvailableCondition, - Status: metav1.ConditionTrue, - Reason: "SecretFound", - Message: "", - ObservedGeneration: cfServiceBinding.Generation, - }) + sbServiceBinding, err := r.reconcileSBServiceBinding(ctx, cfServiceBinding, credentialsSecret) + if err != nil { + log.Info("error creating/updating servicebinding.io servicebinding", "reason", err) + return ctrl.Result{}, err + } - actualSBServiceBinding := servicebindingv1beta1.ServiceBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("cf-binding-%s", cfServiceBinding.Name), - Namespace: cfServiceBinding.Namespace, - }, + if !isSbServiceBindingReady(sbServiceBinding) { + readyConditionBuilder.WithReason("ServiceBindingNotReady") + return ctrl.Result{}, nil } - desiredSBServiceBinding := generateDesiredServiceBinding(&actualSBServiceBinding, cfServiceBinding, cfApp, credentialsSecret) + readyConditionBuilder.Ready() + return ctrl.Result{}, nil +} + +func isSbServiceBindingReady(sbServiceBinding *servicebindingv1beta1.ServiceBinding) bool { + readyCondition := meta.FindStatusCondition(sbServiceBinding.Status.Conditions, "Ready") + if readyCondition == nil { + return false + } - _, err = controllerutil.CreateOrPatch(ctx, r.k8sClient, &actualSBServiceBinding, sbServiceBindingMutateFn(&actualSBServiceBinding, desiredSBServiceBinding)) - if err != nil { - log.Info("error calling Create on servicebinding.io ServiceBinding", "reason", err) - return ctrl.Result{}, err + if readyCondition.Status != metav1.ConditionTrue { + return false } - return ctrl.Result{}, nil + return sbServiceBinding.Generation == sbServiceBinding.Status.ObservedGeneration } func (r *CFServiceBindingReconciler) reconcileCredentials(ctx context.Context, cfServiceInstance *korifiv1alpha1.CFServiceInstance, cfServiceBinding *korifiv1alpha1.CFServiceBinding) (*corev1.Secret, error) { @@ -258,13 +278,6 @@ func (r *CFServiceBindingReconciler) reconcileCredentials(ctx context.Context, c } cfServiceBinding.Status.Binding.Name = bindingSecret.Name - meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ - Type: BindingSecretAvailableCondition, - Status: metav1.ConditionTrue, - Reason: "SecretAvailable", - Message: "", - ObservedGeneration: cfServiceBinding.Generation, - }) return bindingSecret, nil } @@ -281,65 +294,65 @@ func isLegacyServiceBinding(cfServiceBinding *korifiv1alpha1.CFServiceBinding, c return cfServiceInstance.Name == cfServiceBinding.Status.Binding.Name && cfServiceInstance.Spec.SecretName == cfServiceBinding.Status.Binding.Name } -func sbServiceBindingMutateFn(actualSBServiceBinding, desiredSBServiceBinding *servicebindingv1beta1.ServiceBinding) controllerutil.MutateFn { - return func() error { - actualSBServiceBinding.Labels = desiredSBServiceBinding.Labels - actualSBServiceBinding.OwnerReferences = desiredSBServiceBinding.OwnerReferences - actualSBServiceBinding.Spec = desiredSBServiceBinding.Spec - return nil - } -} +func (r *CFServiceBindingReconciler) reconcileSBServiceBinding(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding, credentialsSecret *corev1.Secret) (*servicebindingv1beta1.ServiceBinding, error) { + sbServiceBinding := r.toSBServiceBinding(cfServiceBinding) -func generateDesiredServiceBinding(actualServiceBinding *servicebindingv1beta1.ServiceBinding, cfServiceBinding *korifiv1alpha1.CFServiceBinding, cfApp *korifiv1alpha1.CFApp, secret *corev1.Secret) *servicebindingv1beta1.ServiceBinding { - var desiredServiceBinding servicebindingv1beta1.ServiceBinding - actualServiceBinding.DeepCopyInto(&desiredServiceBinding) - desiredServiceBinding.Labels = map[string]string{ - ServiceBindingGUIDLabel: cfServiceBinding.Name, - korifiv1alpha1.CFAppGUIDLabelKey: cfApp.Name, - ServiceCredentialBindingTypeLabel: "app", - } - desiredServiceBinding.OwnerReferences = []metav1.OwnerReference{ - { - APIVersion: "korifi.cloudfoundry.org/v1alpha1", - Kind: "CFServiceBinding", - Name: cfServiceBinding.Name, - UID: cfServiceBinding.UID, - }, - } + _, err := controllerutil.CreateOrPatch(ctx, r.k8sClient, sbServiceBinding, func() error { + sbServiceBinding.Spec.Name = getSBServiceBindingName(cfServiceBinding) - bindingName := secret.Name - if cfServiceBinding.Spec.DisplayName != nil { - bindingName = *cfServiceBinding.Spec.DisplayName + secretType, hasType := credentialsSecret.Data["type"] + if hasType && len(secretType) > 0 { + sbServiceBinding.Spec.Type = string(secretType) + } + + secretProvider, hasProvider := credentialsSecret.Data["provider"] + if hasProvider { + sbServiceBinding.Spec.Provider = string(secretProvider) + } + return controllerutil.SetControllerReference(cfServiceBinding, sbServiceBinding, r.scheme) + }) + if err != nil { + return nil, err } - desiredServiceBinding.Spec = servicebindingv1beta1.ServiceBindingSpec{ - Name: bindingName, - Type: "user-provided", - Workload: servicebindingv1beta1.ServiceBindingWorkloadReference{ - APIVersion: "apps/v1", - Kind: "StatefulSet", - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - korifiv1alpha1.CFAppGUIDLabelKey: cfApp.Name, - }, + return sbServiceBinding, nil +} + +func (r *CFServiceBindingReconciler) toSBServiceBinding(cfServiceBinding *korifiv1alpha1.CFServiceBinding) *servicebindingv1beta1.ServiceBinding { + return &servicebindingv1beta1.ServiceBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("cf-binding-%s", cfServiceBinding.Name), + Namespace: cfServiceBinding.Namespace, + Labels: map[string]string{ + ServiceBindingGUIDLabel: cfServiceBinding.Name, + korifiv1alpha1.CFAppGUIDLabelKey: cfServiceBinding.Spec.AppRef.Name, + ServiceCredentialBindingTypeLabel: "app", }, }, - Service: servicebindingv1beta1.ServiceBindingServiceReference{ - APIVersion: "korifi.cloudfoundry.org/v1alpha1", - Kind: "CFServiceBinding", - Name: cfServiceBinding.Name, + Spec: servicebindingv1beta1.ServiceBindingSpec{ + Type: "user-provided", + Workload: servicebindingv1beta1.ServiceBindingWorkloadReference{ + APIVersion: "apps/v1", + Kind: "StatefulSet", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + korifiv1alpha1.CFAppGUIDLabelKey: cfServiceBinding.Spec.AppRef.Name, + }, + }, + }, + Service: servicebindingv1beta1.ServiceBindingServiceReference{ + APIVersion: "korifi.cloudfoundry.org/v1alpha1", + Kind: "CFServiceBinding", + Name: cfServiceBinding.Name, + }, }, } +} - secretType, ok := secret.Data["type"] - if ok && len(secretType) > 0 { - desiredServiceBinding.Spec.Type = string(secretType) - } - - secretProvider, ok := secret.Data["provider"] - if ok { - desiredServiceBinding.Spec.Provider = string(secretProvider) +func getSBServiceBindingName(cfServiceBinding *korifiv1alpha1.CFServiceBinding) string { + if cfServiceBinding.Spec.DisplayName != nil { + return *cfServiceBinding.Spec.DisplayName } - return &desiredServiceBinding + return cfServiceBinding.Status.Binding.Name } diff --git a/controllers/controllers/services/bindings/controller_test.go b/controllers/controllers/services/bindings/controller_test.go index 3bf6ac36b..f55d328de 100644 --- a/controllers/controllers/services/bindings/controller_test.go +++ b/controllers/controllers/services/bindings/controller_test.go @@ -6,11 +6,10 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/controllers/services/bindings" - . "code.cloudfoundry.org/korifi/controllers/controllers/workloads/testutils" - . "code.cloudfoundry.org/korifi/tests/matchers" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" + . "code.cloudfoundry.org/korifi/tests/matchers" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -39,7 +38,19 @@ var _ = Describe("CFServiceBinding", func() { }, })).To(Succeed()) - cfApp = BuildCFAppCRObject(uuid.NewString(), testNamespace) + cfApp = &korifiv1alpha1.CFApp{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: testNamespace, + }, + Spec: korifiv1alpha1.CFAppSpec{ + DisplayName: uuid.NewString(), + DesiredState: korifiv1alpha1.StartedState, + Lifecycle: korifiv1alpha1.Lifecycle{ + Type: "docker", + }, + }, + } Expect( adminClient.Create(ctx, cfApp), ).To(Succeed()) @@ -119,13 +130,6 @@ var _ = Describe("CFServiceBinding", func() { }).Should(Succeed()) }) - It("sets the BindingSecretAvailable condition to true", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(binding.Status.Conditions, bindings.BindingSecretAvailableCondition)).To(BeTrue()) - }).Should(Succeed()) - }) - It("sets the binding status credentials name to the instance credentials secret", func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) @@ -153,6 +157,17 @@ var _ = Describe("CFServiceBinding", func() { }).Should(Succeed()) }) + It("sets the binding Ready status condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("ServiceBindingNotReady")), + ))) + }).Should(Succeed()) + }) + It("creates a servicebinding.io ServiceBinding", func() { Eventually(func(g Gomega) { sbServiceBinding := &servicebindingv1beta1.ServiceBinding{ @@ -196,6 +211,136 @@ var _ = Describe("CFServiceBinding", func() { }).Should(Succeed()) }) + It("sets the binding status binding name to the binding secret name", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Binding.Name).To(Equal(binding.Name)) + }).Should(Succeed()) + }) + + When("the CFServiceBinding has a displayName set", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, binding, func() { + binding.Spec.DisplayName = tools.PtrTo("a-custom-binding-name") + })).To(Succeed()) + }) + + It("sets the displayName as the name on the servicebinding.io ServiceBinding", func() { + Eventually(func(g Gomega) { + sbServiceBinding := &servicebindingv1beta1.ServiceBinding{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: fmt.Sprintf("cf-binding-%s", binding.Name), + }, + } + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed()) + g.Expect(sbServiceBinding.Spec.Name).To(Equal("a-custom-binding-name")) + }).Should(Succeed()) + }) + }) + + When("the servicebinding.io binding is ready", func() { + BeforeEach(func() { + Eventually(func(g Gomega) { + sbServiceBinding := &servicebindingv1beta1.ServiceBinding{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: fmt.Sprintf("cf-binding-%s", binding.Name), + }, + } + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed()) + g.Expect(k8s.Patch(ctx, adminClient, sbServiceBinding, func() { + meta.SetStatusCondition(&sbServiceBinding.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "whatever", + Message: "", + }) + sbServiceBinding.Status.ObservedGeneration = sbServiceBinding.Generation + })).To(Succeed()) + }).Should(Succeed()) + }) + + It("sets the binding Ready status condition to true", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + + When("the servicebinding.io binding ready status is outdated", func() { + BeforeEach(func() { + Eventually(func(g Gomega) { + sbServiceBinding := &servicebindingv1beta1.ServiceBinding{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: fmt.Sprintf("cf-binding-%s", binding.Name), + }, + } + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed()) + g.Expect(k8s.Patch(ctx, adminClient, sbServiceBinding, func() { + sbServiceBinding.Status.ObservedGeneration = sbServiceBinding.Generation - 1 + })).To(Succeed()) + }).Should(Succeed()) + }) + + It("sets the binding Ready status condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("ServiceBindingNotReady")), + ))) + }).Should(Succeed()) + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("ServiceBindingNotReady")), + ))) + }).Should(Succeed()) + }) + }) + }) + + When("the servicebinding.io binding is not ready", func() { + BeforeEach(func() { + Eventually(func(g Gomega) { + sbServiceBinding := &servicebindingv1beta1.ServiceBinding{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: fmt.Sprintf("cf-binding-%s", binding.Name), + }, + } + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed()) + g.Expect(k8s.Patch(ctx, adminClient, sbServiceBinding, func() { + meta.SetStatusCondition(&sbServiceBinding.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionFalse, + Reason: "whatever", + Message: "", + }) + })).To(Succeed()) + }).Should(Succeed()) + }) + + It("sets the binding Ready status condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("ServiceBindingNotReady")), + ))) + }).Should(Succeed()) + }) + }) + When("the credentials secret has its 'type' attribute set", func() { BeforeEach(func() { Eventually(func(g Gomega) { @@ -266,6 +411,7 @@ var _ = Describe("CFServiceBinding", func() { }, } g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed()) + fmt.Fprintf(GinkgoWriter, "servicebinding.io: %+v\n", sbServiceBinding) g.Expect(sbServiceBinding.Spec.Type).To(Equal("my-type")) }).Should(Succeed()) }) @@ -325,11 +471,20 @@ var _ = Describe("CFServiceBinding", func() { }) }) - It("sets the binding status binding name to the binding secret name", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) - g.Expect(binding.Status.Binding.Name).To(Equal(binding.Name)) - }).Should(Succeed()) + When("the service instance is not available", func() { + BeforeEach(func() { + Expect(adminClient.Delete(ctx, instance)).To(Succeed()) + }) + + It("sets not ready status", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) }) When("the credentials secret is not available", func() { @@ -339,35 +494,48 @@ var _ = Describe("CFServiceBinding", func() { })).To(Succeed()) }) - It("sets the BindingSecretAvailable condition to false", func() { + It("sets the Ready condition to false", func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(bindings.BindingSecretAvailableCondition)), + HasType(Equal(korifiv1alpha1.StatusConditionReady)), HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("CredentialsSecretNotAvailable")), ))) }).Should(Succeed()) }) }) - When("the CFServiceBinding has a displayName set", func() { + When("the CFApp is not available", func() { BeforeEach(func() { - Expect(k8s.PatchResource(ctx, adminClient, binding, func() { - binding.Spec.DisplayName = tools.PtrTo("a-custom-binding-name") + Expect(adminClient.Delete(ctx, cfApp)).To(Succeed()) + }) + + It("sets the Ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ))) + }).Should(Succeed()) + }) + }) + + When("the app VCAP_SERVICES secret is not set in the CFApp status", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, cfApp, func() { + cfApp.Status.VCAPServicesSecretName = "" })).To(Succeed()) }) - It("sets the displayName as the name on the servicebinding.io ServiceBinding", func() { + It("sets the Ready condition to false", func() { Eventually(func(g Gomega) { - sbServiceBinding := &servicebindingv1beta1.ServiceBinding{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testNamespace, - Name: fmt.Sprintf("cf-binding-%s", binding.Name), - }, - } - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed()) - g.Expect(sbServiceBinding.Spec.Name).To(Equal("a-custom-binding-name")) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("VcapServicesSecretNotAvailable")), + ))) }).Should(Succeed()) }) }) @@ -391,13 +559,12 @@ var _ = Describe("CFServiceBinding", func() { }).Should(Succeed()) }) - It("sets credentials secret not available condition", func() { + It("sets the binding Ready status condition to false", func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( - HasType(Equal(bindings.BindingSecretAvailableCondition)), + HasType(Equal(korifiv1alpha1.StatusConditionReady)), HasStatus(Equal(metav1.ConditionFalse)), - HasReason(Equal("FailedReconcilingCredentialsSecret")), ))) }).Should(Succeed()) }) diff --git a/controllers/controllers/services/bindings/suite_test.go b/controllers/controllers/services/bindings/suite_test.go index 76d151ccb..8dadee338 100644 --- a/controllers/controllers/services/bindings/suite_test.go +++ b/controllers/controllers/services/bindings/suite_test.go @@ -47,7 +47,7 @@ var ( ) func TestAPIs(t *testing.T) { - SetDefaultEventuallyTimeout(30 * time.Second) + SetDefaultEventuallyTimeout(10 * time.Second) SetDefaultEventuallyPollingInterval(250 * time.Millisecond) RegisterFailHandler(Fail) diff --git a/controllers/controllers/shared/index.go b/controllers/controllers/shared/index.go index 9ecb75528..94f281263 100644 --- a/controllers/controllers/shared/index.go +++ b/controllers/controllers/shared/index.go @@ -22,8 +22,6 @@ const ( IndexAppTasks = "appTasks" IndexSpaceNamespaceName = "spaceNamespace" IndexOrgNamespaceName = "orgNamespace" - - StatusConditionReady = "Ready" ) func SetupIndexWithManager(mgr manager.Manager) error { diff --git a/controllers/controllers/workloads/cfapp_controller.go b/controllers/controllers/workloads/cfapp_controller.go index 903104717..884ea92fe 100644 --- a/controllers/controllers/workloads/cfapp_controller.go +++ b/controllers/controllers/workloads/cfapp_controller.go @@ -107,6 +107,14 @@ func (r *CFAppReconciler) ReconcileResource(ctx context.Context, cfApp *korifiv1 cfApp.Status.ObservedGeneration = cfApp.Generation log.V(1).Info("set observed generation", "generation", cfApp.Status.ObservedGeneration) + cfApp.Status.ActualState = korifiv1alpha1.StoppedState + + var err error + readyConditionBuilder := k8s.NewReadyConditionBuilder(cfApp) + defer func() { + meta.SetStatusCondition(&cfApp.Status.Conditions, readyConditionBuilder.WithError(err).Build()) + }() + if !cfApp.GetDeletionTimestamp().IsZero() { return r.finalizeCFApp(ctx, cfApp) } @@ -119,7 +127,7 @@ func (r *CFAppReconciler) ReconcileResource(ctx context.Context, cfApp *korifiv1 } secretName := cfApp.Name + "-vcap-application" - err := r.reconcileVCAPSecret(ctx, cfApp, secretName, r.vcapApplicationEnvBuilder) + err = r.reconcileVCAPSecret(ctx, cfApp, secretName, r.vcapApplicationEnvBuilder) if err != nil { return ctrl.Result{}, err } @@ -133,37 +141,17 @@ func (r *CFAppReconciler) ReconcileResource(ctx context.Context, cfApp *korifiv1 cfApp.Status.VCAPServicesSecretName = secretName - cfApp.Status.ActualState = korifiv1alpha1.StoppedState if cfApp.Spec.CurrentDropletRef.Name == "" { - meta.SetStatusCondition(&cfApp.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, - Status: metav1.ConditionFalse, - Reason: "DropletNotAssigned", - ObservedGeneration: cfApp.Generation, - }) - + readyConditionBuilder.WithReason("DropletNotAssigned") return ctrl.Result{}, nil } droplet, err := r.getDroplet(ctx, cfApp) if err != nil { - meta.SetStatusCondition(&cfApp.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, - Status: metav1.ConditionFalse, - Reason: "CannotResolveCurrentDropletRef", - ObservedGeneration: cfApp.Generation, - }) - + readyConditionBuilder.WithReason("CannotResolveCurrentDropletRef") return ctrl.Result{}, err } - meta.SetStatusCondition(&cfApp.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, - Status: metav1.ConditionTrue, - Reason: "DropletAssigned", - ObservedGeneration: cfApp.Generation, - }) - reconciledProcesses, err := r.reconcileProcesses(ctx, cfApp, droplet) if err != nil { return ctrl.Result{}, err @@ -171,6 +159,7 @@ func (r *CFAppReconciler) ReconcileResource(ctx context.Context, cfApp *korifiv1 cfApp.Status.ActualState = getActualState(reconciledProcesses) + readyConditionBuilder.Ready() return ctrl.Result{}, nil } diff --git a/controllers/controllers/workloads/cfapp_controller_test.go b/controllers/controllers/workloads/cfapp_controller_test.go index e81984682..f6c2b5c80 100644 --- a/controllers/controllers/workloads/cfapp_controller_test.go +++ b/controllers/controllers/workloads/cfapp_controller_test.go @@ -5,11 +5,11 @@ import ( "time" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/shared" . "code.cloudfoundry.org/korifi/controllers/controllers/workloads/testutils" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" + . "code.cloudfoundry.org/korifi/tests/matchers" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -123,9 +123,8 @@ var _ = Describe("CFAppReconciler Integration Tests", func() { It("sets the last-stop-app-rev annotation to the value of the app-rev annotation", func() { Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(createdCFApp.Annotations[korifiv1alpha1.CFAppLastStopRevisionKey]).To(Equal("42")) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + g.Expect(cfApp.Annotations[korifiv1alpha1.CFAppLastStopRevisionKey]).To(Equal("42")) }).Should(Succeed()) }) @@ -136,9 +135,8 @@ var _ = Describe("CFAppReconciler Integration Tests", func() { It("doesn't set it", func() { Consistently(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(createdCFApp.Annotations[korifiv1alpha1.CFAppLastStopRevisionKey]).To(Equal("2")) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + g.Expect(cfApp.Annotations[korifiv1alpha1.CFAppLastStopRevisionKey]).To(Equal("2")) }, "1s").Should(Succeed()) }) }) @@ -147,10 +145,9 @@ var _ = Describe("CFAppReconciler Integration Tests", func() { var createdSecret corev1.Secret Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) - vcapApplicationSecretName := createdCFApp.Status.VCAPApplicationSecretName + vcapApplicationSecretName := cfApp.Status.VCAPApplicationSecretName g.Expect(vcapApplicationSecretName).NotTo(BeEmpty()) vcapApplicationSecretLookupKey := types.NamespacedName{Name: vcapApplicationSecretName, Namespace: cfSpace.Status.GUID} @@ -165,10 +162,9 @@ var _ = Describe("CFAppReconciler Integration Tests", func() { getVCAPServicesSecret := func() *corev1.Secret { secret := new(corev1.Secret) Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) - vcapServicesSecretName := createdCFApp.Status.VCAPServicesSecretName + vcapServicesSecretName := cfApp.Status.VCAPServicesSecretName g.Expect(vcapServicesSecretName).NotTo(BeEmpty()) vcapServicesSecretLookupKey := types.NamespacedName{Name: vcapServicesSecretName, Namespace: cfSpace.Status.GUID} @@ -188,21 +184,19 @@ var _ = Describe("CFAppReconciler Integration Tests", func() { It("sets its status conditions", func() { Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) - readyStatusCondition := meta.FindStatusCondition(createdCFApp.Status.Conditions, shared.StatusConditionReady) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + readyStatusCondition := meta.FindStatusCondition(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady) g.Expect(readyStatusCondition).NotTo(BeNil()) g.Expect(readyStatusCondition.Status).To(Equal(metav1.ConditionFalse)) g.Expect(readyStatusCondition.Reason).To(Equal("DropletNotAssigned")) - g.Expect(readyStatusCondition.ObservedGeneration).To(Equal(createdCFApp.Generation)) + g.Expect(readyStatusCondition.ObservedGeneration).To(Equal(cfApp.Generation)) }).Should(Succeed()) }) It("sets the ObservedGeneration status field", func() { Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(createdCFApp.Status.ObservedGeneration).To(Equal(createdCFApp.Generation)) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + g.Expect(cfApp.Status.ObservedGeneration).To(Equal(cfApp.Generation)) }).Should(Succeed()) }) @@ -241,13 +235,12 @@ var _ = Describe("CFAppReconciler Integration Tests", func() { It("sets the ready condition to false", func() { Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) - readyStatusCondition := meta.FindStatusCondition(createdCFApp.Status.Conditions, shared.StatusConditionReady) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + readyStatusCondition := meta.FindStatusCondition(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady) g.Expect(readyStatusCondition).NotTo(BeNil()) g.Expect(readyStatusCondition.Status).To(Equal(metav1.ConditionFalse)) g.Expect(readyStatusCondition.Reason).To(Equal("CannotResolveCurrentDropletRef")) - g.Expect(readyStatusCondition.ObservedGeneration).To(Equal(createdCFApp.Generation)) + g.Expect(readyStatusCondition.ObservedGeneration).To(Equal(cfApp.Generation)) }).Should(Succeed()) }) }) @@ -436,38 +429,33 @@ var _ = Describe("CFAppReconciler Integration Tests", func() { It("sets the ready condition to true", func() { Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) - readyStatusCondition := meta.FindStatusCondition(createdCFApp.Status.Conditions, shared.StatusConditionReady) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + readyStatusCondition := meta.FindStatusCondition(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady) g.Expect(readyStatusCondition).NotTo(BeNil()) g.Expect(readyStatusCondition.Status).To(Equal(metav1.ConditionTrue)) - g.Expect(readyStatusCondition.Reason).To(Equal("DropletAssigned")) - g.Expect(readyStatusCondition.ObservedGeneration).To(Equal(createdCFApp.Generation)) }).Should(Succeed()) }) When("the droplet disappears", func() { JustBeforeEach(func() { Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) - readyStatusCondition := meta.FindStatusCondition(createdCFApp.Status.Conditions, shared.StatusConditionReady) - g.Expect(readyStatusCondition).NotTo(BeNil()) - g.Expect(readyStatusCondition.Status).To(Equal(metav1.ConditionTrue)) - g.Expect(readyStatusCondition.ObservedGeneration).To(Equal(createdCFApp.Generation)) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + g.Expect(cfApp.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) }).Should(Succeed()) Expect(adminClient.Delete(context.Background(), cfBuild)).To(Succeed()) }) It("sets the ready condition to false", func() { Eventually(func(g Gomega) { - createdCFApp, err := getApp(cfSpace.Status.GUID, cfAppGUID) - g.Expect(err).NotTo(HaveOccurred()) - readyStatusCondition := meta.FindStatusCondition(createdCFApp.Status.Conditions, shared.StatusConditionReady) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + readyStatusCondition := meta.FindStatusCondition(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady) g.Expect(readyStatusCondition).NotTo(BeNil()) g.Expect(readyStatusCondition.Status).To(Equal(metav1.ConditionFalse)) g.Expect(readyStatusCondition.Reason).To(Equal("CannotResolveCurrentDropletRef")) - g.Expect(readyStatusCondition.ObservedGeneration).To(Equal(createdCFApp.Generation)) + g.Expect(readyStatusCondition.ObservedGeneration).To(Equal(cfApp.Generation)) }).Should(Succeed()) }) }) @@ -563,13 +551,6 @@ var _ = Describe("CFAppReconciler Integration Tests", func() { }) }) -func getApp(nsGUID, appGUID string) (*korifiv1alpha1.CFApp, error) { - cfAppLookupKey := types.NamespacedName{Name: appGUID, Namespace: nsGUID} - createdCFApp := &korifiv1alpha1.CFApp{} - err := adminClient.Get(context.Background(), cfAppLookupKey, createdCFApp) - return createdCFApp, err -} - func findProcessWithType(cfApp *korifiv1alpha1.CFApp, processType string) *korifiv1alpha1.CFProcess { cfProcessList := &korifiv1alpha1.CFProcessList{} diff --git a/controllers/controllers/workloads/cforg_controller.go b/controllers/controllers/workloads/cforg_controller.go index 798e94f1d..5a5c7c8a4 100644 --- a/controllers/controllers/workloads/cforg_controller.go +++ b/controllers/controllers/workloads/cforg_controller.go @@ -20,7 +20,6 @@ import ( "context" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/k8sns" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/labels" "code.cloudfoundry.org/korifi/tools/k8s" @@ -129,9 +128,9 @@ func (r *CFOrgReconciler) ReconcileResource(ctx context.Context, cfOrg *korifiv1 } meta.SetStatusCondition(&cfOrg.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionTrue, - Reason: shared.StatusConditionReady, + Reason: korifiv1alpha1.StatusConditionReady, ObservedGeneration: cfOrg.Generation, }) diff --git a/controllers/controllers/workloads/cfpackage_controller.go b/controllers/controllers/workloads/cfpackage_controller.go index e552f96d7..079c99e84 100644 --- a/controllers/controllers/workloads/cfpackage_controller.go +++ b/controllers/controllers/workloads/cfpackage_controller.go @@ -20,7 +20,6 @@ import ( "context" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/tools/image" "code.cloudfoundry.org/korifi/tools/k8s" @@ -125,7 +124,7 @@ func (r *CFPackageReconciler) ReconcileResource(ctx context.Context, cfPackage * readyReason = "SourceImageSet" } meta.SetStatusCondition(&cfPackage.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, + Type: korifiv1alpha1.StatusConditionReady, Status: readyCondition, Reason: readyReason, ObservedGeneration: cfPackage.Generation, diff --git a/controllers/controllers/workloads/cfpackage_controller_test.go b/controllers/controllers/workloads/cfpackage_controller_test.go index 6ee4346ee..77fe84a96 100644 --- a/controllers/controllers/workloads/cfpackage_controller_test.go +++ b/controllers/controllers/workloads/cfpackage_controller_test.go @@ -5,7 +5,6 @@ import ( "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/controllers/controllers/workloads/testutils" "code.cloudfoundry.org/korifi/tools" @@ -59,8 +58,8 @@ var _ = Describe("CFPackageReconciler Integration Tests", func() { Expect(meta.FindStatusCondition(createdCFPackage.Status.Conditions, workloads.InitializedConditionType).ObservedGeneration).To(Equal(createdCFPackage.Generation)) - Expect(meta.IsStatusConditionFalse(createdCFPackage.Status.Conditions, shared.StatusConditionReady)).To(BeTrue()) - Expect(meta.FindStatusCondition(createdCFPackage.Status.Conditions, shared.StatusConditionReady).ObservedGeneration).To(Equal(createdCFPackage.Generation)) + Expect(meta.IsStatusConditionFalse(createdCFPackage.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) + Expect(meta.FindStatusCondition(createdCFPackage.Status.Conditions, korifiv1alpha1.StatusConditionReady).ObservedGeneration).To(Equal(createdCFPackage.Generation)) Expect(createdCFPackage.GetOwnerReferences()).To(ConsistOf(metav1.OwnerReference{ APIVersion: korifiv1alpha1.GroupVersion.Identifier(), @@ -112,7 +111,7 @@ var _ = Describe("CFPackageReconciler Integration Tests", func() { It("sets the ready condition to true", func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfPackage), &createdCFPackage)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(createdCFPackage.Status.Conditions, shared.StatusConditionReady)).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(createdCFPackage.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) }) }) diff --git a/controllers/controllers/workloads/cfprocess_controller.go b/controllers/controllers/workloads/cfprocess_controller.go index 3e0da9ad0..98c559b55 100644 --- a/controllers/controllers/workloads/cfprocess_controller.go +++ b/controllers/controllers/workloads/cfprocess_controller.go @@ -168,9 +168,9 @@ func (r *CFProcessReconciler) ReconcileResource(ctx context.Context, cfProcess * } meta.SetStatusCondition(&cfProcess.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionTrue, - Reason: shared.StatusConditionReady, + Reason: korifiv1alpha1.StatusConditionReady, ObservedGeneration: cfProcess.Generation, }) diff --git a/controllers/controllers/workloads/cfspace_controller.go b/controllers/controllers/workloads/cfspace_controller.go index 9c0ce4d75..c9ef79e08 100644 --- a/controllers/controllers/workloads/cfspace_controller.go +++ b/controllers/controllers/workloads/cfspace_controller.go @@ -152,7 +152,7 @@ func (r *CFSpaceReconciler) ReconcileResource(ctx context.Context, cfSpace *kori log.Info("not ready yet", "reason", "error propagating service accounts", "error", err) meta.SetStatusCondition(&cfSpace.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionFalse, Reason: "ServiceAccountPropagation", Message: err.Error(), @@ -163,9 +163,9 @@ func (r *CFSpaceReconciler) ReconcileResource(ctx context.Context, cfSpace *kori } meta.SetStatusCondition(&cfSpace.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionTrue, - Reason: shared.StatusConditionReady, + Reason: korifiv1alpha1.StatusConditionReady, ObservedGeneration: cfSpace.Generation, }) diff --git a/controllers/controllers/workloads/cftask_controller.go b/controllers/controllers/workloads/cftask_controller.go index 61e018bf0..9310357d4 100644 --- a/controllers/controllers/workloads/cftask_controller.go +++ b/controllers/controllers/workloads/cftask_controller.go @@ -23,7 +23,6 @@ import ( "time" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/go-logr/logr" @@ -184,7 +183,7 @@ func (r *CFTaskReconciler) getApp(ctx context.Context, cfTask *korifiv1alpha1.CF return nil, err } - if !meta.IsStatusConditionTrue(cfApp.Status.Conditions, shared.StatusConditionReady) { + if !meta.IsStatusConditionTrue(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady) { log.Info("CFApp not staged", "appNamespace", cfApp.Namespace, "appName", cfApp.Name) r.recorder.Eventf(cfTask, "Warning", "AppNotStaged", "App %s:%s is not staged", cfApp.Namespace, cfApp.Name) return nil, errors.New("app not staged") diff --git a/controllers/controllers/workloads/k8sns/reconciler.go b/controllers/controllers/workloads/k8sns/reconciler.go index cd9003a4b..4bf8608a6 100644 --- a/controllers/controllers/workloads/k8sns/reconciler.go +++ b/controllers/controllers/workloads/k8sns/reconciler.go @@ -143,7 +143,7 @@ func (r *Reconciler[T, NS]) setNotReady(log logr.Logger, obj NS, err error, reas log.Info("not ready yet", "reason", reason, "error", err) meta.SetStatusCondition(obj.GetStatus().GetConditions(), metav1.Condition{ - Type: shared.StatusConditionReady, + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionFalse, Reason: reason, Message: err.Error(), diff --git a/controllers/controllers/workloads/suite_test.go b/controllers/controllers/workloads/suite_test.go index 7713f8d46..640aa76d8 100644 --- a/controllers/controllers/workloads/suite_test.go +++ b/controllers/controllers/workloads/suite_test.go @@ -357,7 +357,7 @@ func createOrg(rootNamespace string) *korifiv1alpha1.CFOrg { Expect(adminClient.Create(ctx, org)).To(Succeed()) Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(org), org)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(org.Status.Conditions, shared.StatusConditionReady)).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(org.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) return org } @@ -375,7 +375,7 @@ func createSpace(org *korifiv1alpha1.CFOrg) *korifiv1alpha1.CFSpace { Expect(adminClient.Create(ctx, cfSpace)).To(Succeed()) Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfSpace), cfSpace)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(cfSpace.Status.Conditions, shared.StatusConditionReady)).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(cfSpace.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) return cfSpace } diff --git a/statefulset-runner/controllers/appworkload_controller.go b/statefulset-runner/controllers/appworkload_controller.go index adbc2cd95..34e36e769 100644 --- a/statefulset-runner/controllers/appworkload_controller.go +++ b/statefulset-runner/controllers/appworkload_controller.go @@ -206,9 +206,9 @@ func (r *AppWorkloadReconciler) ReconcileResource(ctx context.Context, appWorklo appWorkload.Status.ActualInstances = updatedStatefulSet.Status.Replicas meta.SetStatusCondition(&appWorkload.Status.Conditions, metav1.Condition{ - Type: shared.StatusConditionReady, + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionTrue, - Reason: shared.StatusConditionReady, + Reason: korifiv1alpha1.StatusConditionReady, ObservedGeneration: appWorkload.Generation, }) diff --git a/tests/crds/crds_suite_test.go b/tests/crds/crds_suite_test.go index d7027877f..4d1c49cc5 100644 --- a/tests/crds/crds_suite_test.go +++ b/tests/crds/crds_suite_test.go @@ -8,7 +8,6 @@ import ( "time" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/tests/helpers" "code.cloudfoundry.org/korifi/tests/helpers/fail_handler" "code.cloudfoundry.org/korifi/tools" @@ -141,7 +140,7 @@ var _ = BeforeEach(func() { Expect(k8sClient.Create(ctx, testOrg)).To(Succeed()) Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(testOrg), testOrg)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(testOrg.StatusConditions(), shared.StatusConditionReady)).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(testOrg.StatusConditions(), korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) testSpace = &korifiv1alpha1.CFSpace{ @@ -157,7 +156,7 @@ var _ = BeforeEach(func() { Expect(k8sClient.Create(ctx, testSpace)).To(Succeed()) Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(testSpace), testSpace)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(testSpace.StatusConditions(), shared.StatusConditionReady)).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(testSpace.StatusConditions(), korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) }) diff --git a/tools/k8s/condition_builder.go b/tools/k8s/condition_builder.go new file mode 100644 index 000000000..995a034c3 --- /dev/null +++ b/tools/k8s/condition_builder.go @@ -0,0 +1,63 @@ +package k8s + +import ( + "time" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type ReadyConditionBuilder struct { + status metav1.ConditionStatus + observedGeneration int64 + reason string + message string +} + +func NewReadyConditionBuilder(obj client.Object) *ReadyConditionBuilder { + return &ReadyConditionBuilder{ + status: metav1.ConditionFalse, + observedGeneration: obj.GetGeneration(), + reason: "Unknown", + } +} + +func (b *ReadyConditionBuilder) WithStatus(status metav1.ConditionStatus) *ReadyConditionBuilder { + b.status = status + return b +} + +func (b *ReadyConditionBuilder) WithReason(reason string) *ReadyConditionBuilder { + b.reason = reason + return b +} + +func (b *ReadyConditionBuilder) WithMessage(message string) *ReadyConditionBuilder { + b.message = message + return b +} + +func (b *ReadyConditionBuilder) WithError(err error) *ReadyConditionBuilder { + if err == nil { + return b + } + + b.message = err.Error() + return b +} + +func (b *ReadyConditionBuilder) Ready() *ReadyConditionBuilder { + return b.WithStatus(metav1.ConditionTrue).WithReason("Ready") +} + +func (b *ReadyConditionBuilder) Build() metav1.Condition { + return metav1.Condition{ + Type: korifiv1alpha1.StatusConditionReady, + Status: b.status, + ObservedGeneration: b.observedGeneration, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: b.reason, + Message: b.message, + } +} diff --git a/tools/k8s/condition_builder_test.go b/tools/k8s/condition_builder_test.go new file mode 100644 index 000000000..fb53be9c2 --- /dev/null +++ b/tools/k8s/condition_builder_test.go @@ -0,0 +1,102 @@ +package k8s_test + +import ( + "errors" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/tools/k8s" +) + +var _ = Describe("ReadyConditionBuilder", func() { + var ( + builder *k8s.ReadyConditionBuilder + condition metav1.Condition + ) + + BeforeEach(func() { + builder = k8s.NewReadyConditionBuilder(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 4, + }, + }) + }) + + JustBeforeEach(func() { + condition = builder.Build() + }) + + It("creates a default condition", func() { + Expect(condition.Type).To(Equal(korifiv1alpha1.StatusConditionReady)) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.ObservedGeneration).To(BeEquivalentTo(4)) + Expect(condition.LastTransitionTime).NotTo(BeZero()) + Expect(condition.Reason).To(Equal("Unknown")) + Expect(condition.Message).To(BeEmpty()) + }) + + When("the status is set to true", func() { + BeforeEach(func() { + builder.WithStatus(metav1.ConditionTrue) + }) + + It("sets condition status to true", func() { + Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + }) + }) + + When("the reason is set", func() { + BeforeEach(func() { + builder.WithReason("SomeReason") + }) + + It("sets the reason", func() { + Expect(condition.Reason).To(Equal("SomeReason")) + }) + }) + + When("the message is set", func() { + BeforeEach(func() { + builder.WithMessage("Some Message") + }) + + It("sets the reason", func() { + Expect(condition.Message).To(Equal("Some Message")) + }) + }) + + When("the error is set", func() { + BeforeEach(func() { + builder.WithError(errors.New("some-error")) + }) + + It("sets the reason", func() { + Expect(condition.Message).To(Equal("some-error")) + }) + }) + + When("nil error is set", func() { + BeforeEach(func() { + builder.WithError(nil) + }) + + It("sets the reason", func() { + Expect(condition.Message).To(BeEmpty()) + }) + }) + + When("ready is set", func() { + BeforeEach(func() { + builder.Ready() + }) + + It("sets ready status and reason", func() { + Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + Expect(condition.Reason).To(Equal("Ready")) + }) + }) +})