add Progressing condition

Signed-off-by: Ankita Thomas <ankithom@redhat.com>

api/v1alpha1/clusterextension_types.go

@@ -93,8 +93,6 @@ const (
 	ReasonResolutionUnknown         = "ResolutionUnknown"
 	ReasonSuccess                   = "Success"
 	ReasonDeprecated                = "Deprecated"
-	ReasonDeleteFailed              = "DeleteFailed"
-	ReasonDeleting                  = "Deleting"
 )
 
 func init() {
@@ -118,8 +116,6 @@ func init() {
 		ReasonInvalidSpec,
 		ReasonSuccess,
 		ReasonDeprecated,
-		ReasonDeleteFailed,
-		ReasonDeleting,
 	)
 }
 

api/v1alpha1/clusterextension_types_test.go

@@ -22,7 +22,7 @@ func TestClusterExtensionTypeRegistration(t *testing.T) {
 	}
 
 	for _, tt := range types {
-		if !slices.Contains(conditionsets.ConditionTypes, tt) {
+		if !slices.Contains(conditionsets.ConditionTypes, tt) && !slices.Contains(conditionsets.ExtensionConditionTypes, tt) {
 			t.Errorf("append Type%s to conditionsets.ConditionTypes in this package's init function", tt)
 		}
 	}
@@ -32,6 +32,11 @@ func TestClusterExtensionTypeRegistration(t *testing.T) {
 			t.Errorf("there must be a Type%[1]s string literal constant for type %[1]q (i.e. 'const Type%[1]s = %[1]q')", tt)
 		}
 	}
+	for _, tt := range conditionsets.ExtensionConditionTypes {
+		if !slices.Contains(types, tt) {
+			t.Errorf("there must be a Type%[1]s string literal constant for type %[1]q (i.e. 'const Type%[1]s = %[1]q')", tt)
+		}
+	}
 }
 
 func TestClusterExtensionReasonRegistration(t *testing.T) {
@@ -41,7 +46,7 @@ func TestClusterExtensionReasonRegistration(t *testing.T) {
 	}
 
 	for _, r := range reasons {
-		if !slices.Contains(conditionsets.ConditionReasons, r) {
+		if !slices.Contains(conditionsets.ConditionReasons, r) && !slices.Contains(conditionsets.ExtensionConditionReasons, r) {
 			t.Errorf("append Reason%s to conditionsets.ConditionReasons in this package's init function.", r)
 		}
 	}
@@ -50,6 +55,11 @@ func TestClusterExtensionReasonRegistration(t *testing.T) {
 			t.Errorf("there must be a Reason%[1]s string literal constant for reason %[1]q (i.e. 'const Reason%[1]s = %[1]q')", r)
 		}
 	}
+	for _, r := range conditionsets.ExtensionConditionReasons {
+		if !slices.Contains(reasons, r) {
+			t.Errorf("there must be a Reason%[1]s string literal constant for reason %[1]q (i.e. 'const Reason%[1]s = %[1]q')", r)
+		}
+	}
 }
 
 // parseConstants parses the values of the top-level constants in the current

api/v1alpha1/extension_types.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"github.com/operator-framework/operator-controller/internal/conditionsets"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -133,6 +134,25 @@ type ExtensionList struct {
 	Items           []Extension `json:"items"`
 }
 
+const (
+	// TypeProgressing indicates whether operator-controller is
+	// reconciling, installing, updating or deleting an extension.
+	TypeProgressing = "Progressing"
+
+	ReasonProgressing                = "Progressing"
+	ReasonFailedToReachDesiredIntent = "FailedToReachDesiredIntent"
+	ReasonReachedDesiredIntent       = "ReachedDesiredIntent"
+)
+
 func init() {
 	SchemeBuilder.Register(&Extension{}, &ExtensionList{})
+
+	conditionsets.ExtensionConditionTypes = []string{
+		TypeProgressing,
+	}
+	conditionsets.ExtensionConditionReasons = []string{
+		ReasonProgressing,
+		ReasonFailedToReachDesiredIntent,
+		ReasonReachedDesiredIntent,
+	}
 }

