Simplify status condition reasons

Signed-off-by: Catherine Chan-Tse <cchantse@redhat.com>
operator-framework · Sep 10, 2024 · 6d3af51 · 6d3af51
1 parent 0741d36
commit 6d3af51
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 50 deletions.
diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go
@@ -425,19 +425,11 @@ const (
 	TypeBundleDeprecated  = "BundleDeprecated"
 	TypeUnpacked          = "Unpacked"
 
-	ReasonErrorGettingClient = "ErrorGettingClient"
-	ReasonBundleLoadFailed   = "BundleLoadFailed"
-
-	ReasonInstallationFailed = "InstallationFailed"
-	ReasonResolutionFailed   = "ResolutionFailed"
-
-	ReasonSuccess       = "Success"
-	ReasonDeprecated    = "Deprecated"
-	ReasonUpgradeFailed = "UpgradeFailed"
-
-	ReasonUnpackSuccess = "UnpackSuccess"
-	ReasonUnpackFailed  = "UnpackFailed"
+	ReasonSuccess    = "Succeeded"
+	ReasonDeprecated = "Deprecated"
+	ReasonFailed     = "Failed"
 
+	ReasonErrorGettingClient       = "ErrorGettingClient"
 	ReasonErrorGettingReleaseState = "ErrorGettingReleaseState"
 
 	ReasonUnverifiable = "Unverifiable"
@@ -460,15 +452,10 @@ func init() {
 	)
 	// TODO(user): add Reasons from above
 	conditionsets.ConditionReasons = append(conditionsets.ConditionReasons,
-		ReasonResolutionFailed,
-		ReasonInstallationFailed,
 		ReasonSuccess,
 		ReasonDeprecated,
-		ReasonUpgradeFailed,
-		ReasonBundleLoadFailed,
+		ReasonFailed,
 		ReasonErrorGettingClient,
-		ReasonUnpackSuccess,
-		ReasonUnpackFailed,
 		ReasonErrorGettingReleaseState,
 		ReasonUnverifiable,
 	)
@@ -508,11 +495,9 @@ type ClusterExtensionStatus struct {
 	//   - "Unpacked", represents whether or not the bundle contents have been successfully unpacked
 	//
 	// The current set of reasons are:
-	//   - "ResolutionFailed", this reason is set on the "Resolved" condition when an error has occurred during resolution.
-	//   - "InstallationFailed", this reason is set on the "Installed" condition when an error has occurred during installation
-	//   - "Success", this reason is set on the "Resolved" and "Installed" conditions when resolution and installation/upgrading is successful
-	//   - "UnpackSuccess", this reason is set on the "Unpacked" condition when unpacking a bundle's content is successful
-	//   - "UnpackFailed", this reason is set on the "Unpacked" condition when an error has been encountered while unpacking the contents of a bundle
+	//   - "Success", this reason is set on the "Unpacked", "Resolved" and "Installed" conditions when unpacking a bundle's content, resolution and installation/upgrading is successful
+	//   - "Failed", this reason is set on the "Unpacked", "Resolved" and "Installed" conditions when an error has occurred while unpacking the contents of a bundle, during resolution or installation.
+	//
 	//
 	// +patchMergeKey=type
 	// +patchStrategy=merge

diff --git a/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml b/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml
@@ -484,11 +484,8 @@ spec:
                     - "Unpacked", represents whether or not the bundle contents have been successfully unpacked
 
                   The current set of reasons are:
-                    - "ResolutionFailed", this reason is set on the "Resolved" condition when an error has occurred during resolution.
-                    - "InstallationFailed", this reason is set on the "Installed" condition when an error has occurred during installation
-                    - "Success", this reason is set on the "Resolved" and "Installed" conditions when resolution and installation/upgrading is successful
-                    - "UnpackSuccess", this reason is set on the "Unpacked" condition when unpacking a bundle's content is successful
-                    - "UnpackFailed", this reason is set on the "Unpacked" condition when an error has been encountered while unpacking the contents of a bundle
+                    - "Success", this reason is set on the "Unpacked", "Resolved" and "Installed" conditions when unpacking a bundle's content, resolution and installation/upgrading is successful
+                    - "Failed", this reason is set on the "Unpacked", "Resolved" and "Installed" conditions when an error has occurred while unpacking the contents of a bundle, during resolution or installation.
                 items:
                   description: Condition contains details for one aspect of the current
                     state of this API Resource.

diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go
@@ -194,7 +194,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 		setInstallStatus(ext, nil)
 		setResolutionStatus(ext, nil)
 		setResolvedStatusConditionFailed(ext, err.Error())
-		ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonResolutionFailed, err.Error())
+		ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
 		return ctrl.Result{}, err
 	}
 	if finalizeResult.Updated || finalizeResult.StatusUpdated {
@@ -220,7 +220,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 		setInstallStatus(ext, nil)
 		setResolutionStatus(ext, nil)
 		setResolvedStatusConditionFailed(ext, err.Error())
-		ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonResolutionFailed, err.Error())
+		ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
 		return ctrl.Result{}, err
 	}
 
@@ -262,7 +262,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 	switch unpackResult.State {
 	case rukpaksource.StatePending:
 		setStatusUnpackFailed(ext, unpackResult.Message)
-		ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonUnpackFailed, "unpack pending")
+		ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, "unpack pending")
 		return ctrl.Result{}, nil
 	case rukpaksource.StateUnpacked:
 		setStatusUnpacked(ext, fmt.Sprintf("unpack successful: %v", unpackResult.Message))

diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go
@@ -86,7 +86,7 @@ func TestClusterExtensionResolutionFails(t *testing.T) {
 	cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
 	require.NotNil(t, cond)
 	require.Equal(t, metav1.ConditionFalse, cond.Status)
-	require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonFailed, cond.Reason)
 	require.Equal(t, fmt.Sprintf("no package %q found", pkgName), cond.Message)
 
 	verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
@@ -166,7 +166,7 @@ func TestClusterExtensionResolutionSucceeds(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.ReasonUnpackFailed, unpackedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonFailed, unpackedCond.Reason)
 
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
 }
@@ -242,7 +242,7 @@ func TestClusterExtensionUnpackFails(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.ReasonUnpackFailed, unpackedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonFailed, unpackedCond.Reason)
 
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
 }
@@ -320,7 +320,7 @@ func TestClusterExtensionUnpackUnexpectedState(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.ReasonUnpackFailed, unpackedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonFailed, unpackedCond.Reason)
 
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
 }
@@ -402,7 +402,7 @@ func TestClusterExtensionUnpackSucceeds(t *testing.T) {
 	unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked)
 	require.NotNil(t, unpackedCond)
 	require.Equal(t, metav1.ConditionTrue, unpackedCond.Status)
-	require.Equal(t, ocv1alpha1.ReasonUnpackSuccess, unpackedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonSuccess, unpackedCond.Reason)
 
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
 }
@@ -484,13 +484,13 @@ func TestClusterExtensionInstallationFailedApplierFails(t *testing.T) {
 	unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked)
 	require.NotNil(t, unpackedCond)
 	require.Equal(t, metav1.ConditionTrue, unpackedCond.Status)
-	require.Equal(t, ocv1alpha1.ReasonUnpackSuccess, unpackedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonSuccess, unpackedCond.Reason)
 
 	t.Log("By checking the expected installed conditions")
 	installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
 	require.NotNil(t, installedCond)
 	require.Equal(t, metav1.ConditionFalse, installedCond.Status)
-	require.Equal(t, ocv1alpha1.ReasonInstallationFailed, installedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonFailed, installedCond.Reason)
 
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
 }
@@ -575,7 +575,7 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
 	unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked)
 	require.NotNil(t, unpackedCond)
 	require.Equal(t, metav1.ConditionTrue, unpackedCond.Status)
-	require.Equal(t, ocv1alpha1.ReasonUnpackSuccess, unpackedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonSuccess, unpackedCond.Reason)
 
 	t.Log("By checking the expected installed conditions")
 	installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
@@ -675,7 +675,7 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
 	unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked)
 	require.NotNil(t, unpackedCond)
 	require.Equal(t, metav1.ConditionTrue, unpackedCond.Status)
-	require.Equal(t, ocv1alpha1.ReasonUnpackSuccess, unpackedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonSuccess, unpackedCond.Reason)
 
 	t.Log("By checking the expected installed conditions")
 	installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
@@ -772,7 +772,7 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
 	unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked)
 	require.NotNil(t, unpackedCond)
 	require.Equal(t, metav1.ConditionTrue, unpackedCond.Status)
