Simplify status condition reasons (#1241)

Signed-off-by: Catherine Chan-Tse <[email protected]>

Author: oceanc80

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

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.

docs/refs/api/operator-controller-api-reference.md

@@ -210,7 +210,7 @@ _Appears in:_
 | --- | --- | --- | --- |
 | `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ |  |  |  |
 | `resolution` _[ClusterExtensionResolutionStatus](#clusterextensionresolutionstatus)_ |  |  |  |
-| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#condition-v1-meta) array_ | conditions is a representation of the current state for this ClusterExtension.<br />The status is represented by a set of "conditions".<br /><br />Each condition is generally structured in the following format:<br />  - Type: a string representation of the condition type. More or less the condition "name".<br />  - Status: a string representation of the state of the condition. Can be one of ["True", "False", "Unknown"].<br />  - Reason: a string representation of the reason for the current state of the condition. Typically useful for building automation around particular Type+Reason combinations.<br />  - Message: a human readable message that further elaborates on the state of the condition<br /><br />The current set of condition types are:<br />  - "Installed", represents whether or not the package referenced in the spec.packageName field has been installed<br />  - "Resolved", represents whether or not a bundle was found that satisfies the selection criteria outlined in the spec<br />  - "Deprecated", represents an aggregation of the PackageDeprecated, ChannelDeprecated, and BundleDeprecated condition types.<br />  - "PackageDeprecated", represents whether or not the package specified in the spec.packageName field has been deprecated<br />  - "ChannelDeprecated", represents whether or not the channel specified in spec.channel has been deprecated<br />  - "BundleDeprecated", represents whether or not the bundle installed is deprecated<br />  - "Unpacked", represents whether or not the bundle contents have been successfully unpacked<br /><br />The current set of reasons are:<br />  - "ResolutionFailed", this reason is set on the "Resolved" condition when an error has occurred during resolution.<br />  - "InstallationFailed", this reason is set on the "Installed" condition when an error has occurred during installation<br />  - "Success", this reason is set on the "Resolved" and "Installed" conditions when resolution and installation/upgrading is successful<br />  - "UnpackSuccess", this reason is set on the "Unpacked" condition when unpacking a bundle's content is successful<br />  - "UnpackFailed", this reason is set on the "Unpacked" condition when an error has been encountered while unpacking the contents of a bundle |  |  |
+| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#condition-v1-meta) array_ | conditions is a representation of the current state for this ClusterExtension.<br />The status is represented by a set of "conditions".<br /><br />Each condition is generally structured in the following format:<br />  - Type: a string representation of the condition type. More or less the condition "name".<br />  - Status: a string representation of the state of the condition. Can be one of ["True", "False", "Unknown"].<br />  - Reason: a string representation of the reason for the current state of the condition. Typically useful for building automation around particular Type+Reason combinations.<br />  - Message: a human readable message that further elaborates on the state of the condition<br /><br />The current set of condition types are:<br />  - "Installed", represents whether or not the package referenced in the spec.packageName field has been installed<br />  - "Resolved", represents whether or not a bundle was found that satisfies the selection criteria outlined in the spec<br />  - "Deprecated", represents an aggregation of the PackageDeprecated, ChannelDeprecated, and BundleDeprecated condition types.<br />  - "PackageDeprecated", represents whether or not the package specified in the spec.packageName field has been deprecated<br />  - "ChannelDeprecated", represents whether or not the channel specified in spec.channel has been deprecated<br />  - "BundleDeprecated", represents whether or not the bundle installed is deprecated<br />  - "Unpacked", represents whether or not the bundle contents have been successfully unpacked<br /><br />The current set of reasons are:<br />  - "Success", this reason is set on the "Unpacked", "Resolved" and "Installed" conditions when unpacking a bundle's content, resolution and installation/upgrading is successful<br />  - "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. |  |  |
 
 
 #### PreflightConfig

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
 	}
 
@@ -263,7 +263,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))

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)

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(),
 	})

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)