internal/conditionsets/conditionsets.go

@@ -22,3 +22,7 @@ package conditionsets
 // NOTE: These are populated by init() in api/v1alpha1/clusterextension_types.go
 var ConditionTypes []string
 var ConditionReasons []string
+
+// ExtensionConditionTypes is the set of Extension exclusive condition Types.
+var ExtensionConditionTypes []string
+var ExtensionConditionReasons []string

internal/controllers/common_controller.go

@@ -65,17 +65,6 @@ func setResolvedStatusConditionFailed(conditions *[]metav1.Condition, message st
 	})
 }
 
-// setResolvedStatusConditionUnknown sets the resolved status condition to unknown.
-func setResolvedStatusConditionUnknown(conditions *[]metav1.Condition, message string, generation int64) {
-	apimeta.SetStatusCondition(conditions, metav1.Condition{
-		Type:               ocv1alpha1.TypeResolved,
-		Status:             metav1.ConditionUnknown,
-		Reason:             ocv1alpha1.ReasonResolutionUnknown,
-		Message:            message,
-		ObservedGeneration: generation,
-	})
-}
-
 // setInstalledStatusConditionSuccess sets the installed status condition to success.
 func setInstalledStatusConditionSuccess(conditions *[]metav1.Condition, message string, generation int64) {
 	apimeta.SetStatusCondition(conditions, metav1.Condition{
@@ -98,29 +87,7 @@ func setInstalledStatusConditionFailed(conditions *[]metav1.Condition, message s
 	})
 }
 
-// setInstalledStatusConditionDeleting sets the installed status condition to unknown for deletes in progress.
-func setInstalledStatusConditionDeleting(conditions *[]metav1.Condition, message string, generation int64) {
-	apimeta.SetStatusCondition(conditions, metav1.Condition{
-		Type:               ocv1alpha1.TypeInstalled,
-		Status:             metav1.ConditionUnknown,
-		Reason:             ocv1alpha1.ReasonDeleting,
-		Message:            message,
-		ObservedGeneration: generation,
-	})
-}
-
-// setInstalledStatusConditionDeleteFailed sets the installed status condition to unknown for failed deletes.
-func setInstalledStatusConditionDeleteFailed(conditions *[]metav1.Condition, message string, generation int64) {
-	apimeta.SetStatusCondition(conditions, metav1.Condition{
-		Type:               ocv1alpha1.TypeInstalled,
-		Status:             metav1.ConditionUnknown,
-		Reason:             ocv1alpha1.ReasonDeleteFailed,
-		Message:            message,
-		ObservedGeneration: generation,
-	})
-}
-
-// setDEprecationStatusesUnknown sets the deprecation status conditions to unknown.
+// setDeprecationStatusesUnknown sets the deprecation status conditions to unknown.
 func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message string, generation int64) {
 	conditionTypes := []string{
 		ocv1alpha1.TypeDeprecated,
@@ -139,3 +106,36 @@ func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message strin
 		})
 	}
 }
+
+// setProgressingStatusConditionSuccess sets the progressing status condition to false for a successful install or upgrade.
+func setProgressingStatusConditionSuccess(conditions *[]metav1.Condition, message string, generation int64) {
+	apimeta.SetStatusCondition(conditions, metav1.Condition{
+		Type:               ocv1alpha1.TypeProgressing,
+		Status:             metav1.ConditionFalse,
+		Reason:             ocv1alpha1.ReasonReachedDesiredIntent,
+		Message:            message,
+		ObservedGeneration: generation,
+	})
+}
+
+// setProgressingStatusConditionFailed sets the progressing status condition to False for a failed install or upgrade.
+func setProgressingStatusConditionFailed(conditions *[]metav1.Condition, message string, generation int64) {
+	apimeta.SetStatusCondition(conditions, metav1.Condition{
+		Type:               ocv1alpha1.TypeProgressing,
+		Status:             metav1.ConditionFalse,
+		Reason:             ocv1alpha1.ReasonFailedToReachDesiredIntent,
+		Message:            message,
+		ObservedGeneration: generation,
+	})
+}
+
+// setProgressingStatusConditionProgressing sets the progressing status condition to true for an app being reconciled.
+func setProgressingStatusConditionProgressing(conditions *[]metav1.Condition, message string, generation int64) {
+	apimeta.SetStatusCondition(conditions, metav1.Condition{
+		Type:               ocv1alpha1.TypeProgressing,
+		Status:             metav1.ConditionTrue,
+		Reason:             ocv1alpha1.ReasonProgressing,
+		Message:            message,
+		ObservedGeneration: generation,
+	})
+}

