diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index c9760f95..08dbf189 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/api/v1alpha1/clusterextension_types.go @@ -115,21 +115,17 @@ const ( ReasonErrorGettingClient = "ErrorGettingClient" ReasonBundleLoadFailed = "BundleLoadFailed" - ReasonInstallationFailed = "InstallationFailed" - ReasonInstallationStatusUnknown = "InstallationStatusUnknown" - ReasonInstallationSucceeded = "InstallationSucceeded" - ReasonResolutionFailed = "ResolutionFailed" + ReasonInstallationFailed = "InstallationFailed" + ReasonResolutionFailed = "ResolutionFailed" ReasonSuccess = "Success" ReasonDeprecated = "Deprecated" ReasonUpgradeFailed = "UpgradeFailed" - ReasonUnpackPending = "UnpackPending" ReasonUnpackSuccess = "UnpackSuccess" ReasonUnpackFailed = "UnpackFailed" ReasonErrorGettingReleaseState = "ErrorGettingReleaseState" - ReasonCreateDynamicWatchFailed = "CreateDynamicWatchFailed" ) func init() { @@ -145,7 +141,6 @@ func init() { ) // TODO(user): add Reasons from above conditionsets.ConditionReasons = append(conditionsets.ConditionReasons, - ReasonInstallationSucceeded, ReasonResolutionFailed, ReasonInstallationFailed, ReasonSuccess, @@ -153,12 +148,9 @@ func init() { ReasonUpgradeFailed, ReasonBundleLoadFailed, ReasonErrorGettingClient, - ReasonInstallationStatusUnknown, - ReasonUnpackPending, ReasonUnpackSuccess, ReasonUnpackFailed, ReasonErrorGettingReleaseState, - ReasonCreateDynamicWatchFailed, ) } @@ -167,8 +159,11 @@ type BundleMetadata struct { Version string `json:"version"` } -// ClusterExtensionStatus defines the observed state of ClusterExtension +// ClusterExtensionStatus defines the observed state of ClusterExtension. type ClusterExtensionStatus struct { + // InstalledBundle should only be modified when a new bundle is successfully installed. This ensures that if there + // is a previously successfully installed a bundle, and an upgrade fails, it is still communicated that there is + // still a bundle that is currently installed and owned by the ClusterExtension. // +optional InstalledBundle *BundleMetadata `json:"installedBundle,omitempty"` // +optional diff --git a/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml b/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml index 6331068a..833cc55f 100644 --- a/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml +++ b/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml @@ -95,7 +95,7 @@ spec: - packageName type: object status: - description: ClusterExtensionStatus defines the observed state of ClusterExtension + description: ClusterExtensionStatus defines the observed state of ClusterExtension. properties: conditions: items: @@ -170,6 +170,10 @@ spec: - type x-kubernetes-list-type: map installedBundle: + description: |- + InstalledBundle should only be modified when a new bundle is successfully installed. This ensures that if there + is a previously successfully installed a bundle, and an upgrade fails, it is still communicated that there is + still a bundle that is currently installed and owned by the ClusterExtension. properties: name: type: string diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 3e392174..637edcfa 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -273,13 +273,11 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp switch unpackResult.State { case rukpaksource.StatePending: - setStatusUnpackPending(ext, unpackResult.Message) - setInstalledStatusConditionUnknown(ext, "installation has not been attempted as unpack is pending") - + setStatusUnpackFailed(ext, unpackResult.Message) + ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonUnpackFailed, "unpack 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. + // TODO: https://github.com/operator-framework/rukpak/pull/897 merged, add finalizer to clean the stored bundles if err := r.Storage.Store(ctx, ext, unpackResult.Bundle); err != nil { setStatusUnpackFailed(ext, err.Error()) return ctrl.Result{}, err @@ -378,7 +376,7 @@ 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, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonCreateDynamicWatchFailed, err)) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonInstallationFailed, err)) return ctrl.Result{}, err } @@ -402,7 +400,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp return nil }(); err != nil { ext.Status.InstalledBundle = nil - setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonCreateDynamicWatchFailed, err)) + setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonInstallationFailed, err)) return ctrl.Result{}, err } } diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index 09608afb..4c30c260 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -184,7 +184,7 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) { unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked) require.NotNil(t, unpackedCond) require.Equal(t, metav1.ConditionFalse, unpackedCond.Status) - require.Equal(t, ocv1alpha1.ReasonUnpackPending, unpackedCond.Reason) + require.Equal(t, ocv1alpha1.ReasonUnpackFailed, unpackedCond.Reason) require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) } @@ -242,7 +242,7 @@ func TestClusterExtensionChannelExistsNoVersion(t *testing.T) { unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked) require.NotNil(t, unpackedCond) require.Equal(t, metav1.ConditionFalse, unpackedCond.Status) - require.Equal(t, ocv1alpha1.ReasonUnpackPending, unpackedCond.Reason) + require.Equal(t, ocv1alpha1.ReasonUnpackFailed, unpackedCond.Reason) verifyInvariants(ctx, t, reconciler.Client, clusterExtension) require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) diff --git a/internal/controllers/common_controller.go b/internal/controllers/common_controller.go index 8b07f404..61cf0a95 100644 --- a/internal/controllers/common_controller.go +++ b/internal/controllers/common_controller.go @@ -43,17 +43,6 @@ func setResolvedStatusConditionSuccess(ext *ocv1alpha1.ClusterExtension, message }) } -// setInstalledStatusConditionUnknown sets the installed status condition to unknown. -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: ext.GetGeneration(), - }) -} - // setResolvedStatusConditionFailed sets the resolved status condition to failed. func setResolvedStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message string) { apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ @@ -118,18 +107,6 @@ func setStatusUnpackFailed(ext *ocv1alpha1.ClusterExtension, message string) { }) } -// TODO: verify if we need to update the installBundle status or leave it as is. -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: ocv1alpha1.ReasonUnpackPending, - Message: message, - ObservedGeneration: ext.GetGeneration(), - }) -} - func setStatusUnpacked(ext *ocv1alpha1.ClusterExtension, message string) { apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypeUnpacked,