From 9c45c186e111dbe6de9e68f2a436fb24c0bb1bac Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Mon, 27 May 2024 09:44:11 +0000 Subject: [PATCH] Use the ready condition builder in relevant controllers Korifi custom resources that have their `Ready` condition set by controllers now utilise the ready condition builder pattern we recently came up with. The afffected resources are: * CFPackage * CFOrg * CFSpace * CFProcess * AppWorkload While being here, * take the opportunity to slightly improve their tests * use the `korifiv1alpha1.StatusConditionReady` constant wherever it makes sense fixes #3237 wip * Address added TODOs wip --- api/repositories/buildpack_repository_test.go | 7 +- api/repositories/org_repository_test.go | 6 +- api/repositories/package_repository_test.go | 8 +- api/repositories/repositories_suite_test.go | 4 +- api/repositories/space_repository_test.go | 4 +- .../cf_buildpack_build_controller_test.go | 150 +++++++++--------- .../controllers/workloads/cforg_controller.go | 15 +- .../workloads/cforg_controller_test.go | 24 ++- .../workloads/cfpackage_controller.go | 39 ++--- .../workloads/cfpackage_controller_test.go | 92 +++++++---- .../workloads/cfprocess_controller.go | 17 +- .../workloads/cfprocess_controller_test.go | 65 ++++---- .../workloads/cfspace_controller.go | 23 +-- .../workloads/cfspace_controller_test.go | 72 ++++----- .../workloads/cftask_controller_test.go | 8 +- .../workloads/env/env_suite_test.go | 2 +- .../workloads/k8sns/reconciler_test.go | 5 +- .../workloads/testutils/shared_test_utils.go | 116 +------------- .../controllers/appworkload_controller.go | 43 ++--- .../appworkload_controller_test.go | 72 +++++++-- .../controllers/appworkload_to_stset_test.go | 60 ++++++- .../appworkload_controller_test.go | 27 +++- statefulset-runner/controllers/pdb.go | 28 ++-- statefulset-runner/controllers/pdb_test.go | 12 +- statefulset-runner/controllers/suite_test.go | 67 +------- 25 files changed, 459 insertions(+), 507 deletions(-) diff --git a/api/repositories/buildpack_repository_test.go b/api/repositories/buildpack_repository_test.go index 679631c15..85a1a72ce 100644 --- a/api/repositories/buildpack_repository_test.go +++ b/api/repositories/buildpack_repository_test.go @@ -9,6 +9,7 @@ import ( . "code.cloudfoundry.org/korifi/api/repositories" "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -96,7 +97,7 @@ var _ = Describe("BuildpackRepository", func() { When("there is a ready condition with a message", func() { BeforeEach(func() { meta.SetStatusCondition(&builderInfo.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionFalse, Reason: "testing", Message: "this is a test", @@ -113,7 +114,7 @@ var _ = Describe("BuildpackRepository", func() { When("there is a ready condition with an empty message", func() { BeforeEach(func() { meta.SetStatusCondition(&builderInfo.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionFalse, Reason: "testing", Message: "", @@ -168,7 +169,7 @@ func createBuilderInfoWithCleanup(ctx context.Context, name, stack string, build } meta.SetStatusCondition(&builderInfo.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionTrue, Reason: "testing", }) diff --git a/api/repositories/org_repository_test.go b/api/repositories/org_repository_test.go index 2eba7a7d3..325cbe3a2 100644 --- a/api/repositories/org_repository_test.go +++ b/api/repositories/org_repository_test.go @@ -67,7 +67,7 @@ var _ = Describe("OrgRepository", func() { Expect(k8s.Patch(ctx, k8sClient, cfOrg, func() { cfOrg.Status.GUID = cfOrg.Name meta.SetStatusCondition(&cfOrg.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: conditionStatus, Reason: "blah", Message: conditionMessage, @@ -199,7 +199,7 @@ var _ = Describe("OrgRepository", func() { When("the org is not ready", func() { BeforeEach(func() { meta.SetStatusCondition(&(cfOrg1.Status.Conditions), metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionFalse, Reason: "because", Message: "because", @@ -207,7 +207,7 @@ var _ = Describe("OrgRepository", func() { Expect(k8sClient.Status().Update(ctx, cfOrg1)).To(Succeed()) meta.SetStatusCondition(&(cfOrg2.Status.Conditions), metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionUnknown, Reason: "because", Message: "because", diff --git a/api/repositories/package_repository_test.go b/api/repositories/package_repository_test.go index 61279f39a..107c900f7 100644 --- a/api/repositories/package_repository_test.go +++ b/api/repositories/package_repository_test.go @@ -396,8 +396,8 @@ var _ = Describe("PackageRepository", func() { BeforeEach(func() { Expect(k8s.Patch(ctx, k8sClient, cfPackage, func() { meta.SetStatusCondition(&cfPackage.Status.Conditions, metav1.Condition{ - Type: "Ready", - Status: "True", + Type: korifiv1alpha1.StatusConditionReady, + Status: metav1.ConditionTrue, Reason: "Ready", ObservedGeneration: cfPackage.Generation, }) @@ -507,8 +507,8 @@ var _ = Describe("PackageRepository", func() { Expect(k8sClient.Create(ctx, cfPackage)).To(Succeed()) Expect(k8s.Patch(ctx, k8sClient, cfPackage, func() { meta.SetStatusCondition(&cfPackage.Status.Conditions, metav1.Condition{ - Type: "Ready", - Status: "True", + Type: korifiv1alpha1.StatusConditionReady, + Status: metav1.ConditionTrue, Reason: "Ready", ObservedGeneration: cfPackage.Generation, }) diff --git a/api/repositories/repositories_suite_test.go b/api/repositories/repositories_suite_test.go index 7bc922a3e..3594d7878 100644 --- a/api/repositories/repositories_suite_test.go +++ b/api/repositories/repositories_suite_test.go @@ -151,7 +151,7 @@ func createOrgWithCleanup(ctx context.Context, displayName string) *korifiv1alph Expect(k8sClient.Create(ctx, cfOrg)).To(Succeed()) meta.SetStatusCondition(&(cfOrg.Status.Conditions), metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionTrue, Reason: "cus", Message: "cus", @@ -191,7 +191,7 @@ func createSpaceWithCleanup(ctx context.Context, orgGUID, name string) *korifiv1 cfSpace.Status.GUID = cfSpace.Name meta.SetStatusCondition(&(cfSpace.Status.Conditions), metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionTrue, Reason: "cus", Message: "cus", diff --git a/api/repositories/space_repository_test.go b/api/repositories/space_repository_test.go index be2e3e971..751e4668e 100644 --- a/api/repositories/space_repository_test.go +++ b/api/repositories/space_repository_test.go @@ -74,7 +74,7 @@ var _ = Describe("SpaceRepository", func() { Expect(k8s.Patch(ctx, k8sClient, cfSpace, func() { cfSpace.Status.GUID = cfSpace.Name meta.SetStatusCondition(&cfSpace.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: conditionStatus, Reason: "blah", Message: conditionMessage, @@ -227,7 +227,7 @@ var _ = Describe("SpaceRepository", func() { When("the space anchor is not ready", func() { BeforeEach(func() { meta.SetStatusCondition(&(space11.Status.Conditions), metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionFalse, Reason: "cus", Message: "cus", diff --git a/controllers/controllers/workloads/cf_buildpack_build_controller_test.go b/controllers/controllers/workloads/cf_buildpack_build_controller_test.go index fee894712..bdbecb71c 100644 --- a/controllers/controllers/workloads/cf_buildpack_build_controller_test.go +++ b/controllers/controllers/workloads/cf_buildpack_build_controller_test.go @@ -7,7 +7,9 @@ import ( . "code.cloudfoundry.org/korifi/controllers/controllers/workloads/testutils" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" @@ -23,14 +25,10 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { ) var ( - cfSpace *korifiv1alpha1.CFSpace - cfAppGUID string - cfPackageGUID string - cfBuildGUID string - desiredCFApp *korifiv1alpha1.CFApp - desiredCFPackage *korifiv1alpha1.CFPackage - desiredCFBuild *korifiv1alpha1.CFBuild - desiredBuildpacks []string + cfSpace *korifiv1alpha1.CFSpace + cfApp *korifiv1alpha1.CFApp + cfPackage *korifiv1alpha1.CFPackage + cfBuild *korifiv1alpha1.CFBuild ) eventuallyBuildWorkloadShould := func(assertion func(*korifiv1alpha1.BuildWorkload, Gomega)) { @@ -38,7 +36,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { Eventually(func(g Gomega) { workload := new(korifiv1alpha1.BuildWorkload) - lookupKey := types.NamespacedName{Name: cfBuildGUID, Namespace: cfSpace.Status.GUID} + lookupKey := types.NamespacedName{Name: cfBuild.Name, Namespace: cfSpace.Status.GUID} g.Expect(adminClient.Get(context.Background(), lookupKey, workload)).To(Succeed()) assertion(workload, g) }).Should(Succeed()) @@ -46,64 +44,79 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { BeforeEach(func() { cfSpace = createSpace(testOrg) - cfAppGUID = PrefixedGUID("cf-app") - cfPackageGUID = PrefixedGUID("cf-package") - beforeCtx := context.Background() - - desiredCFApp = BuildCFAppCRObject(cfAppGUID, cfSpace.Status.GUID) - Expect(adminClient.Create(beforeCtx, desiredCFApp)).To(Succeed()) + cfAppGUID := uuid.NewString() + cfApp = &korifiv1alpha1.CFApp{ + ObjectMeta: metav1.ObjectMeta{ + Name: cfAppGUID, + Namespace: cfSpace.Status.GUID, + }, + Spec: korifiv1alpha1.CFAppSpec{ + DisplayName: "test-app-name", + DesiredState: "STOPPED", + Lifecycle: korifiv1alpha1.Lifecycle{ + Type: "buildpack", + }, + EnvSecretName: cfAppGUID + "-env", + }, + } + Expect(adminClient.Create(ctx, cfApp)).To(Succeed()) Eventually(func(g Gomega) { - actualCFApp := &korifiv1alpha1.CFApp{} - g.Expect(adminClient.Get(beforeCtx, types.NamespacedName{Name: cfAppGUID, Namespace: cfSpace.Status.GUID}, actualCFApp)).To(Succeed()) - g.Expect(actualCFApp.Status.VCAPServicesSecretName).NotTo(BeEmpty()) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + g.Expect(cfApp.Status.VCAPServicesSecretName).NotTo(BeEmpty()) }).Should(Succeed()) - envVarSecret := BuildCFAppEnvVarsSecret(desiredCFApp.Name, cfSpace.Status.GUID, map[string]string{ + envVarSecret := BuildCFAppEnvVarsSecret(cfApp.Name, cfSpace.Status.GUID, map[string]string{ "a_key": "a-val", "b_key": "b-val", }) Expect(adminClient.Create(context.Background(), envVarSecret)).To(Succeed()) dockerRegistrySecret := BuildDockerRegistrySecret(wellFormedRegistryCredentialsSecret, cfSpace.Status.GUID) - Expect(adminClient.Create(beforeCtx, dockerRegistrySecret)).To(Succeed()) + Expect(adminClient.Create(ctx, dockerRegistrySecret)).To(Succeed()) registryServiceAccountName := "kpack-service-account" registryServiceAccount := BuildServiceAccount(registryServiceAccountName, cfSpace.Status.GUID, wellFormedRegistryCredentialsSecret) - Expect(adminClient.Create(beforeCtx, registryServiceAccount)).To(Succeed()) - - desiredBuildpacks = []string{"first-buildpack", "second-buildpack"} - - desiredCFPackage = BuildCFPackageCRObject(cfPackageGUID, cfSpace.Status.GUID, cfAppGUID, "ref") - Expect(adminClient.Create(ctx, desiredCFPackage)).To(Succeed()) + Expect(adminClient.Create(ctx, registryServiceAccount)).To(Succeed()) + + cfPackage = &korifiv1alpha1.CFPackage{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: cfSpace.Status.GUID, + }, + Spec: korifiv1alpha1.CFPackageSpec{ + Type: "bits", + AppRef: corev1.LocalObjectReference{ + Name: cfAppGUID, + }, + }, + } + Expect(adminClient.Create(ctx, cfPackage)).To(Succeed()) kpackSecret := BuildDockerRegistrySecret("source-registry-image-pull-secret", cfSpace.Status.GUID) Expect(adminClient.Create(ctx, kpackSecret)).To(Succeed()) - - cfBuildGUID = PrefixedGUID("cf-build") }) JustBeforeEach(func() { - desiredCFBuild = BuildCFBuildObject(cfBuildGUID, cfSpace.Status.GUID, cfPackageGUID, cfAppGUID) - desiredCFBuild.Spec.Lifecycle.Data.Buildpacks = desiredBuildpacks - Expect(adminClient.Create(context.Background(), desiredCFBuild)).To(Succeed()) + cfBuild = BuildCFBuildObject(uuid.NewString(), cfSpace.Status.GUID, cfPackage.Name, cfApp.Name) + cfBuild.Spec.Lifecycle.Data.Buildpacks = []string{"first-buildpack", "second-buildpack"} + Expect(adminClient.Create(context.Background(), cfBuild)).To(Succeed()) }) It("creates a BuildWorkload with the buildRef, source, env, and buildpacks set", func() { - createdCFApp := &korifiv1alpha1.CFApp{} - Expect(adminClient.Get(context.Background(), types.NamespacedName{Name: cfAppGUID, Namespace: cfSpace.Status.GUID}, createdCFApp)).To(Succeed()) + Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) eventuallyBuildWorkloadShould(func(workload *korifiv1alpha1.BuildWorkload, g Gomega) { - g.Expect(workload.Spec.BuildRef.Name).To(Equal(cfBuildGUID)) - g.Expect(workload.Spec.Source).To(Equal(desiredCFPackage.Spec.Source)) + g.Expect(workload.Spec.BuildRef.Name).To(Equal(cfBuild.Name)) + g.Expect(workload.Spec.Source).To(Equal(cfPackage.Spec.Source)) g.Expect(workload.Spec.Env).To(ConsistOf( MatchFields(IgnoreExtras, Fields{ "Name": Equal("a_key"), "ValueFrom": PointTo(MatchFields(IgnoreExtras, Fields{ "SecretKeyRef": PointTo(MatchFields(IgnoreExtras, Fields{ "LocalObjectReference": MatchFields(IgnoreExtras, Fields{ - "Name": Equal(createdCFApp.Spec.EnvSecretName), + "Name": Equal(cfApp.Spec.EnvSecretName), }), "Key": Equal("a_key"), })), @@ -114,7 +127,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { "ValueFrom": PointTo(MatchFields(IgnoreExtras, Fields{ "SecretKeyRef": PointTo(MatchFields(IgnoreExtras, Fields{ "LocalObjectReference": MatchFields(IgnoreExtras, Fields{ - "Name": Equal(createdCFApp.Spec.EnvSecretName), + "Name": Equal(cfApp.Spec.EnvSecretName), }), "Key": Equal("b_key"), })), @@ -125,7 +138,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { "ValueFrom": PointTo(MatchFields(IgnoreExtras, Fields{ "SecretKeyRef": PointTo(MatchFields(IgnoreExtras, Fields{ "LocalObjectReference": MatchFields(IgnoreExtras, Fields{ - "Name": Equal(createdCFApp.Status.VCAPServicesSecretName), + "Name": Equal(cfApp.Status.VCAPServicesSecretName), }), "Key": Equal("VCAP_SERVICES"), })), @@ -136,19 +149,19 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { "ValueFrom": PointTo(MatchFields(IgnoreExtras, Fields{ "SecretKeyRef": PointTo(MatchFields(IgnoreExtras, Fields{ "LocalObjectReference": MatchFields(IgnoreExtras, Fields{ - "Name": Equal(createdCFApp.Status.VCAPApplicationSecretName), + "Name": Equal(cfApp.Status.VCAPApplicationSecretName), }), "Key": Equal("VCAP_APPLICATION"), })), })), }), )) - g.Expect(workload.Spec.Buildpacks).To(Equal(desiredBuildpacks)) + g.Expect(workload.Spec.Buildpacks).To(ConsistOf("first-buildpack", "second-buildpack")) g.Expect(workload.GetOwnerReferences()).To(ConsistOf(metav1.OwnerReference{ - UID: desiredCFBuild.UID, + UID: cfBuild.UID, Kind: "CFBuild", APIVersion: "korifi.cloudfoundry.org/v1alpha1", - Name: desiredCFBuild.Name, + Name: cfBuild.Name, Controller: tools.PtrTo(true), BlockOwnerDeletion: tools.PtrTo(true), })) @@ -156,7 +169,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { }) It("sets the 'build-running' status conditions on CFBuild", func() { - lookupKey := types.NamespacedName{Name: cfBuildGUID, Namespace: cfSpace.Status.GUID} + lookupKey := types.NamespacedName{Name: cfBuild.Name, Namespace: cfSpace.Status.GUID} Eventually(func(g Gomega) { createdCFBuild := new(korifiv1alpha1.CFBuild) g.Expect(adminClient.Get(context.Background(), lookupKey, createdCFBuild)).To(Succeed()) @@ -199,7 +212,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { Name: "service-binding-guid", Namespace: cfSpace.Status.GUID, Labels: map[string]string{ - korifiv1alpha1.CFAppGUIDLabelKey: desiredCFApp.Name, + korifiv1alpha1.CFAppGUIDLabelKey: cfApp.Name, }, }, Spec: korifiv1alpha1.CFServiceBindingSpec{ @@ -208,7 +221,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { Name: "service-instance-guid", }, AppRef: corev1.LocalObjectReference{ - Name: desiredCFApp.Name, + Name: cfApp.Name, }, }, } @@ -239,7 +252,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { It("sets the VCAP_SERVICES env var in the image", func() { createdCFApp := &korifiv1alpha1.CFApp{} - Expect(adminClient.Get(context.Background(), types.NamespacedName{Name: cfAppGUID, Namespace: cfSpace.Status.GUID}, createdCFApp)).To(Succeed()) + Expect(adminClient.Get(context.Background(), types.NamespacedName{Name: cfApp.Name, Namespace: cfSpace.Status.GUID}, createdCFApp)).To(Succeed()) eventuallyBuildWorkloadShould(func(workload *korifiv1alpha1.BuildWorkload, g Gomega) { g.Expect(workload.Spec.Env).To(ContainElements( @@ -282,7 +295,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { }, }, } - newCFBuild = BuildCFBuildObject(newCFBuildGUID, cfSpace.Status.GUID, cfPackageGUID, cfAppGUID) + newCFBuild = BuildCFBuildObject(newCFBuildGUID, cfSpace.Status.GUID, cfPackage.Name, cfApp.Name) Expect(adminClient.Create(ctx, existingBuildWorkload)).To(Succeed()) Expect(adminClient.Create(ctx, newCFBuild)).To(Succeed()) @@ -311,7 +324,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { When("the BuildWorkload failed", func() { JustBeforeEach(func() { testCtx := context.Background() - lookupKey := types.NamespacedName{Name: cfBuildGUID, Namespace: cfSpace.Status.GUID} + lookupKey := types.NamespacedName{Name: cfBuild.Name, Namespace: cfSpace.Status.GUID} Eventually(func(g Gomega) { workload := new(korifiv1alpha1.BuildWorkload) g.Expect(adminClient.Get(testCtx, lookupKey, workload)).To(Succeed()) @@ -326,7 +339,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { }) It("sets the CFBuild status condition Succeeded = False", func() { - lookupKey := types.NamespacedName{Name: cfBuildGUID, Namespace: cfSpace.Status.GUID} + lookupKey := types.NamespacedName{Name: cfBuild.Name, Namespace: cfSpace.Status.GUID} createdCFBuild := new(korifiv1alpha1.CFBuild) Eventually(func(g Gomega) { g.Expect(adminClient.Get(context.Background(), lookupKey, createdCFBuild)).To(Succeed()) @@ -353,13 +366,9 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { buildStack = "cflinuxfs3" ) - var ( - returnedProcessTypes []korifiv1alpha1.ProcessType - returnedPorts []int32 - ) + var returnedProcessTypes []korifiv1alpha1.ProcessType JustBeforeEach(func() { - returnedPorts = []int32{42} returnedProcessTypes = []korifiv1alpha1.ProcessType{ { Type: "web", @@ -367,7 +376,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { }, } - lookupKey := types.NamespacedName{Name: cfBuildGUID, Namespace: cfSpace.Status.GUID} + lookupKey := types.NamespacedName{Name: cfBuild.Name, Namespace: cfSpace.Status.GUID} Eventually(func(g Gomega) { workload := new(korifiv1alpha1.BuildWorkload) g.Expect(adminClient.Get(ctx, lookupKey, workload)).To(Succeed()) @@ -383,7 +392,7 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { ImagePullSecrets: []corev1.LocalObjectReference{{Name: imagePullSecretName}}, }, Stack: buildStack, - Ports: returnedPorts, + Ports: []int32{42}, ProcessTypes: returnedProcessTypes, } })).To(Succeed()) @@ -391,36 +400,31 @@ var _ = Describe("CFBuildpackBuildReconciler Integration Tests", func() { }) It("sets the CFBuild status condition Succeeded = True", func() { - lookupKey := types.NamespacedName{Name: cfBuildGUID, Namespace: cfSpace.Status.GUID} - createdCFBuild := new(korifiv1alpha1.CFBuild) - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(context.Background(), lookupKey, createdCFBuild)).To(Succeed()) - stagingStatusCondition := meta.FindStatusCondition(createdCFBuild.Status.Conditions, korifiv1alpha1.StagingConditionType) + g.Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfBuild), cfBuild)).To(Succeed()) + stagingStatusCondition := meta.FindStatusCondition(cfBuild.Status.Conditions, korifiv1alpha1.StagingConditionType) g.Expect(stagingStatusCondition).NotTo(BeNil()) g.Expect(stagingStatusCondition.Status).To(Equal(metav1.ConditionFalse)) g.Expect(stagingStatusCondition.Reason).To(Equal("BuildNotRunning")) - g.Expect(stagingStatusCondition.ObservedGeneration).To(Equal(createdCFBuild.Generation)) + g.Expect(stagingStatusCondition.ObservedGeneration).To(Equal(cfBuild.Generation)) - succeededStatusCondition := meta.FindStatusCondition(createdCFBuild.Status.Conditions, korifiv1alpha1.SucceededConditionType) + succeededStatusCondition := meta.FindStatusCondition(cfBuild.Status.Conditions, korifiv1alpha1.SucceededConditionType) g.Expect(succeededStatusCondition).NotTo(BeNil()) g.Expect(succeededStatusCondition.Status).To(Equal(metav1.ConditionTrue)) g.Expect(succeededStatusCondition.Reason).To(Equal("BuildSucceeded")) - g.Expect(succeededStatusCondition.ObservedGeneration).To(Equal(createdCFBuild.Generation)) + g.Expect(succeededStatusCondition.ObservedGeneration).To(Equal(cfBuild.Generation)) }).Should(Succeed()) }) It("sets CFBuild.status.droplet", func() { - lookupKey := types.NamespacedName{Name: cfBuildGUID, Namespace: cfSpace.Status.GUID} Eventually(func(g Gomega) { - createdCFBuild := new(korifiv1alpha1.CFBuild) - g.Expect(adminClient.Get(context.Background(), lookupKey, createdCFBuild)).To(Succeed()) - g.Expect(createdCFBuild.Status.Droplet).NotTo(BeNil()) - g.Expect(createdCFBuild.Status.Droplet.Registry.Image).To(Equal(buildImageRef)) - g.Expect(createdCFBuild.Status.Droplet.Registry.ImagePullSecrets).To(ConsistOf(corev1.LocalObjectReference{Name: imagePullSecretName})) - g.Expect(createdCFBuild.Status.Droplet.Stack).To(Equal(buildStack)) - g.Expect(createdCFBuild.Status.Droplet.ProcessTypes).To(Equal(returnedProcessTypes)) - g.Expect(createdCFBuild.Status.Droplet.Ports).To(Equal(returnedPorts)) + g.Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfBuild), cfBuild)).To(Succeed()) + g.Expect(cfBuild.Status.Droplet).NotTo(BeNil()) + g.Expect(cfBuild.Status.Droplet.Registry.Image).To(Equal(buildImageRef)) + g.Expect(cfBuild.Status.Droplet.Registry.ImagePullSecrets).To(ConsistOf(corev1.LocalObjectReference{Name: imagePullSecretName})) + g.Expect(cfBuild.Status.Droplet.Stack).To(Equal(buildStack)) + g.Expect(cfBuild.Status.Droplet.ProcessTypes).To(Equal(returnedProcessTypes)) + g.Expect(cfBuild.Status.Droplet.Ports).To(ConsistOf(BeEquivalentTo(42))) }).Should(Succeed()) }) }) diff --git a/controllers/controllers/workloads/cforg_controller.go b/controllers/controllers/workloads/cforg_controller.go index 5a5c7c8a4..375a01c90 100644 --- a/controllers/controllers/workloads/cforg_controller.go +++ b/controllers/controllers/workloads/cforg_controller.go @@ -28,7 +28,6 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -122,18 +121,18 @@ func (r *CFOrgReconciler) enqueueCFOrgRequests(ctx context.Context, object clien //+kubebuilder:rbac:groups="policy",resources=podsecuritypolicies,verbs=use func (r *CFOrgReconciler) ReconcileResource(ctx context.Context, cfOrg *korifiv1alpha1.CFOrg) (ctrl.Result, error) { + var err error + readyConditionBuilder := k8s.NewReadyConditionBuilder(cfOrg) + defer func() { + meta.SetStatusCondition(&cfOrg.Status.Conditions, readyConditionBuilder.WithError(err).Build()) + }() + nsReconcileResult, err := r.namespaceReconciler.ReconcileResource(ctx, cfOrg) if (nsReconcileResult != ctrl.Result{}) || (err != nil) { return nsReconcileResult, err } - meta.SetStatusCondition(&cfOrg.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.StatusConditionReady, - Status: metav1.ConditionTrue, - Reason: korifiv1alpha1.StatusConditionReady, - ObservedGeneration: cfOrg.Generation, - }) - + readyConditionBuilder.Ready() return ctrl.Result{}, nil } diff --git a/controllers/controllers/workloads/cforg_controller_test.go b/controllers/controllers/workloads/cforg_controller_test.go index 85b706fe0..105759e9b 100644 --- a/controllers/controllers/workloads/cforg_controller_test.go +++ b/controllers/controllers/workloads/cforg_controller_test.go @@ -2,7 +2,6 @@ package workloads_test import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" - . "code.cloudfoundry.org/korifi/controllers/controllers/workloads/testutils" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" @@ -16,16 +15,12 @@ import ( ) var _ = Describe("CFOrgReconciler Integration Tests", func() { - var ( - orgGUID string - cfOrg *korifiv1alpha1.CFOrg - ) + var cfOrg *korifiv1alpha1.CFOrg BeforeEach(func() { - orgGUID = PrefixedGUID("cf-org") cfOrg = &korifiv1alpha1.CFOrg{ ObjectMeta: metav1.ObjectMeta{ - Name: orgGUID, + Name: uuid.NewString(), Namespace: cfRootNamespace, }, Spec: korifiv1alpha1.CFOrgSpec{ @@ -38,11 +33,11 @@ var _ = Describe("CFOrgReconciler Integration Tests", func() { It("creates an org namespace and sets labels", func() { Eventually(func(g Gomega) { var orgNamespace corev1.Namespace - g.Expect(adminClient.Get(ctx, types.NamespacedName{Name: orgGUID}, &orgNamespace)).To(Succeed()) + g.Expect(adminClient.Get(ctx, types.NamespacedName{Name: cfOrg.Name}, &orgNamespace)).To(Succeed()) g.Expect(orgNamespace.Labels).To(SatisfyAll( HaveKeyWithValue(korifiv1alpha1.OrgNameKey, korifiv1alpha1.OrgSpaceDeprecatedName), - HaveKeyWithValue(korifiv1alpha1.OrgGUIDKey, orgGUID), + HaveKeyWithValue(korifiv1alpha1.OrgGUIDKey, cfOrg.Name), HaveKeyWithValue(api.EnforceLevelLabel, string(api.LevelRestricted)), )) g.Expect(orgNamespace.Annotations).To(HaveKeyWithValue(korifiv1alpha1.OrgNameKey, cfOrg.Spec.DisplayName)) @@ -64,19 +59,18 @@ var _ = Describe("CFOrgReconciler Integration Tests", func() { It("sets the ready status on the CFOrg", func() { Eventually(func(g Gomega) { - var createdOrg korifiv1alpha1.CFOrg - g.Expect(adminClient.Get(ctx, types.NamespacedName{Namespace: cfRootNamespace, Name: orgGUID}, &createdOrg)).To(Succeed()) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfOrg), cfOrg)).To(Succeed()) - g.Expect(createdOrg.Status.GUID).To(Equal(orgGUID)) - g.Expect(createdOrg.Status.ObservedGeneration).To(Equal(createdOrg.Generation)) - g.Expect(meta.IsStatusConditionTrue(createdOrg.Status.Conditions, "Ready")).To(BeTrue()) + g.Expect(cfOrg.Status.GUID).To(Equal(cfOrg.Name)) + g.Expect(cfOrg.Status.ObservedGeneration).To(Equal(cfOrg.Generation)) + g.Expect(meta.IsStatusConditionTrue(cfOrg.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) }) It("sets restricted pod security labels on the namespace", func() { Eventually(func(g Gomega) { var ns corev1.Namespace - g.Expect(adminClient.Get(ctx, types.NamespacedName{Name: orgGUID}, &ns)).To(Succeed()) + g.Expect(adminClient.Get(ctx, types.NamespacedName{Name: cfOrg.Name}, &ns)).To(Succeed()) g.Expect(ns.Labels).To(HaveKeyWithValue(api.EnforceLevelLabel, string(api.LevelRestricted))) }).Should(Succeed()) }) diff --git a/controllers/controllers/workloads/cfpackage_controller.go b/controllers/controllers/workloads/cfpackage_controller.go index 079c99e84..94f7b87d5 100644 --- a/controllers/controllers/workloads/cfpackage_controller.go +++ b/controllers/controllers/workloads/cfpackage_controller.go @@ -90,6 +90,12 @@ func (r *CFPackageReconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builde func (r *CFPackageReconciler) ReconcileResource(ctx context.Context, cfPackage *korifiv1alpha1.CFPackage) (ctrl.Result, error) { log := logr.FromContextOrDiscard(ctx) + var err error + readyConditionBuilder := k8s.NewReadyConditionBuilder(cfPackage) + defer func() { + meta.SetStatusCondition(&cfPackage.Status.Conditions, readyConditionBuilder.WithError(err).Build()) + }() + cfPackage.Status.ObservedGeneration = cfPackage.Generation log.V(1).Info("set observed generation", "generation", cfPackage.Status.ObservedGeneration) @@ -98,7 +104,7 @@ func (r *CFPackageReconciler) ReconcileResource(ctx context.Context, cfPackage * } var cfApp korifiv1alpha1.CFApp - err := r.k8sClient.Get(ctx, types.NamespacedName{Name: cfPackage.Spec.AppRef.Name, Namespace: cfPackage.Namespace}, &cfApp) + err = r.k8sClient.Get(ctx, types.NamespacedName{Name: cfPackage.Spec.AppRef.Name, Namespace: cfPackage.Namespace}, &cfApp) if err != nil { log.Info("error when fetching CFApp", "reason", err) return ctrl.Result{}, err @@ -117,26 +123,23 @@ func (r *CFPackageReconciler) ReconcileResource(ctx context.Context, cfPackage * ObservedGeneration: cfPackage.Generation, }) - readyCondition := metav1.ConditionFalse - readyReason := "Initialized" - if cfPackage.Spec.Source.Registry.Image != "" { - readyCondition = metav1.ConditionTrue - readyReason = "SourceImageSet" - } - meta.SetStatusCondition(&cfPackage.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.StatusConditionReady, - Status: readyCondition, - Reason: readyReason, - ObservedGeneration: cfPackage.Generation, - }) + defer func() { + if err = r.packageCleaner.Clean(ctx, types.NamespacedName{ + Namespace: cfPackage.Namespace, + Name: cfPackage.Spec.AppRef.Name, + }); err != nil { + log.Info("failed deleting older packages", "reason", err) + } + }() - if err = r.packageCleaner.Clean(ctx, types.NamespacedName{ - Namespace: cfPackage.Namespace, - Name: cfPackage.Spec.AppRef.Name, - }); err != nil { - log.Info("failed deleting older packages", "reason", err) + if cfPackage.Spec.Source.Registry.Image == "" { + readyConditionBuilder.WithReason("Initialized") + return ctrl.Result{}, nil } + readyConditionBuilder.WithReason("SourceImageSet") + readyConditionBuilder.Ready() + return ctrl.Result{}, nil } diff --git a/controllers/controllers/workloads/cfpackage_controller_test.go b/controllers/controllers/workloads/cfpackage_controller_test.go index 77fe84a96..8ac82c983 100644 --- a/controllers/controllers/workloads/cfpackage_controller_test.go +++ b/controllers/controllers/workloads/cfpackage_controller_test.go @@ -6,35 +6,43 @@ import ( korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/controllers/workloads" - . "code.cloudfoundry.org/korifi/controllers/controllers/workloads/testutils" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) var _ = Describe("CFPackageReconciler Integration Tests", func() { var ( - cfSpace *korifiv1alpha1.CFSpace - cfApp *korifiv1alpha1.CFApp - cfAppGUID string - cfPackage *korifiv1alpha1.CFPackage - cfPackageGUID string + cfSpace *korifiv1alpha1.CFSpace + cfApp *korifiv1alpha1.CFApp + cfPackage *korifiv1alpha1.CFPackage ) BeforeEach(func() { cfSpace = createSpace(testOrg) - cfAppGUID = GenerateGUID() - cfPackageGUID = GenerateGUID() - - cfApp = BuildCFAppCRObject(cfAppGUID, cfSpace.Status.GUID) + cfApp = &korifiv1alpha1.CFApp{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: cfSpace.Status.GUID, + }, + Spec: korifiv1alpha1.CFAppSpec{ + DisplayName: "test-app-name", + DesiredState: "STOPPED", + Lifecycle: korifiv1alpha1.Lifecycle{ + Type: "buildpack", + }, + }, + } Expect(adminClient.Create(context.Background(), cfApp)).To(Succeed()) }) @@ -44,8 +52,18 @@ var _ = Describe("CFPackageReconciler Integration Tests", func() { BeforeEach(func() { cleanCallCount = packageCleaner.CleanCallCount() - cfPackage = BuildCFPackageCRObject(cfPackageGUID, cfSpace.Status.GUID, cfAppGUID, "ref") - cfPackage.Spec.Source = korifiv1alpha1.PackageSource{} + cfPackage = &korifiv1alpha1.CFPackage{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: cfSpace.Status.GUID, + }, + Spec: korifiv1alpha1.CFPackageSpec{ + Type: "bits", + AppRef: corev1.LocalObjectReference{ + Name: cfApp.Name, + }, + }, + } Expect(adminClient.Create(context.Background(), cfPackage)).To(Succeed()) }) @@ -77,55 +95,65 @@ var _ = Describe("CFPackageReconciler Integration Tests", func() { var cleanedApps []types.NamespacedName for currCall := cleanCallCount; currCall < packageCleaner.CleanCallCount(); currCall++ { - cleanedApps = append(cleanedApps, types.NamespacedName{Namespace: cfSpace.Status.GUID, Name: cfAppGUID}) + cleanedApps = append(cleanedApps, types.NamespacedName{Namespace: cfSpace.Status.GUID, Name: cfApp.Name}) } g.Expect(cleanedApps).To(ContainElement(types.NamespacedName{Namespace: cfApp.Namespace, Name: cfApp.Name})) }).Should(Succeed()) }) It("sets the ObservedGeneration status field", func() { - var createdCFPackage korifiv1alpha1.CFPackage Eventually(func(g Gomega) { - g.Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfPackage), &createdCFPackage)).To(Succeed()) - - g.Expect(createdCFPackage.Status.ObservedGeneration).To(Equal(createdCFPackage.Generation)) + g.Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfPackage), cfPackage)).To(Succeed()) + g.Expect(cfPackage.Status.ObservedGeneration).To(Equal(cfPackage.Generation)) }).Should(Succeed()) }) When("the package is updated with its source image", func() { - var createdCFPackage korifiv1alpha1.CFPackage - BeforeEach(func() { Eventually(func(g Gomega) { - g.Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfPackage), &createdCFPackage)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(createdCFPackage.Status.Conditions, workloads.InitializedConditionType)).To(BeTrue()) + g.Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfPackage), cfPackage)).To(Succeed()) + g.Expect(meta.IsStatusConditionTrue(cfPackage.Status.Conditions, workloads.InitializedConditionType)).To(BeTrue()) }).Should(Succeed()) }) JustBeforeEach(func() { - Expect(k8s.PatchResource(ctx, adminClient, &createdCFPackage, func() { - createdCFPackage.Spec.Source.Registry.Image = "hello" + Expect(k8s.PatchResource(ctx, adminClient, cfPackage, func() { + cfPackage.Spec.Source.Registry.Image = "hello" })).To(Succeed()) }) 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, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) + g.Expect(adminClient.Get(context.Background(), client.ObjectKeyFromObject(cfPackage), cfPackage)).To(Succeed()) + g.Expect(meta.IsStatusConditionTrue(cfPackage.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) }) }) }) When("a CFPackage is deleted", func() { - var ( - deleteCount int - imageRef string - ) + var deleteCount int BeforeEach(func() { - imageRef = GenerateGUID() - cfPackage = BuildCFPackageCRObject(cfPackageGUID, cfSpace.Status.GUID, cfAppGUID, imageRef) + cfPackage = &korifiv1alpha1.CFPackage{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: cfSpace.Status.GUID, + }, + Spec: korifiv1alpha1.CFPackageSpec{ + Type: "bits", + AppRef: corev1.LocalObjectReference{ + Name: cfApp.Name, + }, + Source: korifiv1alpha1.PackageSource{ + Registry: korifiv1alpha1.Registry{ + Image: uuid.NewString(), + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "source-registry-image-pull-secret"}}, + }, + }, + }, + } + deleteCount = imageDeleter.DeleteCallCount() }) @@ -147,7 +175,7 @@ var _ = Describe("CFPackageReconciler Integration Tests", func() { _, creds, ref, tagsToDelete := imageDeleter.DeleteArgsForCall(deleteCount) g.Expect(creds.Namespace).To(Equal(cfSpace.Status.GUID)) g.Expect(creds.SecretNames).To(ConsistOf("package-repo-secret-name")) - g.Expect(ref).To(Equal(imageRef)) + g.Expect(ref).To(Equal(cfPackage.Spec.Source.Registry.Image)) g.Expect(tagsToDelete).To(ConsistOf(cfPackage.Name)) }).Should(Succeed()) diff --git a/controllers/controllers/workloads/cfprocess_controller.go b/controllers/controllers/workloads/cfprocess_controller.go index 4af75db9b..652a46a38 100644 --- a/controllers/controllers/workloads/cfprocess_controller.go +++ b/controllers/controllers/workloads/cfprocess_controller.go @@ -128,13 +128,19 @@ func (r *CFProcessReconciler) enqueueCFProcessRequestsForRoute(ctx context.Conte func (r *CFProcessReconciler) ReconcileResource(ctx context.Context, cfProcess *korifiv1alpha1.CFProcess) (ctrl.Result, error) { log := logr.FromContextOrDiscard(ctx) + var err error + readyConditionBuilder := k8s.NewReadyConditionBuilder(cfProcess) + defer func() { + meta.SetStatusCondition(&cfProcess.Status.Conditions, readyConditionBuilder.WithError(err).Build()) + }() + cfProcess.Status.ObservedGeneration = cfProcess.Generation log.V(1).Info("set observed generation", "generation", cfProcess.Status.ObservedGeneration) shared.GetConditionOrSetAsUnknown(&cfProcess.Status.Conditions, korifiv1alpha1.StatusConditionReady, cfProcess.Generation) cfApp := new(korifiv1alpha1.CFApp) - err := r.k8sClient.Get(ctx, types.NamespacedName{Name: cfProcess.Spec.AppRef.Name, Namespace: cfProcess.Namespace}, cfApp) + err = r.k8sClient.Get(ctx, types.NamespacedName{Name: cfProcess.Spec.AppRef.Name, Namespace: cfProcess.Namespace}, cfApp) if err != nil { log.Info("error when trying to fetch CFApp", "namespace", cfProcess.Namespace, "name", cfProcess.Spec.AppRef.Name, "reason", err) return ctrl.Result{}, err @@ -167,19 +173,14 @@ func (r *CFProcessReconciler) ReconcileResource(ctx context.Context, cfProcess * return ctrl.Result{}, err } - meta.SetStatusCondition(&cfProcess.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.StatusConditionReady, - Status: metav1.ConditionTrue, - Reason: korifiv1alpha1.StatusConditionReady, - ObservedGeneration: cfProcess.Generation, - }) - appWorkloads, err := r.fetchAppWorkloadsForProcess(ctx, cfProcess) if err != nil { return ctrl.Result{}, err } cfProcess.Status.ActualInstances = getActualInstances(appWorkloads) + + readyConditionBuilder.Ready() return ctrl.Result{}, nil } diff --git a/controllers/controllers/workloads/cfprocess_controller_test.go b/controllers/controllers/workloads/cfprocess_controller_test.go index 9dce49ede..1701975bb 100644 --- a/controllers/controllers/workloads/cfprocess_controller_test.go +++ b/controllers/controllers/workloads/cfprocess_controller_test.go @@ -15,7 +15,6 @@ import ( . "github.com/onsi/gomega/gstruct" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -31,9 +30,9 @@ var _ = Describe("CFProcessReconciler Integration Tests", func() { BeforeEach(func() { cfSpace = createSpace(testOrg) - appGUID := GenerateGUID() - buildGUID := GenerateGUID() - packageGUID := GenerateGUID() + appGUID := uuid.NewString() + buildGUID := uuid.NewString() + packageGUID := uuid.NewString() // Technically the app controller should be creating this process based // on CFApp and CFBuild, but we want to drive testing with a specific @@ -43,12 +42,35 @@ var _ = Describe("CFProcessReconciler Integration Tests", func() { cfProcess = BuildCFProcessCRObject(GenerateGUID(), cfSpace.Status.GUID, appGUID, korifiv1alpha1.ProcessTypeWeb, "process command", "detected-command") Expect(adminClient.Create(ctx, cfProcess)).To(Succeed()) - cfApp = BuildCFAppCRObject(appGUID, cfSpace.Status.GUID) - cfApp.Spec.EnvSecretName = cfApp.Name + "-env" - cfApp.Spec.CurrentDropletRef = corev1.LocalObjectReference{Name: buildGUID} + cfApp = &korifiv1alpha1.CFApp{ + ObjectMeta: metav1.ObjectMeta{ + Name: appGUID, + Namespace: cfSpace.Status.GUID, + }, + Spec: korifiv1alpha1.CFAppSpec{ + DisplayName: "test-app-name", + DesiredState: "STOPPED", + Lifecycle: korifiv1alpha1.Lifecycle{ + Type: "buildpack", + }, + EnvSecretName: appGUID + "-env", + CurrentDropletRef: corev1.LocalObjectReference{Name: buildGUID}, + }, + } Expect(adminClient.Create(ctx, cfApp)).To(Succeed()) - cfPackage := BuildCFPackageCRObject(packageGUID, cfSpace.Status.GUID, cfApp.Name, "ref") + cfPackage := &korifiv1alpha1.CFPackage{ + ObjectMeta: metav1.ObjectMeta{ + Name: packageGUID, + Namespace: cfSpace.Status.GUID, + }, + Spec: korifiv1alpha1.CFPackageSpec{ + Type: "bits", + AppRef: corev1.LocalObjectReference{ + Name: appGUID, + }, + }, + } Expect(adminClient.Create(ctx, cfPackage)).To(Succeed()) cfBuild = BuildCFBuildObject(buildGUID, cfSpace.Status.GUID, packageGUID, cfApp.Name) @@ -121,10 +143,8 @@ var _ = Describe("CFProcessReconciler Integration Tests", func() { It("sets the ObservedGeneration status field", func() { Eventually(func(g Gomega) { - var createdCFProcess korifiv1alpha1.CFProcess - g.Expect(adminClient.Get(ctx, types.NamespacedName{Name: cfProcess.Name, Namespace: cfSpace.Status.GUID}, &createdCFProcess)).To(Succeed()) - - g.Expect(createdCFProcess.Status.ObservedGeneration).To(Equal(createdCFProcess.Generation)) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfProcess), cfProcess)).To(Succeed()) + g.Expect(cfProcess.Status.ObservedGeneration).To(Equal(cfProcess.Generation)) }).Should(Succeed()) }) @@ -341,12 +361,6 @@ var _ = Describe("CFProcessReconciler Integration Tests", func() { }) When("the CFProcess has an http health check", func() { - const ( - healthCheckEndpoint = "/healthy" - healthCheckTimeoutSeconds = 9 - healthCheckInvocationTimeoutSeconds = 3 - ) - BeforeEach(func() { Expect(k8s.PatchResource(ctx, adminClient, cfApp, func() { cfApp.Spec.DesiredState = korifiv1alpha1.StartedState @@ -356,9 +370,9 @@ var _ = Describe("CFProcessReconciler Integration Tests", func() { cfProcess.Spec.HealthCheck = korifiv1alpha1.HealthCheck{ Type: "http", Data: korifiv1alpha1.HealthCheckData{ - HTTPEndpoint: healthCheckEndpoint, - InvocationTimeoutSeconds: healthCheckInvocationTimeoutSeconds, - TimeoutSeconds: healthCheckTimeoutSeconds, + HTTPEndpoint: "/healthy", + InvocationTimeoutSeconds: 3, + TimeoutSeconds: 9, }, } })).To(Succeed()) @@ -385,11 +399,6 @@ var _ = Describe("CFProcessReconciler Integration Tests", func() { }) When("the CFProcess has a port health check", func() { - const ( - healthCheckTimeoutSeconds = 9 - healthCheckInvocationTimeoutSeconds = 3 - ) - BeforeEach(func() { Expect(k8s.PatchResource(ctx, adminClient, cfApp, func() { cfApp.Spec.DesiredState = korifiv1alpha1.StartedState @@ -399,8 +408,8 @@ var _ = Describe("CFProcessReconciler Integration Tests", func() { cfProcess.Spec.HealthCheck = korifiv1alpha1.HealthCheck{ Type: "port", Data: korifiv1alpha1.HealthCheckData{ - InvocationTimeoutSeconds: healthCheckInvocationTimeoutSeconds, - TimeoutSeconds: healthCheckTimeoutSeconds, + InvocationTimeoutSeconds: 3, + TimeoutSeconds: 9, }, } })).To(Succeed()) diff --git a/controllers/controllers/workloads/cfspace_controller.go b/controllers/controllers/workloads/cfspace_controller.go index c9ef79e08..5f7cb3654 100644 --- a/controllers/controllers/workloads/cfspace_controller.go +++ b/controllers/controllers/workloads/cfspace_controller.go @@ -140,6 +140,12 @@ func (r *CFSpaceReconciler) enqueueCFSpaceRequestsForServiceAccount(ctx context. //+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;patch;delete func (r *CFSpaceReconciler) ReconcileResource(ctx context.Context, cfSpace *korifiv1alpha1.CFSpace) (ctrl.Result, error) { + var err error + readyConditionBuilder := k8s.NewReadyConditionBuilder(cfSpace) + defer func() { + meta.SetStatusCondition(&cfSpace.Status.Conditions, readyConditionBuilder.WithError(err).Build()) + }() + nsReconcileResult, err := r.namespaceReconciler.ReconcileResource(ctx, cfSpace) if (nsReconcileResult != ctrl.Result{}) || (err != nil) { return nsReconcileResult, err @@ -151,24 +157,11 @@ func (r *CFSpaceReconciler) ReconcileResource(ctx context.Context, cfSpace *kori if err != nil { log.Info("not ready yet", "reason", "error propagating service accounts", "error", err) - meta.SetStatusCondition(&cfSpace.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.StatusConditionReady, - Status: metav1.ConditionFalse, - Reason: "ServiceAccountPropagation", - Message: err.Error(), - ObservedGeneration: cfSpace.Generation, - }) - + readyConditionBuilder.WithReason("ServiceAccountPropagation") return ctrl.Result{}, err } - meta.SetStatusCondition(&cfSpace.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.StatusConditionReady, - Status: metav1.ConditionTrue, - Reason: korifiv1alpha1.StatusConditionReady, - ObservedGeneration: cfSpace.Generation, - }) - + readyConditionBuilder.Ready() return ctrl.Result{}, nil } diff --git a/controllers/controllers/workloads/cfspace_controller_test.go b/controllers/controllers/workloads/cfspace_controller_test.go index 046acf1cb..33131e1bc 100644 --- a/controllers/controllers/workloads/cfspace_controller_test.go +++ b/controllers/controllers/workloads/cfspace_controller_test.go @@ -22,16 +22,12 @@ import ( ) var _ = Describe("CFSpaceReconciler Integration Tests", func() { - var ( - spaceGUID string - cfSpace *korifiv1alpha1.CFSpace - ) + var cfSpace *korifiv1alpha1.CFSpace BeforeEach(func() { - spaceGUID = PrefixedGUID("cf-space") cfSpace = &korifiv1alpha1.CFSpace{ ObjectMeta: metav1.ObjectMeta{ - Name: spaceGUID, + Name: uuid.NewString(), Namespace: testOrg.Status.GUID, }, Spec: korifiv1alpha1.CFSpaceSpec{ @@ -44,11 +40,11 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { It("creates a namespace and sets labels", func() { Eventually(func(g Gomega) { var ns corev1.Namespace - g.Expect(adminClient.Get(ctx, types.NamespacedName{Name: spaceGUID}, &ns)).To(Succeed()) + g.Expect(adminClient.Get(ctx, types.NamespacedName{Name: cfSpace.Name}, &ns)).To(Succeed()) g.Expect(ns.Labels).To(SatisfyAll( HaveKeyWithValue(korifiv1alpha1.SpaceNameKey, korifiv1alpha1.OrgSpaceDeprecatedName), - HaveKeyWithValue(korifiv1alpha1.SpaceGUIDKey, spaceGUID), + HaveKeyWithValue(korifiv1alpha1.SpaceGUIDKey, cfSpace.Name), HaveKeyWithValue(api.EnforceLevelLabel, string(api.LevelRestricted)), )) g.Expect(ns.Annotations).To(HaveKeyWithValue(korifiv1alpha1.SpaceNameKey, cfSpace.Spec.DisplayName)) @@ -74,7 +70,7 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { g.Expect(cfSpace.Status.GUID).To(Equal(cfSpace.Name)) g.Expect(cfSpace.Status.ObservedGeneration).To(Equal(cfSpace.Generation)) - g.Expect(meta.IsStatusConditionTrue(cfSpace.Status.Conditions, "Ready")).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(cfSpace.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) }) @@ -82,7 +78,7 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { var serviceAccount *corev1.ServiceAccount BeforeEach(func() { - serviceAccountName := PrefixedGUID("service-account") + serviceAccountName := uuid.NewString() serviceAccount = &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: serviceAccountName, @@ -108,7 +104,7 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { Expect(adminClient.Create(ctx, serviceAccount)).To(Succeed()) Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfSpace), cfSpace)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(cfSpace.Status.Conditions, "Ready")).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(cfSpace.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }, 20*time.Second).Should(Succeed()) }) @@ -162,10 +158,10 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { It("removes all secret references other than the package registry secret from the propagated service account", func() { Eventually(func(g Gomega) { - var createdServiceAccounts corev1.ServiceAccountList - g.Expect(adminClient.List(ctx, &createdServiceAccounts, client.InNamespace(cfSpace.Name))).To(Succeed()) + var serviceAccounts corev1.ServiceAccountList + g.Expect(adminClient.List(ctx, &serviceAccounts, client.InNamespace(cfSpace.Name))).To(Succeed()) - g.Expect(createdServiceAccounts.Items).To(ContainElements( + g.Expect(serviceAccounts.Items).To(ContainElements( MatchFields(IgnoreExtras, Fields{ "ObjectMeta": MatchFields(IgnoreExtras, Fields{ "Name": Equal(serviceAccount.Name), @@ -188,9 +184,13 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { }) It("deletes the corresponding service account in CFSpace", func() { - Eventually(func() bool { - return apierrors.IsNotFound(adminClient.Get(ctx, types.NamespacedName{Name: serviceAccount.Name, Namespace: cfSpace.Name}, new(corev1.ServiceAccount))) - }).Should(BeTrue(), "timed out waiting for service account to be deleted") + Eventually(func(g Gomega) { + g.Expect(apierrors.IsNotFound(adminClient.Get( + ctx, + types.NamespacedName{Name: serviceAccount.Name, Namespace: cfSpace.Name}, + new(corev1.ServiceAccount), + ))).To(BeTrue()) + }).Should(Succeed()) }) When("the service account is annotated not to propagate deletions", func() { @@ -199,9 +199,13 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { }) It("doesn't delete the corresponding service account in the CFSpace", func() { - Consistently(func() bool { - return apierrors.IsNotFound(adminClient.Get(ctx, types.NamespacedName{Name: serviceAccount.Name, Namespace: cfSpace.Name}, new(corev1.ServiceAccount))) - }).Should(BeFalse(), "space's copy of service account was deleted and shouldn't have been") + Consistently(func(g Gomega) { + g.Expect(apierrors.IsNotFound(adminClient.Get( + ctx, + types.NamespacedName{Name: serviceAccount.Name, Namespace: cfSpace.Name}, + new(corev1.ServiceAccount), + ))).To(BeFalse()) + }).Should(Succeed()) }) }) }) @@ -218,15 +222,14 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { BeforeEach(func() { Eventually(func(g Gomega) { - var createdSpace korifiv1alpha1.CFSpace - g.Expect(adminClient.Get(ctx, types.NamespacedName{Namespace: testOrg.Status.GUID, Name: spaceGUID}, &createdSpace)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(createdSpace.Status.Conditions, "Ready")).To(BeTrue()) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfSpace), cfSpace)).To(Succeed()) + g.Expect(meta.IsStatusConditionTrue(cfSpace.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }, 20*time.Second).Should(Succeed()) rootServiceAccount = createServiceAccount(ctx, PrefixedGUID("existing-service-account"), cfRootNamespace, map[string]string{"cloudfoundry.org/propagate-service-account": "true"}) // Ensure that the service account is propagated into the CFSpace namespace - Eventually(func() error { - return adminClient.Get(ctx, types.NamespacedName{Name: rootServiceAccount.Name, Namespace: cfSpace.Name}, &propagatedServiceAccount) + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, types.NamespacedName{Name: rootServiceAccount.Name, Namespace: cfSpace.Name}, &propagatedServiceAccount)).To(Succeed()) }).Should(Succeed()) // Simulate k8s adding a token secret to the propagated service account AND the propagated service account having a stale image registry credential secret @@ -245,17 +248,16 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { It("updates the secrets on the propagated service account", func() { Eventually(func(g Gomega) { - var updatedPropagatedServiceAccount corev1.ServiceAccount g.Expect( - adminClient.Get(ctx, client.ObjectKeyFromObject(&propagatedServiceAccount), &updatedPropagatedServiceAccount), + adminClient.Get(ctx, client.ObjectKeyFromObject(&propagatedServiceAccount), &propagatedServiceAccount), ).To(Succeed()) - g.Expect(updatedPropagatedServiceAccount.Secrets).To(ConsistOf( + g.Expect(propagatedServiceAccount.Secrets).To(ConsistOf( corev1.ObjectReference{Name: tokenSecretName}, corev1.ObjectReference{Name: dockercfgSecretName}, corev1.ObjectReference{Name: packageRegistrySecretName}, )) - g.Expect(updatedPropagatedServiceAccount.ImagePullSecrets).To(ConsistOf( + g.Expect(propagatedServiceAccount.ImagePullSecrets).To(ConsistOf( corev1.LocalObjectReference{Name: dockercfgSecretName}, corev1.LocalObjectReference{Name: packageRegistrySecretName}, )) @@ -273,9 +275,8 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { BeforeEach(func() { Eventually(func(g Gomega) { - var createdSpace korifiv1alpha1.CFSpace - g.Expect(adminClient.Get(ctx, types.NamespacedName{Namespace: testOrg.Status.GUID, Name: spaceGUID}, &createdSpace)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(createdSpace.Status.Conditions, "Ready")).To(BeTrue()) + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfSpace), cfSpace)).To(Succeed()) + g.Expect(meta.IsStatusConditionTrue(cfSpace.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }, 20*time.Second).Should(Succeed()) rootServiceAccount = &corev1.ServiceAccount{ @@ -310,16 +311,15 @@ var _ = Describe("CFSpaceReconciler Integration Tests", func() { It("is also added to the space's copy of the service account", func() { Eventually(func(g Gomega) { - var updatedPropagatedServiceAccount corev1.ServiceAccount g.Expect( - adminClient.Get(ctx, client.ObjectKeyFromObject(&propagatedServiceAccount), &updatedPropagatedServiceAccount), + adminClient.Get(ctx, client.ObjectKeyFromObject(&propagatedServiceAccount), &propagatedServiceAccount), ).To(Succeed()) - g.Expect(updatedPropagatedServiceAccount.Secrets).To(ConsistOf( + g.Expect(propagatedServiceAccount.Secrets).To(ConsistOf( corev1.ObjectReference{Name: tokenSecretName}, corev1.ObjectReference{Name: dockercfgSecretName}, corev1.ObjectReference{Name: packageRegistrySecretName}, )) - g.Expect(updatedPropagatedServiceAccount.ImagePullSecrets).To(ConsistOf( + g.Expect(propagatedServiceAccount.ImagePullSecrets).To(ConsistOf( corev1.LocalObjectReference{Name: dockercfgSecretName}, corev1.LocalObjectReference{Name: packageRegistrySecretName}, )) diff --git a/controllers/controllers/workloads/cftask_controller_test.go b/controllers/controllers/workloads/cftask_controller_test.go index d2d2dee6c..f7623b903 100644 --- a/controllers/controllers/workloads/cftask_controller_test.go +++ b/controllers/controllers/workloads/cftask_controller_test.go @@ -158,12 +158,8 @@ var _ = Describe("CFTaskReconciler Integration Tests", func() { BeforeEach(func() { task = &korifiv1alpha1.CFTask{} Eventually(func(g Gomega) { - app := new(korifiv1alpha1.CFApp) - g.Expect(adminClient.Get(ctx, types.NamespacedName{Namespace: cfSpace.Status.GUID, Name: cfApp.Name}, app)).To(Succeed()) - - readyCondition := meta.FindStatusCondition(app.Status.Conditions, "Ready") - g.Expect(readyCondition).NotTo(BeNil()) - g.Expect(readyCondition.Status).To(Equal(metav1.ConditionTrue), "App is not staged") + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)).To(Succeed()) + g.Expect(meta.IsStatusConditionTrue(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) }).Should(Succeed()) eventCallCount = eventRecorder.EventfCallCount() diff --git a/controllers/controllers/workloads/env/env_suite_test.go b/controllers/controllers/workloads/env/env_suite_test.go index 39eadc37f..9b48f0340 100644 --- a/controllers/controllers/workloads/env/env_suite_test.go +++ b/controllers/controllers/workloads/env/env_suite_test.go @@ -140,7 +140,7 @@ var _ = BeforeEach(func() { VCAPApplicationSecretName: "app-guid-vcap-application", } meta.SetStatusCondition(&app.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: korifiv1alpha1.StatusConditionReady, Status: metav1.ConditionTrue, Reason: "testing", LastTransitionTime: metav1.Date(2023, 2, 15, 12, 0, 0, 0, time.FixedZone("", 0)), diff --git a/controllers/controllers/workloads/k8sns/reconciler_test.go b/controllers/controllers/workloads/k8sns/reconciler_test.go index 9d433698a..02bc2a18b 100644 --- a/controllers/controllers/workloads/k8sns/reconciler_test.go +++ b/controllers/controllers/workloads/k8sns/reconciler_test.go @@ -178,10 +178,9 @@ var _ = Describe("K8S NS Reconciler Integration Tests", func() { It("sets the NSObj's Ready condition to 'False'", func() { Expect(reconcileErr).To(MatchError(ContainSubstring("error fetching secret"))) - Expect(meta.IsStatusConditionTrue(nsObj.Status.Conditions, "Ready")).To(BeFalse()) - - readyCondition := meta.FindStatusCondition(nsObj.Status.Conditions, "Ready") + readyCondition := meta.FindStatusCondition(nsObj.Status.Conditions, korifiv1alpha1.StatusConditionReady) Expect(readyCondition).NotTo(BeNil()) + Expect(readyCondition.Status).To(Equal(metav1.ConditionFalse)) Expect(readyCondition.Message).To(ContainSubstring(fmt.Sprintf( "error fetching secret %q from namespace %q", "i-do-not-exist", diff --git a/controllers/controllers/workloads/testutils/shared_test_utils.go b/controllers/controllers/workloads/testutils/shared_test_utils.go index f42a5e45d..cd1878b6f 100644 --- a/controllers/controllers/workloads/testutils/shared_test_utils.go +++ b/controllers/controllers/workloads/testutils/shared_test_utils.go @@ -3,8 +3,6 @@ package testutils import ( "encoding/base64" - "k8s.io/apimachinery/pkg/api/meta" - korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/tools" @@ -13,15 +11,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -const ( - CFAppLabelKey = "korifi.cloudfoundry.org/app-guid" - cfAppRevisionKey = "korifi.cloudfoundry.org/app-rev" - cfAppLastStopRevisionKey = "korifi.cloudfoundry.org/last-stop-app-rev" - CFProcessGUIDLabelKey = "korifi.cloudfoundry.org/process-guid" - CFProcessTypeLabelKey = "korifi.cloudfoundry.org/process-type" - appFinalizerName = "cfApp.korifi.cloudfoundry.org" -) - func GenerateGUID() string { return uuid.NewString() } @@ -30,58 +19,6 @@ func PrefixedGUID(prefix string) string { return prefix + "-" + uuid.NewString()[:8] } -func BuildCFAppCRObject(appGUID string, spaceGUID string) *korifiv1alpha1.CFApp { - return &korifiv1alpha1.CFApp{ - ObjectMeta: metav1.ObjectMeta{ - Name: appGUID, - Namespace: spaceGUID, - Annotations: map[string]string{ - cfAppRevisionKey: "5", - cfAppLastStopRevisionKey: "2", - }, - Finalizers: []string{ - appFinalizerName, - }, - }, - Spec: korifiv1alpha1.CFAppSpec{ - DisplayName: "test-app-name", - DesiredState: "STOPPED", - Lifecycle: korifiv1alpha1.Lifecycle{ - Type: "buildpack", - Data: korifiv1alpha1.LifecycleData{ - Buildpacks: []string{}, - Stack: "", - }, - }, - EnvSecretName: appGUID + "-env", - }, - } -} - -func BuildCFOrgObject(orgGUID string, spaceGUID string) *korifiv1alpha1.CFOrg { - return &korifiv1alpha1.CFOrg{ - ObjectMeta: metav1.ObjectMeta{ - Name: orgGUID, - Namespace: spaceGUID, - }, - Spec: korifiv1alpha1.CFOrgSpec{ - DisplayName: "test-org-name", - }, - } -} - -func BuildCFSpaceObject(spaceGUID string, orgGUID string) *korifiv1alpha1.CFSpace { - return &korifiv1alpha1.CFSpace{ - ObjectMeta: metav1.ObjectMeta{ - Name: spaceGUID, - Namespace: orgGUID, - }, - Spec: korifiv1alpha1.CFSpaceSpec{ - DisplayName: "test-space-name", - }, - } -} - func BuildCFAppEnvVarsSecret(appGUID, spaceGUID string, envVars map[string]string) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -92,27 +29,6 @@ func BuildCFAppEnvVarsSecret(appGUID, spaceGUID string, envVars map[string]strin } } -func BuildCFPackageCRObject(packageGUID, namespaceGUID, appGUID, imageRef string) *korifiv1alpha1.CFPackage { - return &korifiv1alpha1.CFPackage{ - ObjectMeta: metav1.ObjectMeta{ - Name: packageGUID, - Namespace: namespaceGUID, - }, - Spec: korifiv1alpha1.CFPackageSpec{ - Type: "bits", - AppRef: corev1.LocalObjectReference{ - Name: appGUID, - }, - Source: korifiv1alpha1.PackageSource{ - Registry: korifiv1alpha1.Registry{ - Image: imageRef, - ImagePullSecrets: []corev1.LocalObjectReference{{Name: "source-registry-image-pull-secret"}}, - }, - }, - }, - } -} - func BuildCFBuildObject(cfBuildGUID string, namespace string, cfPackageGUID string, cfAppGUID string) *korifiv1alpha1.CFBuild { return &korifiv1alpha1.CFBuild{ ObjectMeta: metav1.ObjectMeta{ @@ -197,9 +113,9 @@ func BuildCFProcessCRObject(cfProcessGUID, namespace, cfAppGUID, processType, pr Name: cfProcessGUID, Namespace: namespace, Labels: map[string]string{ - CFAppLabelKey: cfAppGUID, - CFProcessGUIDLabelKey: cfProcessGUID, - CFProcessTypeLabelKey: processType, + korifiv1alpha1.CFAppGUIDLabelKey: cfAppGUID, + korifiv1alpha1.CFProcessGUIDLabelKey: cfProcessGUID, + korifiv1alpha1.CFProcessTypeLabelKey: processType, }, }, Spec: korifiv1alpha1.CFProcessSpec{ @@ -220,29 +136,3 @@ func BuildCFProcessCRObject(cfProcessGUID, namespace, cfAppGUID, processType, pr }, } } - -func SetStatusCondition(conditions *[]metav1.Condition, conditionType string, status metav1.ConditionStatus) { - meta.SetStatusCondition(conditions, metav1.Condition{ - Type: conditionType, - Status: status, - Reason: "reasons", - Message: "", - }) -} - -func UpdateCFBuildWithDropletStatus(cfbuild *korifiv1alpha1.CFBuild) { - cfbuild.Status.Droplet = &korifiv1alpha1.BuildDropletStatus{ - Registry: korifiv1alpha1.Registry{ - Image: "my-image", - ImagePullSecrets: []corev1.LocalObjectReference{{Name: "some-image-pull-secret"}}, - }, - Stack: "cflinuxfs3", - ProcessTypes: []korifiv1alpha1.ProcessType{ - { - Type: "web", - Command: "web-command", - }, - }, - Ports: []int32{8080}, - } -} diff --git a/statefulset-runner/controllers/appworkload_controller.go b/statefulset-runner/controllers/appworkload_controller.go index 63e4a3339..758f6ff96 100644 --- a/statefulset-runner/controllers/appworkload_controller.go +++ b/statefulset-runner/controllers/appworkload_controller.go @@ -25,7 +25,6 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -152,9 +151,11 @@ func filterAppWorkloads(object client.Object) bool { func (r *AppWorkloadReconciler) ReconcileResource(ctx context.Context, appWorkload *korifiv1alpha1.AppWorkload) (ctrl.Result, error) { log := logr.FromContextOrDiscard(ctx) - if appWorkload.Spec.RunnerName != AppWorkloadReconcilerName { - return ctrl.Result{}, nil - } + var err error + readyConditionBuilder := k8s.NewReadyConditionBuilder(appWorkload) + defer func() { + meta.SetStatusCondition(&appWorkload.Status.Conditions, readyConditionBuilder.WithError(err).Build()) + }() appWorkload.Status.ObservedGeneration = appWorkload.Generation log.V(1).Info("set observed generation", "generation", appWorkload.Status.ObservedGeneration) @@ -168,17 +169,17 @@ func (r *AppWorkloadReconciler) ReconcileResource(ctx context.Context, appWorklo return ctrl.Result{}, err } - orig := &appsv1.StatefulSet{ + createdStSet := &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ Name: statefulSet.Name, Namespace: statefulSet.Namespace, }, } - _, err = controllerutil.CreateOrPatch(ctx, r.k8sClient, orig, func() error { - orig.Labels = statefulSet.Labels - orig.Annotations = statefulSet.Annotations - orig.OwnerReferences = statefulSet.OwnerReferences - orig.Spec = statefulSet.Spec + _, err = controllerutil.CreateOrPatch(ctx, r.k8sClient, createdStSet, func() error { + createdStSet.Labels = statefulSet.Labels + createdStSet.Annotations = statefulSet.Annotations + createdStSet.OwnerReferences = statefulSet.OwnerReferences + createdStSet.Spec = statefulSet.Spec return nil }) @@ -187,30 +188,14 @@ func (r *AppWorkloadReconciler) ReconcileResource(ctx context.Context, appWorklo return ctrl.Result{}, err } - updatedStatefulSet := &appsv1.StatefulSet{} - err = r.k8sClient.Get(ctx, client.ObjectKeyFromObject(statefulSet), updatedStatefulSet) - if err != nil { - if apierrors.IsNotFound(err) { - return ctrl.Result{}, nil - } else { - log.Info("error when fetching StatefulSet", "StatefulSet.Name", statefulSet.Name, "StatefulSet.Namespace", statefulSet.Namespace, "reason", err) - return ctrl.Result{}, err - } - } - - err = r.pdb.Update(ctx, updatedStatefulSet) + err = r.pdb.Update(ctx, createdStSet) if err != nil { log.Info("error when creating or patching pod disruption budget", "reason", err) return ctrl.Result{}, err } - appWorkload.Status.ActualInstances = updatedStatefulSet.Status.Replicas - meta.SetStatusCondition(&appWorkload.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.StatusConditionReady, - Status: metav1.ConditionTrue, - Reason: korifiv1alpha1.StatusConditionReady, - ObservedGeneration: appWorkload.Generation, - }) + appWorkload.Status.ActualInstances = createdStSet.Status.Replicas + readyConditionBuilder.Ready() return ctrl.Result{}, nil } diff --git a/statefulset-runner/controllers/appworkload_controller_test.go b/statefulset-runner/controllers/appworkload_controller_test.go index 69e557e38..04db68ecc 100644 --- a/statefulset-runner/controllers/appworkload_controller_test.go +++ b/statefulset-runner/controllers/appworkload_controller_test.go @@ -15,9 +15,11 @@ import ( v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -44,8 +46,64 @@ var _ = Describe("AppWorkload Reconcile", func() { ) BeforeEach(func() { - Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) - appWorkload = createAppWorkload("some-namespace", "guid_1234") + appWorkload = &korifiv1alpha1.AppWorkload{ + ObjectMeta: metav1.ObjectMeta{ + Name: "guid_1234", + Namespace: "some-namespace", + Generation: 1, + Annotations: map[string]string{ + korifiv1alpha1.CFAppLastStopRevisionKey: "lastStopAppRev", + }, + }, + Spec: korifiv1alpha1.AppWorkloadSpec{ + AppGUID: "premium_app_guid_1234", + GUID: "guid_1234", + Version: "version_1234", + Image: "gcr.io/foo/bar", + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "some-secret-name"}}, + Command: []string{ + "/bin/sh", + "-c", + "while true; do echo hello; sleep 10;done", + }, + ProcessType: "worker", + Env: []corev1.EnvVar{}, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(8080)}, + }, + }, + FailureThreshold: 30, + PeriodSeconds: 2, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(8080)}, + }, + }, + PeriodSeconds: 30, + FailureThreshold: 1, + }, + Ports: []int32{8888, 9999}, + Instances: 1, + RunnerName: "statefulset-runner", + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceEphemeralStorage: resource.MustParse("2048Mi"), + corev1.ResourceMemory: resource.MustParse("1024Mi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("5m"), + corev1.ResourceMemory: resource.MustParse("1024Mi"), + }, + }, + }, + } + statefulSet = &v1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ Name: "converted-stset", @@ -155,16 +213,6 @@ var _ = Describe("AppWorkload Reconcile", func() { Expect(reconcileErr).To(MatchError("big sad")) }) }) - - When("reconciler name on the AppWorkload is not statefulset-runner", func() { - BeforeEach(func() { - appWorkload.Spec.RunnerName = "MyCustomReconciler" - }) - - It("does not create/patch statefulset", func() { - Expect(fakeClient.CreateCallCount()).To(Equal(0), "Client.Create call count mismatch") - }) - }) }) When("the appworkload is being deleted", func() { diff --git a/statefulset-runner/controllers/appworkload_to_stset_test.go b/statefulset-runner/controllers/appworkload_to_stset_test.go index 952c4923a..27d3d8060 100644 --- a/statefulset-runner/controllers/appworkload_to_stset_test.go +++ b/statefulset-runner/controllers/appworkload_to_stset_test.go @@ -13,6 +13,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" ) @@ -26,7 +27,64 @@ var _ = Describe("AppWorkload to StatefulSet Converter", func() { BeforeEach(func() { Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) - appWorkload = createAppWorkload("some-namespace", "guid_1234") + appWorkload = &korifiv1alpha1.AppWorkload{ + ObjectMeta: metav1.ObjectMeta{ + Name: "guid_1234", + Namespace: "some-namespace", + Generation: 1, + Annotations: map[string]string{ + korifiv1alpha1.CFAppLastStopRevisionKey: "lastStopAppRev", + }, + }, + Spec: korifiv1alpha1.AppWorkloadSpec{ + AppGUID: "premium_app_guid_1234", + GUID: "guid_1234", + Version: "version_1234", + Image: "gcr.io/foo/bar", + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "some-secret-name"}}, + Command: []string{ + "/bin/sh", + "-c", + "while true; do echo hello; sleep 10;done", + }, + ProcessType: "worker", + Env: []corev1.EnvVar{}, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(8080)}, + }, + }, + FailureThreshold: 30, + PeriodSeconds: 2, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(8080)}, + }, + }, + PeriodSeconds: 30, + FailureThreshold: 1, + }, + Ports: []int32{8888, 9999}, + Instances: 1, + RunnerName: "statefulset-runner", + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceEphemeralStorage: resource.MustParse("2048Mi"), + corev1.ResourceMemory: resource.MustParse("1024Mi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("5m"), + corev1.ResourceMemory: resource.MustParse("1024Mi"), + }, + }, + }, + } + statefulsetRunnerTemporarySetPodSeccompProfile = false }) diff --git a/statefulset-runner/controllers/integration/appworkload_controller_test.go b/statefulset-runner/controllers/integration/appworkload_controller_test.go index 9883c5446..c67b9fdc1 100644 --- a/statefulset-runner/controllers/integration/appworkload_controller_test.go +++ b/statefulset-runner/controllers/integration/appworkload_controller_test.go @@ -13,6 +13,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -69,10 +70,9 @@ var _ = Describe("AppWorkloadsController", func() { getStatefulsetForAppWorkload := func(g Gomega) appsv1.StatefulSet { stsetList := appsv1.StatefulSetList{} g.Eventually(func(g Gomega) { - err := k8sClient.List(context.Background(), &stsetList, client.MatchingLabels{ + g.Expect(k8sClient.List(ctx, &stsetList, client.MatchingLabels{ controllers.LabelGUID: appWorkload.Spec.GUID, - }) - g.Expect(err).NotTo(HaveOccurred()) + })).To(Succeed()) g.Expect(stsetList.Items).To(HaveLen(1)) }).Should(Succeed()) @@ -124,6 +124,25 @@ var _ = Describe("AppWorkloadsController", func() { }).Should(Succeed()) }) }) + + When("the appworkload runner name is not 'statefulset-runner'", func() { + BeforeEach(func() { + appWorkload.Spec.RunnerName = "another-runner" + }) + + It("does not reconcile it", func() { + Consistently(func(g Gomega) { + stsetList := appsv1.StatefulSetList{} + g.Expect(k8sClient.List(ctx, &stsetList, client.MatchingLabels{ + controllers.LabelGUID: appWorkload.Spec.GUID, + })).To(Succeed()) + g.Expect(stsetList.Items).To(BeEmpty()) + + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(appWorkload), appWorkload)).To(Succeed()) + g.Expect(meta.FindStatusCondition(appWorkload.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeNil()) + }).Should(Succeed()) + }) + }) }) When("AppWorkload update", func() { @@ -132,7 +151,7 @@ var _ = Describe("AppWorkloadsController", func() { }) JustBeforeEach(func() { - Expect(k8s.Patch(context.Background(), k8sClient, appWorkload, func() { + Expect(k8s.Patch(ctx, k8sClient, appWorkload, func() { appWorkload.Spec.Instances = 2 appWorkload.Spec.Resources.Requests[corev1.ResourceCPU] = resource.MustParse("1024m") appWorkload.Spec.Resources.Requests[corev1.ResourceMemory] = resource.MustParse("10Mi") diff --git a/statefulset-runner/controllers/pdb.go b/statefulset-runner/controllers/pdb.go index dac5de4d9..f197734fa 100644 --- a/statefulset-runner/controllers/pdb.go +++ b/statefulset-runner/controllers/pdb.go @@ -4,8 +4,6 @@ import ( "context" "fmt" - k8serrors "k8s.io/apimachinery/pkg/api/errors" - appsv1 "k8s.io/api/apps/v1" policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -42,26 +40,22 @@ func (c *PDBUpdater) createPDB(ctx context.Context, statefulSet *appsv1.Stateful ObjectMeta: metav1.ObjectMeta{ Name: statefulSet.Name, Namespace: statefulSet.Namespace, - Labels: map[string]string{ - LabelGUID: statefulSet.Labels[LabelGUID], - LabelVersion: statefulSet.Labels[LabelVersion], - }, - }, - Spec: policyv1.PodDisruptionBudgetSpec{ - MinAvailable: &minAvailable, - Selector: statefulSet.Spec.Selector, }, } - if err := controllerutil.SetControllerReference(statefulSet, pdb, scheme.Scheme); err != nil { - return fmt.Errorf("pdb updater failed to set owner ref: %w", err) - } + _, err := controllerutil.CreateOrPatch(ctx, c.client, pdb, func() error { + pdb.Labels = map[string]string{ + LabelGUID: statefulSet.Labels[LabelGUID], + LabelVersion: statefulSet.Labels[LabelVersion], + } + pdb.Spec = policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailable, + Selector: statefulSet.Spec.Selector, + } - err := c.client.Create(ctx, pdb) + return controllerutil.SetControllerReference(statefulSet, pdb, scheme.Scheme) + }) if err != nil { - if k8serrors.IsAlreadyExists(err) { - return nil - } return fmt.Errorf("failed to create pod distruption budget: %w", err) } diff --git a/statefulset-runner/controllers/pdb_test.go b/statefulset-runner/controllers/pdb_test.go index b29afbf1e..c311ede38 100644 --- a/statefulset-runner/controllers/pdb_test.go +++ b/statefulset-runner/controllers/pdb_test.go @@ -29,6 +29,8 @@ var _ = Describe("PDB", func() { creator = controllers.NewPDBUpdater(fakeClient) instances = 2 + fakeClient.GetReturns(k8serrors.NewNotFound(schema.GroupResource{}, "jim")) + stSet = &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ Name: "name", @@ -126,15 +128,5 @@ var _ = Describe("PDB", func() { }) }) }) - - When("the pod distruption budget already exists", func() { - BeforeEach(func() { - fakeClient.CreateReturns(k8serrors.NewAlreadyExists(schema.GroupResource{}, "boom")) - }) - - It("succeeds", func() { - Expect(updateErr).NotTo(HaveOccurred()) - }) - }) }) }) diff --git a/statefulset-runner/controllers/suite_test.go b/statefulset-runner/controllers/suite_test.go index de6b8f8aa..edbd5cabf 100644 --- a/statefulset-runner/controllers/suite_test.go +++ b/statefulset-runner/controllers/suite_test.go @@ -4,7 +4,7 @@ import ( "testing" "go.uber.org/zap/zapcore" - "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/log/zap" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" @@ -12,9 +12,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -33,67 +30,9 @@ var _ = BeforeSuite(func() { }) var _ = BeforeEach(func() { + Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) + fakeClient = new(fake.Client) fakeStatusWriter = &fake.StatusWriter{} fakeClient.StatusReturns(fakeStatusWriter) }) - -func createAppWorkload(namespace, name string) *korifiv1alpha1.AppWorkload { - return &korifiv1alpha1.AppWorkload{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Generation: 1, - Annotations: map[string]string{ - korifiv1alpha1.CFAppLastStopRevisionKey: "lastStopAppRev", - }, - }, - Spec: korifiv1alpha1.AppWorkloadSpec{ - AppGUID: "premium_app_guid_1234", - GUID: "guid_1234", - Version: "version_1234", - Image: "gcr.io/foo/bar", - ImagePullSecrets: []corev1.LocalObjectReference{{Name: "some-secret-name"}}, - Command: []string{ - "/bin/sh", - "-c", - "while true; do echo hello; sleep 10;done", - }, - ProcessType: "worker", - Env: []corev1.EnvVar{}, - StartupProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/healthz", - Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(8080)}, - }, - }, - FailureThreshold: 30, - PeriodSeconds: 2, - }, - LivenessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/healthz", - Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(8080)}, - }, - }, - PeriodSeconds: 30, - FailureThreshold: 1, - }, - Ports: []int32{8888, 9999}, - Instances: 1, - RunnerName: "statefulset-runner", - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceEphemeralStorage: resource.MustParse("2048Mi"), - corev1.ResourceMemory: resource.MustParse("1024Mi"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("5m"), - corev1.ResourceMemory: resource.MustParse("1024Mi"), - }, - }, - }, - } -}