internal/controllers/extension_controller.go

@@ -127,14 +127,14 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext
 	if !features.OperatorControllerFeatureGate.Enabled(features.EnableExtensionAPI) {
 		l.Info("extension feature is gated", "name", ext.GetName(), "namespace", ext.GetNamespace())
 
-		// Set the TypeInstalled condition to Unknown to indicate that the resolution
+		// Set the TypeInstalled condition to Failed to indicate that the resolution
 		// hasn't been attempted yet, due to the spec being invalid.
 		ext.Status.InstalledBundle = nil
-		setInstalledStatusConditionUnknown(&ext.Status.Conditions, "extension feature is disabled", ext.GetGeneration())
-		// Set the TypeResolved condition to Unknown to indicate that the resolution
+		setInstalledStatusConditionFailed(&ext.Status.Conditions, "extension feature is disabled", ext.GetGeneration())
+		// Set the TypeResolved condition to Failed to indicate that the resolution
 		// hasn't been attempted yet, due to the spec being invalid.
 		ext.Status.ResolvedBundle = nil
-		setResolvedStatusConditionUnknown(&ext.Status.Conditions, "extension feature is disabled", ext.GetGeneration())
+		setResolvedStatusConditionFailed(&ext.Status.Conditions, "extension feature is disabled", ext.GetGeneration())
 
 		setDeprecationStatusesUnknown(&ext.Status.Conditions, "extension feature is disabled", ext.GetGeneration())
 		return ctrl.Result{}, nil
@@ -150,8 +150,10 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext
 	// TODO: Improve the resolution logic.
 	bundle, err := r.resolve(ctx, *ext)
 	if err != nil {
-		ext.Status.InstalledBundle = nil
-		setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as resolution failed", ext.GetGeneration())
+		if c := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled); c == nil {
+			ext.Status.InstalledBundle = nil
+			setInstalledStatusConditionFailed(&ext.Status.Conditions, "installation has not been attempted as resolution failed", ext.GetGeneration())
+		}
 		ext.Status.ResolvedBundle = nil
 		setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
 		setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as resolution failed", ext.GetGeneration())
@@ -162,35 +164,47 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext
 	ext.Status.ResolvedBundle = bundleMetadataFor(bundle)
 	setResolvedStatusConditionSuccess(&ext.Status.Conditions, fmt.Sprintf("resolved to %q", bundle.Image), ext.GetGeneration())
 
+	// Populate the deprecation status using the resolved bundle
+	SetDeprecationStatusInExtension(ext, bundle)
+
 	mediaType, err := bundle.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())
