diff --git a/api/v1alpha1/clusterextension_types_test.go b/api/v1alpha1/clusterextension_types_test.go index 0ed4f1a08..520c8464f 100644 --- a/api/v1alpha1/clusterextension_types_test.go +++ b/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 diff --git a/api/v1alpha1/extension_types.go b/api/v1alpha1/extension_types.go index a6b73112c..6109c2cc8 100644 --- a/api/v1alpha1/extension_types.go +++ b/api/v1alpha1/extension_types.go @@ -18,6 +18,8 @@ package v1alpha1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/operator-framework/operator-controller/internal/conditionsets" ) const ( @@ -133,6 +135,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, + } } diff --git a/internal/conditionsets/conditionsets.go b/internal/conditionsets/conditionsets.go index fa4087148..6982ad30c 100644 --- a/internal/conditionsets/conditionsets.go +++ b/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 diff --git a/internal/controllers/common_controller.go b/internal/controllers/common_controller.go index fd721f469..377ad3040 100644 --- a/internal/controllers/common_controller.go +++ b/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{ @@ -117,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, + }) +} diff --git a/internal/controllers/extension_controller.go b/internal/controllers/extension_controller.go index 0d7a9a74c..5c69eb5cf 100644 --- a/internal/controllers/extension_controller.go +++ b/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()) @@ -164,26 +166,37 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext 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()) + setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", 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()) + setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", 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()) + setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) + } + setProgressingStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) return ctrl.Result{}, err } @@ -191,6 +204,8 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext // originally Reason: ocv1alpha1.ReasonInstallationFailed ext.Status.InstalledBundle = 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()) + setProgressingStatusConditionProgressing(&ext.Status.Conditions, "installation failed", ext.GetGeneration()) return ctrl.Result{}, err } @@ -199,14 +214,19 @@ 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()) + setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) + setProgressingStatusConditionProgressing(&ext.Status.Conditions, "installation failed", ext.GetGeneration()) return ctrl.Result{}, err } - mapAppStatusToInstalledCondition(existingTypedApp, ext, bundle) + ext.Status.InstalledBundle = bundleMetadataFor(bundle) + setInstalledStatusConditionSuccess(&ext.Status.Conditions, fmt.Sprintf("successfully installed %v", ext.Status.InstalledBundle), ext.GetGeneration()) SetDeprecationStatusInExtension(ext, bundle) + // TODO: add conditions to determine extension health + mapAppStatusToCondition(existingTypedApp, ext) + return ctrl.Result{}, nil } @@ -228,28 +248,37 @@ func (r *ExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// TODO: follow up with mapping of all the available App statuses: https://github.com/carvel-dev/kapp-controller/blob/855063edee53315811a13ee8d5df1431ba258ede/pkg/apis/kappctrl/v1alpha1/status.go#L28-L35 -// mapAppStatusToInstalledCondition currently maps only the installed condition. -func mapAppStatusToInstalledCondition(existingApp *kappctrlv1alpha1.App, ext *ocv1alpha1.Extension, bundle *catalogmetadata.Bundle) { - appReady := findStatusCondition(existingApp.Status.GenericStatus.Conditions, kappctrlv1alpha1.ReconcileSucceeded) - if appReady == nil { - ext.Status.InstalledBundle = nil - setInstalledStatusConditionUnknown(&ext.Status.Conditions, "install status unknown", ext.Generation) +// mapAppStatusToCondition maps the reconciling/deleting App conditions to the installed/deleting conditions on the Extension. +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 } - - if appReady.Status != corev1.ConditionTrue { - ext.Status.InstalledBundle = nil - setInstalledStatusConditionFailed( - &ext.Status.Conditions, - appReady.Message, - ext.GetGeneration(), - ) - return + message := existingApp.Status.FriendlyDescription + if len(message) == 0 || strings.Contains(message, "Error (see .status.usefulErrorMessage for details)") { + message = existingApp.Status.UsefulErrorMessage } - ext.Status.InstalledBundle = bundleMetadataFor(bundle) - setInstalledStatusConditionSuccess(&ext.Status.Conditions, appReady.Message, ext.Generation) + appStatusMapFn := map[kappctrlv1alpha1.ConditionType]func(*[]metav1.Condition, string, int64){ + kappctrlv1alpha1.Deleting: setProgressingStatusConditionProgressing, + kappctrlv1alpha1.Reconciling: setProgressingStatusConditionProgressing, + kappctrlv1alpha1.DeleteFailed: setProgressingStatusConditionFailed, + kappctrlv1alpha1.ReconcileFailed: setProgressingStatusConditionFailed, + kappctrlv1alpha1.ReconcileSucceeded: setProgressingStatusConditionSuccess, + } + 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 + } + appStatusMapFn[cond](&ext.Status.Conditions, fmt.Sprintf("App %s: %s", c.Type, message), ext.Generation) + return + } + } + if len(message) == 0 { + message = "waiting for app" + } + setProgressingStatusConditionProgressing(&ext.Status.Conditions, message, ext.Generation) } // setDeprecationStatus will set the appropriate deprecation statuses for a Extension