From 6ec21faae8c064e248b049fcf5ec211dc3185d1a Mon Sep 17 00:00:00 2001 From: everettraven Date: Wed, 31 Jan 2024 14:55:12 -0500 Subject: [PATCH 1/2] (feat): Add healthy status condition to ClusterExtension that is based on the corresponding BundleDeployment's Healthy status condition setting Signed-off-by: everettraven --- api/v1alpha1/clusterextension_types.go | 8 + .../clusterextension_controller.go | 89 ++++++++++- .../clusterextension_controller_test.go | 138 ++++++++++++++++++ 3 files changed, 232 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index b763cb60a..e4759b7d0 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/api/v1alpha1/clusterextension_types.go @@ -77,6 +77,7 @@ const ( // TODO(user): add more Types, here and into init() TypeInstalled = "Installed" TypeResolved = "Resolved" + TypeHealthy = "Healthy" // TypeDeprecated is a rollup condition that is present when // any of the deprecated conditions are present. TypeDeprecated = "Deprecated" @@ -93,6 +94,9 @@ const ( ReasonResolutionUnknown = "ResolutionUnknown" ReasonSuccess = "Success" ReasonDeprecated = "Deprecated" + ReasonHealthy = "Healthy" + ReasonUnhealthy = "Unhealthy" + ReasonHealthStatusUnknown = "HealthStatusUnknown" ) func init() { @@ -104,6 +108,7 @@ func init() { TypePackageDeprecated, TypeChannelDeprecated, TypeBundleDeprecated, + TypeHealthy, ) // TODO(user): add Reasons from above conditionsets.ConditionReasons = append(conditionsets.ConditionReasons, @@ -116,6 +121,9 @@ func init() { ReasonInvalidSpec, ReasonSuccess, ReasonDeprecated, + ReasonHealthy, + ReasonUnhealthy, + ReasonHealthStatusUnknown, ) } diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 126b673f4..619bd065b 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -134,6 +134,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp setResolvedStatusConditionUnknown(&ext.Status.Conditions, "validation has not been attempted as spec is invalid", ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as spec is invalid", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationStatusUnknown, "health checks have not been attempted as spec is invalid", ext.GetGeneration()) return ctrl.Result{}, nil } @@ -146,6 +147,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted due to failure to gather data for resolution", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationStatusUnknown, "health checks have not been attempted due to failure to gather data for resolution", ext.GetGeneration()) return ctrl.Result{}, err } @@ -158,6 +160,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as resolution failed", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationStatusUnknown, "health checks have not been attempted as resolution failed", ext.GetGeneration()) return ctrl.Result{}, err } @@ -171,6 +174,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as resolution failed", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationStatusUnknown, "health checks have not been attempted as resolution failed", ext.GetGeneration()) return ctrl.Result{}, err } @@ -184,12 +188,14 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp if err != nil { setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationFailed, "health checks have not been attempted as installation has failed", ext.GetGeneration()) return ctrl.Result{}, err } bundleProvisioner, err := mapBundleMediaTypeToBundleProvisioner(mediaType) if err != nil { setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationFailed, "health checks have not been attempted as installation has failed", ext.GetGeneration()) return ctrl.Result{}, err } // Ensure a BundleDeployment exists with its bundle source from the bundle @@ -200,6 +206,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp ext.Status.InstalledBundleResource = "" setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationFailed, "health checks have not been attempted as installation has failed", ext.GetGeneration()) return ctrl.Result{}, err } @@ -210,12 +217,13 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp ext.Status.InstalledBundleResource = "" setInstalledStatusConditionUnknown(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationFailed, "health checks have not been attempted as installation has failed", ext.GetGeneration()) return ctrl.Result{}, err } - // Let's set the proper Installed condition and InstalledBundleResource field based on the + // Let's set the proper conditions and status fields based on the // existing BundleDeployment object status. - mapBDStatusToInstalledCondition(existingTypedBundleDeployment, ext) + mapBDStatusToClusterExtensionConditions(existingTypedBundleDeployment, ext) SetDeprecationStatus(ext, bundle) @@ -240,11 +248,12 @@ func (r *ClusterExtensionReconciler) variables(ctx context.Context) ([]deppy.Var return GenerateVariables(allBundles, clusterExtensionList.Items, bundleDeploymentList.Items) } -func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alpha2.BundleDeployment, ext *ocv1alpha1.ClusterExtension) { +func mapBDStatusToClusterExtensionConditions(existingTypedBundleDeployment *rukpakv1alpha2.BundleDeployment, ext *ocv1alpha1.ClusterExtension) { bundleDeploymentReady := apimeta.FindStatusCondition(existingTypedBundleDeployment.Status.Conditions, rukpakv1alpha2.TypeInstalled) if bundleDeploymentReady == nil { ext.Status.InstalledBundleResource = "" setInstalledStatusConditionUnknown(&ext.Status.Conditions, "bundledeployment status is unknown", ext.GetGeneration()) + setHealthyStatusUnknown(&ext.Status.Conditions, ocv1alpha1.ReasonInstallationStatusUnknown, "installation status is unknown", ext.GetGeneration()) return } @@ -255,6 +264,11 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph fmt.Sprintf("bundledeployment not ready: %s", bundleDeploymentReady.Message), ext.GetGeneration(), ) + setHealthyStatusUnhealthy( + &ext.Status.Conditions, + fmt.Sprintf("bundledeployment not ready: %s", bundleDeploymentReady.Message), + ext.GetGeneration(), + ) return } @@ -282,6 +296,45 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph fmt.Sprintf("unknown bundledeployment source type %q", bundleDeploymentSource.Type), ext.GetGeneration(), ) + setHealthyStatusUnknown( + &ext.Status.Conditions, + ocv1alpha1.ReasonInstallationStatusUnknown, + "installation status is unknown", + ext.GetGeneration(), + ) + } + + bundleDeploymentHealthy := apimeta.FindStatusCondition(existingTypedBundleDeployment.Status.Conditions, rukpakv1alpha2.TypeHealthy) + if bundleDeploymentHealthy == nil { + setHealthyStatusUnknown( + &ext.Status.Conditions, + ocv1alpha1.ReasonHealthStatusUnknown, + fmt.Sprintf("bundledeployment %q missing status condition %q", existingTypedBundleDeployment.Name, rukpakv1alpha2.TypeHealthy), + ext.GetGeneration(), + ) + return + } + + switch bundleDeploymentHealthy.Status { + case metav1.ConditionTrue: + setHealthyStatusHealthy( + &ext.Status.Conditions, + "extension is healthy", + ext.GetGeneration(), + ) + case metav1.ConditionFalse: + setHealthyStatusUnhealthy( + &ext.Status.Conditions, + fmt.Sprintf("extension is unhealthy: %s", bundleDeploymentHealthy.Message), + ext.GetGeneration(), + ) + case metav1.ConditionUnknown: + setHealthyStatusUnknown( + &ext.Status.Conditions, + ocv1alpha1.ReasonHealthStatusUnknown, + "extension health is unknown", + ext.GetGeneration(), + ) } } @@ -577,6 +630,36 @@ func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message strin } } +func setHealthyStatusUnknown(conditions *[]metav1.Condition, reason, message string, generation int64) { + apimeta.SetStatusCondition(conditions, metav1.Condition{ + Type: ocv1alpha1.TypeHealthy, + Reason: reason, + Status: metav1.ConditionUnknown, + Message: message, + ObservedGeneration: generation, + }) +} + +func setHealthyStatusHealthy(conditions *[]metav1.Condition, message string, generation int64) { + apimeta.SetStatusCondition(conditions, metav1.Condition{ + Type: ocv1alpha1.TypeHealthy, + Reason: ocv1alpha1.ReasonHealthy, + Status: metav1.ConditionTrue, + Message: message, + ObservedGeneration: generation, + }) +} + +func setHealthyStatusUnhealthy(conditions *[]metav1.Condition, message string, generation int64) { + apimeta.SetStatusCondition(conditions, metav1.Condition{ + Type: ocv1alpha1.TypeHealthy, + Reason: ocv1alpha1.ReasonUnhealthy, + Status: metav1.ConditionFalse, + Message: message, + ObservedGeneration: generation, + }) +} + // Generate reconcile requests for all cluster extensions affected by a catalog change func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) handler.MapFunc { return func(ctx context.Context, _ client.Object) []reconcile.Request { diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index bfe2d7cd2..80673cc97 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -608,6 +608,144 @@ func TestClusterExtensionExpectedBundleDeployment(t *testing.T) { require.NoError(t, cl.DeleteAllOf(ctx, &rukpakv1alpha2.BundleDeployment{})) } +func TestClusterExtensionHealthStatus(t *testing.T) { + cl, reconciler := newClientAndReconciler(t) + ctx := context.Background() + extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} + const pkgName = "prometheus" + + t.Log("When the cluster extension specifies a valid available package") + t.Log("By initializing cluster state") + clusterExtension := &ocv1alpha1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, + Spec: ocv1alpha1.ClusterExtensionSpec{PackageName: pkgName}, + } + require.NoError(t, cl.Create(ctx, clusterExtension)) + + t.Log("When the expected BundleDeployment already exists") + t.Log("When the BundleDeployment spec is up-to-date") + t.Log("By patching the existing BD") + bd := &rukpakv1alpha2.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: extKey.Name, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: ocv1alpha1.GroupVersion.String(), + Kind: "ClusterExtension", + Name: clusterExtension.Name, + UID: clusterExtension.UID, + Controller: pointer.Bool(true), + BlockOwnerDeletion: pointer.Bool(true), + }, + }, + }, + Spec: rukpakv1alpha2.BundleDeploymentSpec{ + ProvisionerClassName: "core-rukpak-io-registry", + Source: rukpakv1alpha2.BundleSource{ + Type: rukpakv1alpha2.SourceTypeImage, + Image: &rukpakv1alpha2.ImageSource{ + Ref: "quay.io/operatorhubio/prometheus@fake2.0.0", + }, + }, + }, + } + + require.NoError(t, cl.Create(ctx, bd)) + bd.Status.ObservedGeneration = bd.GetGeneration() + + t.Log("When the BundleDeployment status is mapped to the expected ClusterExtension status") + t.Log("It verifies cluster extension status when bundle deployment is waiting to be created") + t.Log("By updating the status of bundleDeployment") + require.NoError(t, cl.Status().Update(ctx, bd)) + + t.Log("It verifies cluster extension status when bundleDeployment installed status is true") + apimeta.SetStatusCondition(&bd.Status.Conditions, metav1.Condition{ + Type: rukpakv1alpha2.TypeInstalled, + Status: metav1.ConditionTrue, + Message: "cluster extension installed successfully", + Reason: rukpakv1alpha2.ReasonInstallationSucceeded, + }) + + t.Log("It verifies cluster extension status when bundleDeployment healthy status is unknown") + apimeta.SetStatusCondition(&bd.Status.Conditions, metav1.Condition{ + Type: rukpakv1alpha2.TypeHealthy, + Status: metav1.ConditionUnknown, + Message: "message", + Reason: rukpakv1alpha2.ReasonHealthy, + }) + + t.Log("By updating the status of bundleDeployment") + require.NoError(t, cl.Status().Update(ctx, bd)) + + t.Log("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) + require.Equal(t, ctrl.Result{}, res) + require.NoError(t, err) + + t.Log("By fetching the updated cluster extension after reconcile") + ext := &ocv1alpha1.ClusterExtension{} + require.NoError(t, cl.Get(ctx, extKey, ext)) + + t.Log("By checking the expected conditions") + cond := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeHealthy) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionUnknown, cond.Status) + require.Equal(t, ocv1alpha1.ReasonHealthStatusUnknown, cond.Reason) + + t.Log("It verifies cluster extension status when bundleDeployment healthy status is false") + apimeta.SetStatusCondition(&bd.Status.Conditions, metav1.Condition{ + Type: rukpakv1alpha2.TypeHealthy, + Status: metav1.ConditionFalse, + Message: "unhealthymessage", + Reason: rukpakv1alpha2.ReasonUnhealthy, + }) + + t.Log("By updating the status of bundleDeployment") + require.NoError(t, cl.Status().Update(ctx, bd)) + + t.Log("running reconcile") + res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) + require.Equal(t, ctrl.Result{}, res) + require.NoError(t, err) + + t.Log("By fetching the updated cluster extension after reconcile") + ext = &ocv1alpha1.ClusterExtension{} + require.NoError(t, cl.Get(ctx, extKey, ext)) + + t.Log("By checking the expected conditions") + cond = apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeHealthy) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1alpha1.ReasonUnhealthy, cond.Reason) + require.Equal(t, "extension is unhealthy: unhealthymessage", cond.Message) + + t.Log("It verifies cluster extension status when bundleDeployment healthy status is true") + apimeta.SetStatusCondition(&bd.Status.Conditions, metav1.Condition{ + Type: rukpakv1alpha2.TypeHealthy, + Status: metav1.ConditionTrue, + Message: "healthy", + Reason: rukpakv1alpha2.ReasonHealthy, + }) + + t.Log("By updating the status of bundleDeployment") + require.NoError(t, cl.Status().Update(ctx, bd)) + + t.Log("running reconcile") + res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) + require.Equal(t, ctrl.Result{}, res) + require.NoError(t, err) + + t.Log("By fetching the updated cluster extension after reconcile") + ext = &ocv1alpha1.ClusterExtension{} + require.NoError(t, cl.Get(ctx, extKey, ext)) + + t.Log("By checking the expected conditions") + cond = apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeHealthy) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionTrue, cond.Status) + require.Equal(t, ocv1alpha1.ReasonHealthy, cond.Reason) +} + func TestClusterExtensionDuplicatePackage(t *testing.T) { cl, reconciler := newClientAndReconciler(t) ctx := context.Background() From d92d0378a0e3c2fc1021c7a1240da9dad8a54bc0 Mon Sep 17 00:00:00 2001 From: everettraven Date: Thu, 1 Feb 2024 13:55:18 -0500 Subject: [PATCH 2/2] update e2e tests to get expected status condition length from conditionsets Signed-off-by: everettraven --- test/e2e/install_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/e2e/install_test.go b/test/e2e/install_test.go index 37502deb5..0899a59b4 100644 --- a/test/e2e/install_test.go +++ b/test/e2e/install_test.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/conditionsets" ) const ( @@ -81,7 +82,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) { t.Log("By eventually reporting a successful resolution and bundle path") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) - assert.Len(ct, clusterExtension.Status.Conditions, 6) + assert.Len(ct, clusterExtension.Status.Conditions, len(conditionsets.ConditionTypes)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) if !assert.NotNil(ct, cond) { return @@ -137,7 +138,7 @@ func TestClusterExtensionInstallPlain(t *testing.T) { t.Log("By eventually reporting a successful resolution and bundle path") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) - assert.Len(ct, clusterExtension.Status.Conditions, 6) + assert.Len(ct, clusterExtension.Status.Conditions, len(conditionsets.ConditionTypes)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) if !assert.NotNil(ct, cond) { return