-	require.Equal(t, ocv1alpha1.ReasonUnpackSuccess, unpackedCond.Reason)
+	require.Equal(t, ocv1alpha1.ReasonSuccess, unpackedCond.Reason)
 
 	t.Log("By checking the expected installed conditions")
 	installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)

diff --git a/internal/controllers/common_controller.go b/internal/controllers/common_controller.go
@@ -39,7 +39,7 @@ func setResolvedStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message
 	apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
 		Type:               ocv1alpha1.TypeResolved,
 		Status:             metav1.ConditionFalse,
-		Reason:             ocv1alpha1.ReasonResolutionFailed,
+		Reason:             ocv1alpha1.ReasonFailed,
 		Message:            message,
 		ObservedGeneration: ext.GetGeneration(),
 	})
@@ -61,7 +61,7 @@ func setInstalledStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message
 	apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
 		Type:               ocv1alpha1.TypeInstalled,
 		Status:             metav1.ConditionFalse,
-		Reason:             ocv1alpha1.ReasonInstallationFailed,
+		Reason:             ocv1alpha1.ReasonFailed,
 		Message:            message,
 		ObservedGeneration: ext.GetGeneration(),
 	})
@@ -72,7 +72,7 @@ func setStatusUnpackFailed(ext *ocv1alpha1.ClusterExtension, message string) {
 	apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
 		Type:               ocv1alpha1.TypeUnpacked,
 		Status:             metav1.ConditionFalse,
-		Reason:             ocv1alpha1.ReasonUnpackFailed,
+		Reason:             ocv1alpha1.ReasonFailed,
 		Message:            message,
 		ObservedGeneration: ext.GetGeneration(),
 	})
@@ -82,7 +82,7 @@ func setStatusUnpacked(ext *ocv1alpha1.ClusterExtension, message string) {
 	apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
 		Type:               ocv1alpha1.TypeUnpacked,
 		Status:             metav1.ConditionTrue,
-		Reason:             ocv1alpha1.ReasonUnpackSuccess,
+		Reason:             ocv1alpha1.ReasonSuccess,
 		Message:            message,
 		ObservedGeneration: ext.GetGeneration(),
 	})

diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go
@@ -273,7 +273,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) {
 			return
 		}
 		assert.Equal(ct, metav1.ConditionTrue, cond.Status)
-		assert.Equal(ct, ocv1alpha1.ReasonUnpackSuccess, cond.Reason)
+		assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
 		assert.Contains(ct, cond.Message, "unpack successful")
 	}, pollDuration, pollInterval)
 
@@ -324,7 +324,7 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) {
 			return
 		}
 		assert.Equal(ct, metav1.ConditionFalse, cond.Status)
-		assert.Equal(ct, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
+		assert.Equal(ct, ocv1alpha1.ReasonFailed, cond.Reason)
 		assert.Contains(ct, cond.Message, "in multiple catalogs with the same priority [operatorhubio test-catalog]")
 		assert.Nil(ct, clusterExtension.Status.Resolution)
 	}, pollDuration, pollInterval)
@@ -393,7 +393,7 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
 		if !assert.NotNil(ct, cond) {
 			return
 		}
-		assert.Equal(ct, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
+		assert.Equal(ct, ocv1alpha1.ReasonFailed, cond.Reason)
 		assert.Equal(ct, "error upgrading from currently installed version \"1.0.0\": no bundles found for package \"prometheus\" matching version \"1.2.0\"", cond.Message)
 		assert.Empty(ct, clusterExtension.Status.Resolution)
 	}, pollDuration, pollInterval)
@@ -848,7 +848,7 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes
 			return
 		}
 		assert.Equal(ct, metav1.ConditionTrue, cond.Status)
-		assert.Equal(ct, ocv1alpha1.ReasonUnpackSuccess, cond.Reason)
+		assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
 		assert.Contains(ct, cond.Message, "unpack successful")
 	}, pollDuration, pollInterval)
 
@@ -860,7 +860,7 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes
 			return
 		}
 		assert.Equal(ct, metav1.ConditionFalse, cond.Status)
-		assert.Equal(ct, ocv1alpha1.ReasonInstallationFailed, cond.Reason)
+		assert.Equal(ct, ocv1alpha1.ReasonFailed, cond.Reason)
 		assert.Contains(ct, cond.Message, "forbidden")
 	}, pollDuration, pollInterval)