+		if c := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled); c == nil {
+			ext.Status.InstalledBundle = nil
+			setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("failed to read bundle mediaType: %v", err), ext.GetGeneration())
+		}
+		setProgressingStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("failed to read bundle mediaType: %v", err), ext.GetGeneration())
 		return ctrl.Result{}, err
 	}
 
 	// TODO: this needs to include the registryV1 bundle option. As of this PR, this only supports direct
 	// installation of a set of manifests.
 	if mediaType != catalogmetadata.MediaTypePlain {
-		// Set the TypeInstalled condition to Unknown to indicate that the resolution
-		// hasn't been attempted yet, due to the spec being invalid.
-		ext.Status.InstalledBundle = nil
-		setInstalledStatusConditionUnknown(&ext.Status.Conditions, fmt.Sprintf("bundle type %s not supported currently", mediaType), ext.GetGeneration())
-		setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
+		if c := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled); c == nil {
+			// Set the TypeInstalled condition to Failed to indicate that the resolution
+			// hasn't been attempted yet, due to the spec being invalid.
+			ext.Status.InstalledBundle = nil
+			setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("bundle type %s not supported currently", mediaType), ext.GetGeneration())
+		}
+		setProgressingStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("bundle type %s not supported currently", mediaType), ext.GetGeneration())
 		return ctrl.Result{}, nil
 	}
 
 	app, err := r.GenerateExpectedApp(*ext, bundle)
 	if err != nil {
-		setInstalledStatusConditionUnknown(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
-		setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
+		if c := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled); c == nil {
+			ext.Status.InstalledBundle = nil
+			setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
+		}
+		setProgressingStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
 		return ctrl.Result{}, err
 	}
 
 	if err := r.ensureApp(ctx, app); err != nil {
 		// originally Reason: ocv1alpha1.ReasonInstallationFailed
 		ext.Status.InstalledBundle = nil
 		setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
+		setProgressingStatusConditionProgressing(&ext.Status.Conditions, "installation failed", ext.GetGeneration())
 		return ctrl.Result{}, err
 	}
 
@@ -199,14 +213,16 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext
 	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(app.UnstructuredContent(), existingTypedApp); err != nil {
 		// originally Reason: ocv1alpha1.ReasonInstallationStatusUnknown
 		ext.Status.InstalledBundle = nil
-		setInstalledStatusConditionUnknown(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
-		setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
+		setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
+		setProgressingStatusConditionProgressing(&ext.Status.Conditions, "installation failed", ext.GetGeneration())
 		return ctrl.Result{}, err
 	}
 
-	version, _ := bundle.Version()
-	MapAppStatusToCondition(existingTypedApp, ext, &ocv1alpha1.BundleMetadata{Name: bundle.Name, Version: fmt.Sprintf("%v", version)})
-	SetDeprecationStatusInExtension(ext, bundle)
+	ext.Status.InstalledBundle = bundleMetadataFor(bundle)
+	setInstalledStatusConditionSuccess(&ext.Status.Conditions, fmt.Sprintf("successfully installed %v", ext.Status.InstalledBundle), ext.GetGeneration())
+
+	// TODO: add conditions to determine extension health
+	mapAppStatusToCondition(existingTypedApp, ext)
 
 	return ctrl.Result{}, nil
 }
