diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index c69803614..557c0570d 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/api/v1alpha1/clusterextension_types.go @@ -106,7 +106,14 @@ const ( ReasonDeprecated = "Deprecated" ReasonUpgradeFailed = "UpgradeFailed" ReasonHasValidBundleUnknown = "HasValidBundleUnknown" - ReasonUnpackPending = "UnpackPending" + + ReasonUnpackPending = "UnpackPending" + ReasonUnpackSuccess = "UnpackSuccess" + ReasonUnpackFailed = "UnpackFailed" + ReasonUnpacking = "Unpacking" + + ReasonErrorGettingReleaseState = "ErrorGettingReleaseState" + ReasonCreateDynamicWatchFailed = "CreateDynamicWatchFailed" ) func init() { @@ -134,6 +141,11 @@ func init() { ReasonInstallationStatusUnknown, ReasonHasValidBundleUnknown, ReasonUnpackPending, + ReasonUnpackSuccess, + ReasonUnpacking, + ReasonUnpackFailed, + ReasonErrorGettingReleaseState, + ReasonCreateDynamicWatchFailed, ) } diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index ae75bcc59..7adb6f151 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -199,7 +199,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp // Note: We don't distinguish between resolution-specific errors and generic errors ext.Status.ResolvedBundle = nil ext.Status.InstalledBundle = nil - setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) + setResolvedStatusConditionFailed(ext, err.Error()) ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonResolutionFailed, err.Error()) return ctrl.Result{}, err } @@ -207,9 +207,9 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp if err := r.validateBundle(bundle); err != nil { ext.Status.ResolvedBundle = nil ext.Status.InstalledBundle = nil - setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) - setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) - setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) + setResolvedStatusConditionFailed(ext, err.Error()) + setInstalledStatusConditionFailed(ext, err.Error()) + setDeprecationStatusesUnknown(ext, "deprecation checks have not been attempted as installation has failed") return ctrl.Result{}, err } // set deprecation status after _successful_ resolution @@ -219,13 +219,13 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp if err != nil { ext.Status.ResolvedBundle = nil ext.Status.InstalledBundle = nil - setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) - setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.Generation) + setResolvedStatusConditionFailed(ext, err.Error()) + setInstalledStatusConditionFailed(ext, err.Error()) return ctrl.Result{}, err } ext.Status.ResolvedBundle = bundleMetadataFor(bundle) - setResolvedStatusConditionSuccess(&ext.Status.Conditions, fmt.Sprintf("resolved to %q", bundle.Image), ext.GetGeneration()) + setResolvedStatusConditionSuccess(ext, fmt.Sprintf("resolved to %q", bundle.Image)) // Generate a BundleDeployment from the ClusterExtension to Unpack. // Note: The BundleDeployment here is not a k8s API, its a simple Go struct which @@ -233,50 +233,53 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp bd := r.generateBundleDeploymentForUnpack(bundle.Image, ext) unpackResult, err := r.Unpacker.Unpack(ctx, bd) if err != nil { - return ctrl.Result{}, updateStatusUnpackFailing(&ext.Status, err) + setStatusUnpackFailed(ext, err.Error()) + return ctrl.Result{}, err } switch unpackResult.State { case rukpaksource.StatePending: - updateStatusUnpackPending(&ext.Status, unpackResult, ext.GetGeneration()) + setStatusUnpackPending(ext, unpackResult.Message) // There must be a limit to number of entries if status is stuck at // unpack pending. - setHasValidBundleUnknown(&ext.Status.Conditions, "unpack pending", ext.GetGeneration()) - setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as unpack is pending", ext.GetGeneration()) + setHasValidBundleUnknown(ext, "unpack pending") + setInstalledStatusConditionUnknown(ext, "installation has not been attempted as unpack is pending") return ctrl.Result{}, nil case rukpaksource.StateUnpacking: - updateStatusUnpacking(&ext.Status, unpackResult) - setHasValidBundleUnknown(&ext.Status.Conditions, "unpack pending", ext.GetGeneration()) - setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as unpack is pending", ext.GetGeneration()) + setStatusUnpacking(ext, unpackResult.Message) + setHasValidBundleUnknown(ext, "unpack pending") + setInstalledStatusConditionUnknown(ext, "installation has not been attempted as unpack is pending") return ctrl.Result{}, nil case rukpaksource.StateUnpacked: // TODO: Add finalizer to clean the stored bundles, after https://github.com/operator-framework/rukpak/pull/897 // merges. if err := r.Storage.Store(ctx, ext, unpackResult.Bundle); err != nil { - return ctrl.Result{}, updateStatusUnpackFailing(&ext.Status, err) + setStatusUnpackFailed(ext, err.Error()) + return ctrl.Result{}, err } - updateStatusUnpacked(&ext.Status, unpackResult) + setStatusUnpacked(ext, unpackResult.Message) default: - return ctrl.Result{}, updateStatusUnpackFailing(&ext.Status, err) + setStatusUnpackFailed(ext, err.Error()) + return ctrl.Result{}, err } bundleFS, err := r.Storage.Load(ctx, ext) if err != nil { - setHasValidBundleFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) + setHasValidBundleFailed(ext, err.Error()) return ctrl.Result{}, err } chrt, values, err := r.Handler.Handle(ctx, bundleFS, ext) if err != nil { - setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) + setInstalledStatusConditionFailed(ext, err.Error()) return ctrl.Result{}, err } ac, err := r.ActionClientGetter.ActionClientFor(ctx, ext) if err != nil { ext.Status.InstalledBundle = nil - setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonErrorGettingClient, err), ext.Generation) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonErrorGettingClient, err)) return ctrl.Result{}, err } @@ -292,7 +295,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp rel, state, err := r.getReleaseState(ac, ext, chrt, values, post) if err != nil { - setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("%s:%v", rukpakv1alpha2.ReasonErrorGettingReleaseState, err), ext.Generation) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonErrorGettingReleaseState, err)) return ctrl.Result{}, err } @@ -304,18 +307,18 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp return nil }, helmclient.AppendInstallPostRenderer(post)) if err != nil { - setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonInstallationFailed, err), ext.Generation) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonInstallationFailed, err)) return ctrl.Result{}, err } case stateNeedsUpgrade: rel, err = ac.Upgrade(ext.GetName(), r.ReleaseNamespace, chrt, values, helmclient.AppendUpgradePostRenderer(post)) if err != nil { - setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonUpgradeFailed, err), ext.Generation) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonUpgradeFailed, err)) return ctrl.Result{}, err } case stateUnchanged: if err := ac.Reconcile(rel); err != nil { - setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonResolutionFailed, err), ext.Generation) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonResolutionFailed, err)) return ctrl.Result{}, err } default: @@ -324,14 +327,14 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name)) if err != nil { - setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("%s:%v", rukpakv1alpha2.ReasonCreateDynamicWatchFailed, err), ext.Generation) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonCreateDynamicWatchFailed, err)) return ctrl.Result{}, err } for _, obj := range relObjects { uMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) if err != nil { - setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("%s:%v", rukpakv1alpha2.ReasonCreateDynamicWatchFailed, err), ext.Generation) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonCreateDynamicWatchFailed, err)) return ctrl.Result{}, err } @@ -353,12 +356,12 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp return nil }(); err != nil { ext.Status.InstalledBundle = nil - setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("%s:%v", rukpakv1alpha2.ReasonCreateDynamicWatchFailed, err), ext.Generation) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonCreateDynamicWatchFailed, err)) return ctrl.Result{}, err } } ext.Status.InstalledBundle = bundleMetadataFor(bundle) - setInstalledStatusConditionSuccess(&ext.Status.Conditions, fmt.Sprintf("Instantiated bundle %s successfully", ext.GetName()), ext.Generation) + setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Instantiated bundle %s successfully", ext.GetName())) return ctrl.Result{}, nil } diff --git a/internal/controllers/common_controller.go b/internal/controllers/common_controller.go index 047f1176b..ab99574da 100644 --- a/internal/controllers/common_controller.go +++ b/internal/controllers/common_controller.go @@ -22,9 +22,6 @@ import ( apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" - "github.com/operator-framework/rukpak/pkg/source" - ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) @@ -36,84 +33,84 @@ type BundleProvider interface { } // setResolvedStatusConditionSuccess sets the resolved status condition to success. -func setResolvedStatusConditionSuccess(conditions *[]metav1.Condition, message string, generation int64) { - apimeta.SetStatusCondition(conditions, metav1.Condition{ +func setResolvedStatusConditionSuccess(ext *ocv1alpha1.ClusterExtension, message string) { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypeResolved, Status: metav1.ConditionTrue, Reason: ocv1alpha1.ReasonSuccess, Message: message, - ObservedGeneration: generation, + ObservedGeneration: ext.GetGeneration(), }) } // setInstalledStatusConditionUnknown sets the installed status condition to unknown. -func setInstalledStatusConditionUnknown(conditions *[]metav1.Condition, message string, generation int64) { - apimeta.SetStatusCondition(conditions, metav1.Condition{ +func setInstalledStatusConditionUnknown(ext *ocv1alpha1.ClusterExtension, message string) { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypeInstalled, Status: metav1.ConditionUnknown, Reason: ocv1alpha1.ReasonInstallationStatusUnknown, Message: message, - ObservedGeneration: generation, + ObservedGeneration: ext.GetGeneration(), }) } // setHasValidBundleUnknown sets the valid bundle condition to unknown. -func setHasValidBundleUnknown(conditions *[]metav1.Condition, message string, generation int64) { - apimeta.SetStatusCondition(conditions, metav1.Condition{ +func setHasValidBundleUnknown(ext *ocv1alpha1.ClusterExtension, message string) { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypeHasValidBundle, Status: metav1.ConditionUnknown, Reason: ocv1alpha1.ReasonHasValidBundleUnknown, Message: message, - ObservedGeneration: generation, + ObservedGeneration: ext.GetGeneration(), }) } // setHasValidBundleFalse sets the ivalid bundle condition to false -func setHasValidBundleFailed(conditions *[]metav1.Condition, message string, generation int64) { - apimeta.SetStatusCondition(conditions, metav1.Condition{ +func setHasValidBundleFailed(ext *ocv1alpha1.ClusterExtension, message string) { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypeHasValidBundle, Status: metav1.ConditionFalse, Reason: ocv1alpha1.ReasonBundleLoadFailed, Message: message, - ObservedGeneration: generation, + ObservedGeneration: ext.GetGeneration(), }) } // setResolvedStatusConditionFailed sets the resolved status condition to failed. -func setResolvedStatusConditionFailed(conditions *[]metav1.Condition, message string, generation int64) { - apimeta.SetStatusCondition(conditions, metav1.Condition{ +func setResolvedStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message string) { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypeResolved, Status: metav1.ConditionFalse, Reason: ocv1alpha1.ReasonResolutionFailed, Message: message, - ObservedGeneration: generation, + ObservedGeneration: ext.GetGeneration(), }) } // setInstalledStatusConditionSuccess sets the installed status condition to success. -func setInstalledStatusConditionSuccess(conditions *[]metav1.Condition, message string, generation int64) { - apimeta.SetStatusCondition(conditions, metav1.Condition{ +func setInstalledStatusConditionSuccess(ext *ocv1alpha1.ClusterExtension, message string) { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypeInstalled, Status: metav1.ConditionTrue, Reason: ocv1alpha1.ReasonSuccess, Message: message, - ObservedGeneration: generation, + ObservedGeneration: ext.GetGeneration(), }) } // setInstalledStatusConditionFailed sets the installed status condition to failed. -func setInstalledStatusConditionFailed(conditions *[]metav1.Condition, message string, generation int64) { - apimeta.SetStatusCondition(conditions, metav1.Condition{ +func setInstalledStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message string) { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypeInstalled, Status: metav1.ConditionFalse, Reason: ocv1alpha1.ReasonInstallationFailed, Message: message, - ObservedGeneration: generation, + ObservedGeneration: ext.GetGeneration(), }) } // setDeprecationStatusesUnknown sets the deprecation status conditions to unknown. -func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message string, generation int64) { +func setDeprecationStatusesUnknown(ext *ocv1alpha1.ClusterExtension, message string) { conditionTypes := []string{ ocv1alpha1.TypeDeprecated, ocv1alpha1.TypePackageDeprecated, @@ -122,55 +119,57 @@ func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message strin } for _, conditionType := range conditionTypes { - apimeta.SetStatusCondition(conditions, metav1.Condition{ + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: conditionType, Reason: ocv1alpha1.ReasonDeprecated, Status: metav1.ConditionUnknown, Message: message, - ObservedGeneration: generation, + ObservedGeneration: ext.GetGeneration(), }) } } -func updateStatusUnpackFailing(status *ocv1alpha1.ClusterExtensionStatus, err error) error { - status.InstalledBundle = nil - apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: rukpakv1alpha2.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: rukpakv1alpha2.ReasonUnpackFailed, - Message: err.Error(), +func setStatusUnpackFailed(ext *ocv1alpha1.ClusterExtension, message string) { + ext.Status.InstalledBundle = nil + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1alpha1.TypeUnpacked, + Status: metav1.ConditionFalse, + Reason: ocv1alpha1.ReasonUnpackFailed, + Message: message, + ObservedGeneration: ext.GetGeneration(), }) - return err } // TODO: verify if we need to update the installBundle status or leave it as is. -func updateStatusUnpackPending(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result, generation int64) { - status.InstalledBundle = nil - apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: rukpakv1alpha2.TypeUnpacked, +func setStatusUnpackPending(ext *ocv1alpha1.ClusterExtension, message string) { + ext.Status.InstalledBundle = nil + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1alpha1.TypeUnpacked, Status: metav1.ConditionFalse, - Reason: rukpakv1alpha2.ReasonUnpackPending, - Message: result.Message, - ObservedGeneration: generation, + Reason: ocv1alpha1.ReasonUnpackPending, + Message: message, + ObservedGeneration: ext.GetGeneration(), }) } // TODO: verify if we need to update the installBundle status or leave it as is. -func updateStatusUnpacking(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result) { - status.InstalledBundle = nil - apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: rukpakv1alpha2.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: rukpakv1alpha2.ReasonUnpacking, - Message: result.Message, +func setStatusUnpacking(ext *ocv1alpha1.ClusterExtension, message string) { + ext.Status.InstalledBundle = nil + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1alpha1.TypeUnpacked, + Status: metav1.ConditionFalse, + Reason: ocv1alpha1.ReasonUnpacking, + Message: message, + ObservedGeneration: ext.GetGeneration(), }) } -func updateStatusUnpacked(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result) { - apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: rukpakv1alpha2.TypeUnpacked, - Status: metav1.ConditionTrue, - Reason: rukpakv1alpha2.ReasonUnpackSuccessful, - Message: result.Message, +func setStatusUnpacked(ext *ocv1alpha1.ClusterExtension, message string) { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1alpha1.TypeUnpacked, + Status: metav1.ConditionTrue, + Reason: ocv1alpha1.ReasonUnpackSuccess, + Message: message, + ObservedGeneration: ext.GetGeneration(), }) } diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index 721a0f716..5eca7747c 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -25,7 +25,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" - rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" ) @@ -97,12 +96,12 @@ func TestClusterExtensionInstallRegistry(t *testing.T) { t.Log("By eventually reporting a successful unpacked") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, rukpakv1alpha2.TypeUnpacked) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked) if !assert.NotNil(ct, cond) { return } assert.Equal(ct, metav1.ConditionTrue, cond.Status) - assert.Equal(ct, rukpakv1alpha2.ReasonUnpackSuccessful, cond.Reason) + assert.Equal(ct, ocv1alpha1.ReasonUnpackSuccess, cond.Reason) assert.Contains(ct, cond.Message, "Successfully unpacked") }, pollDuration, pollInterval)