@@ -230,63 +246,36 @@ func (r *ExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 // mapAppStatusToCondition maps the reconciling/deleting App conditions to the installed/deleting conditions on the Extension.
-func MapAppStatusToCondition(existingApp *kappctrlv1alpha1.App, ext *ocv1alpha1.Extension, bundle *ocv1alpha1.BundleMetadata) {
-	if ext == nil {
-		return
-	}
-	message := "install status unknown"
-	ext.Status.InstalledBundle = nil
-
-	if existingApp == nil {
-		setInstalledStatusConditionUnknown(&ext.Status.Conditions, message, ext.Generation)
+func mapAppStatusToCondition(existingApp *kappctrlv1alpha1.App, ext *ocv1alpha1.Extension) {
+	// Note: App.Status.Inspect errors are never surfaced to App conditions, so are currently ignored when determining App status.
+	if ext == nil || existingApp == nil {
 		return
 	}
-	message = existingApp.Status.FriendlyDescription
-	if strings.Contains(message, "Error (see .status.usefulErrorMessage for details)") || len(message) == 0 {
+	message := existingApp.Status.FriendlyDescription
+	if len(message) == 0 || strings.Contains(message, "Error (see .status.usefulErrorMessage for details)") {
 		message = existingApp.Status.UsefulErrorMessage
 	}
 
-	orderedAppStatuses := []kappctrlv1alpha1.ConditionType{
-		kappctrlv1alpha1.DeleteFailed,
-		kappctrlv1alpha1.Deleting,
-		kappctrlv1alpha1.ReconcileSucceeded,
-		kappctrlv1alpha1.ReconcileFailed,
-		kappctrlv1alpha1.Reconciling,
-	}
 	appStatusMapFn := map[kappctrlv1alpha1.ConditionType]func(*[]metav1.Condition, string, int64){
-		kappctrlv1alpha1.DeleteFailed:       setInstalledStatusConditionDeleteFailed,
-		kappctrlv1alpha1.Deleting:           setInstalledStatusConditionDeleting,
-		kappctrlv1alpha1.ReconcileSucceeded: setInstalledStatusConditionSuccess,
-		kappctrlv1alpha1.ReconcileFailed:    setInstalledStatusConditionFailed,
-		kappctrlv1alpha1.Reconciling:        setInstalledStatusConditionUnknown,
+		kappctrlv1alpha1.Deleting:           setProgressingStatusConditionProgressing,
+		kappctrlv1alpha1.Reconciling:        setProgressingStatusConditionProgressing,
+		kappctrlv1alpha1.DeleteFailed:       setProgressingStatusConditionFailed,
+		kappctrlv1alpha1.ReconcileFailed:    setProgressingStatusConditionFailed,
+		kappctrlv1alpha1.ReconcileSucceeded: setProgressingStatusConditionSuccess,
 	}
-	for _, cond := range orderedAppStatuses {
+	for cond := range appStatusMapFn {
 		if c := findStatusCondition(existingApp.Status.GenericStatus.Conditions, cond); c != nil && c.Status == corev1.ConditionTrue {
 			if len(message) == 0 {
 				message = c.Message
 			}
-			if c.Type == kappctrlv1alpha1.ReconcileSucceeded {
-				ext.Status.InstalledBundle = bundle
-			}
-			appStatusMapFn[cond](&ext.Status.Conditions, message, ext.Generation)
+			appStatusMapFn[cond](&ext.Status.Conditions, fmt.Sprintf("App %s: %s", c.Type, message), ext.Generation)
 			return
 		}
 	}
 	if len(message) == 0 {
-		message = "install status unknown"
-	}
-	setInstalledStatusConditionUnknown(&ext.Status.Conditions, message, ext.Generation)
-}
-
-// findStatusCondition finds the conditionType in conditions.
-// TODO: suggest using upstream conditions to Carvel.
-func findStatusCondition(conditions []kappctrlv1alpha1.Condition, conditionType kappctrlv1alpha1.ConditionType) *kappctrlv1alpha1.Condition {
-	for i := range conditions {
-		if conditions[i].Type == conditionType {
-			return &conditions[i]
-		}
+		message = "waiting for app"
 	}
-	return nil
+	setProgressingStatusConditionProgressing(&ext.Status.Conditions, message, ext.Generation)
 }
 
 // setDeprecationStatus will set the appropriate deprecation statuses for a Extension
@@ -371,6 +360,17 @@ func SetDeprecationStatusInExtension(ext *ocv1alpha1.Extension, bundle *catalogm
 	}
 }
 
+// findStatusCondition finds the conditionType in conditions.
+// TODO: suggest using upstream conditions to Carvel.
+func findStatusCondition(conditions []kappctrlv1alpha1.Condition, conditionType kappctrlv1alpha1.ConditionType) *kappctrlv1alpha1.Condition {
+	for i := range conditions {
+		if conditions[i].Type == conditionType {
+			return &conditions[i]
+		}
+	}
+	return nil
+}
+
 func (r *ExtensionReconciler) ensureApp(ctx context.Context, desiredApp *unstructured.Unstructured) error {
 	existingApp, err := r.existingAppUnstructured(ctx, desiredApp.GetName(), desiredApp.GetNamespace())
 	if client.IgnoreNotFound(err) != nil {