diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index b763cb60a..5af25e0b9 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/api/v1alpha1/clusterextension_types.go @@ -93,6 +93,8 @@ const ( ReasonResolutionUnknown = "ResolutionUnknown" ReasonSuccess = "Success" ReasonDeprecated = "Deprecated" + ReasonDeleteFailed = "DeleteFailed" + ReasonDeleting = "Deleting" ) func init() { @@ -116,6 +118,8 @@ func init() { ReasonInvalidSpec, ReasonSuccess, ReasonDeprecated, + ReasonDeleteFailed, + ReasonDeleting, ) } diff --git a/internal/controllers/common_controller.go b/internal/controllers/common_controller.go index 98067824d..2b2fcc743 100644 --- a/internal/controllers/common_controller.go +++ b/internal/controllers/common_controller.go @@ -99,6 +99,28 @@ 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. func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message string, generation int64) { conditionTypes := []string{ diff --git a/internal/controllers/extension_controller.go b/internal/controllers/extension_controller.go index 04310164c..815ee2d49 100644 --- a/internal/controllers/extension_controller.go +++ b/internal/controllers/extension_controller.go @@ -207,7 +207,7 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext return ctrl.Result{}, err } - mapAppStatusToInstalledCondition(existingTypedApp, ext, bundle.Image) + MapAppStatusToCondition(existingTypedApp, ext, bundle.Image) SetDeprecationStatusInExtension(ext, bundle) return ctrl.Result{}, nil @@ -231,30 +231,64 @@ 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, bundleImage string) { - appReady := findStatusCondition(existingApp.Status.GenericStatus.Conditions, kappctrlv1alpha1.ReconcileSucceeded) - if appReady == nil { - ext.Status.InstalledBundleResource = "" - 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, bundleImage string) { + if ext == nil { return } + message := "install status unknown" + ext.Status.InstalledBundleResource = "" - if appReady.Status != corev1.ConditionTrue { - ext.Status.InstalledBundleResource = "" - setInstalledStatusConditionFailed( - &ext.Status.Conditions, - appReady.Message, - ext.GetGeneration(), - ) + if existingApp == nil { + setInstalledStatusConditionUnknown(&ext.Status.Conditions, message, ext.Generation) return } + message = existingApp.Status.FriendlyDescription + if strings.Contains(message, "Error (see .status.usefulErrorMessage for details)") || len(message) == 0 { + 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, + } + for _, cond := range orderedAppStatuses { + 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.InstalledBundleResource = bundleImage + } + appStatusMapFn[cond](&ext.Status.Conditions, message, ext.Generation) + return + } + } + if len(message) == 0 { + message = "install status unknown" + } + setInstalledStatusConditionUnknown(&ext.Status.Conditions, message, ext.Generation) +} - // InstalledBundleResource this should be converted into a slice as App allows fetching - // from multiple sources. - ext.Status.InstalledBundleResource = bundleImage - setInstalledStatusConditionSuccess(&ext.Status.Conditions, appReady.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] + } + } + return nil } // setDeprecationStatus will set the appropriate deprecation statuses for a Extension @@ -339,17 +373,6 @@ 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 { diff --git a/internal/controllers/extension_controller_test.go b/internal/controllers/extension_controller_test.go index 0d59995a9..a23c82dd9 100644 --- a/internal/controllers/extension_controller_test.go +++ b/internal/controllers/extension_controller_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + kappctrlv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" + corev1 "k8s.io/api/core/v1" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -14,6 +16,8 @@ import ( featuregatetesting "k8s.io/component-base/featuregate/testing" ctrl "sigs.k8s.io/controller-runtime" + "github.com/operator-framework/operator-controller/internal/controllers" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/conditionsets" "github.com/operator-framework/operator-controller/pkg/features" @@ -110,3 +114,305 @@ func verifyExtensionConditionsInvariants(t *testing.T, ext *ocv1alpha1.Extension require.Equal(t, ext.GetGeneration(), cond.ObservedGeneration) } } + +func TestMapAppCondtitionToStatus(t *testing.T) { + testCases := []struct { + name string + app *kappctrlv1alpha1.App + ext *ocv1alpha1.Extension + bundleImage string + expected *ocv1alpha1.Extension + }{ + { + name: "preserve existing conditions on extension while reconciling", + app: &kappctrlv1alpha1.App{ + Status: kappctrlv1alpha1.AppStatus{ + GenericStatus: kappctrlv1alpha1.GenericStatus{ + // ObservedGeneration: 0, + FriendlyDescription: "Paused/Cancelled", + }, + }, + }, + ext: &ocv1alpha1.Extension{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeDeprecated, + Reason: ocv1alpha1.ReasonDeprecated, + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + }, + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonInstallationStatusUnknown, + Message: "install status unknown", + ObservedGeneration: 1, + }, + }, + }, + }, + expected: &ocv1alpha1.Extension{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeDeprecated, + Reason: ocv1alpha1.ReasonDeprecated, + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + }, + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonInstallationStatusUnknown, + Message: "Paused/Cancelled", + ObservedGeneration: 1, + }, + }, + }, + }, + }, + + { + name: "update installedBundleResource on successful reconcile", + bundleImage: "test-bundle", + app: &kappctrlv1alpha1.App{ + Status: kappctrlv1alpha1.AppStatus{ + GenericStatus: kappctrlv1alpha1.GenericStatus{ + Conditions: []kappctrlv1alpha1.Condition{{ + Type: kappctrlv1alpha1.ReconcileSucceeded, + Status: corev1.ConditionTrue, + // Reason: "", + Message: "Reconcile Succeeded", + }}, + }, + }, + }, + ext: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonInstallationStatusUnknown, + }, + }, + }, + }, + expected: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + InstalledBundleResource: "test-bundle", + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionTrue, + Reason: ocv1alpha1.ReasonSuccess, + Message: "Reconcile Succeeded", + }, + }, + }, + }, + }, + { + name: "remove installedBundleResource when not successful reconcile", + bundleImage: "test-bundle", + app: &kappctrlv1alpha1.App{ + Status: kappctrlv1alpha1.AppStatus{ + GenericStatus: kappctrlv1alpha1.GenericStatus{ + Conditions: []kappctrlv1alpha1.Condition{{ + Type: kappctrlv1alpha1.Reconciling, + Status: corev1.ConditionTrue, + Message: "Reconciling", + }}, + FriendlyDescription: "Reconciling", + }, + }, + }, + ext: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + InstalledBundleResource: "test-bundle", + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionTrue, + Reason: ocv1alpha1.ReasonSuccess, + Message: "Success", + }, + }, + }, + }, + expected: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonInstallationStatusUnknown, + Message: "Reconciling", + }, + }, + }, + }, + }, + { + name: "show UsefulErrorMessage when referenced by FriendlyErrorMessage", + app: &kappctrlv1alpha1.App{ + Status: kappctrlv1alpha1.AppStatus{ + GenericStatus: kappctrlv1alpha1.GenericStatus{ + Conditions: []kappctrlv1alpha1.Condition{{ + Type: kappctrlv1alpha1.ReconcileFailed, + Status: corev1.ConditionTrue, + Message: "Reconcile Failed", + }}, + FriendlyDescription: "Reconcile Error (see .status.usefulErrorMessage for details)", + UsefulErrorMessage: "Deployment Error: Exit Status 1", + }, + }, + }, + ext: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + InstalledBundleResource: "test-bundle", + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionTrue, + Reason: ocv1alpha1.ReasonSuccess, + Message: "Success", + }, + }, + }, + }, + expected: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionFalse, + Reason: ocv1alpha1.ReasonInstallationFailed, + Message: "Deployment Error: Exit Status 1", + }, + }, + }, + }, + }, + { + name: "show FriendlyErrorMessage when present", + app: &kappctrlv1alpha1.App{ + Status: kappctrlv1alpha1.AppStatus{ + GenericStatus: kappctrlv1alpha1.GenericStatus{ + Conditions: []kappctrlv1alpha1.Condition{{ + Type: kappctrlv1alpha1.DeleteFailed, + Status: corev1.ConditionTrue, + Message: "Delete Failed", + }}, + FriendlyDescription: "Delete Error: Timed out", + UsefulErrorMessage: "Timed out after 5m", + }, + }, + }, + ext: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonDeleting, + Message: "Deleting", + }, + }, + }, + }, + expected: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonDeleteFailed, + Message: "Delete Error: Timed out", + }, + }, + }, + }, + }, + { + name: "update status without message when none exist on App", + app: &kappctrlv1alpha1.App{ + Status: kappctrlv1alpha1.AppStatus{ + GenericStatus: kappctrlv1alpha1.GenericStatus{ + Conditions: []kappctrlv1alpha1.Condition{{ + Type: kappctrlv1alpha1.Deleting, + Status: corev1.ConditionTrue, + }}, + }, + }, + }, + ext: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonInstallationStatusUnknown, + Message: "Reconciling", + }, + }, + }, + }, + expected: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonDeleting, + }, + }, + }, + }, + }, + { + name: "set default installed condition for empty app status", + app: &kappctrlv1alpha1.App{}, + ext: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonInstallationStatusUnknown, + Message: "Reconciling", + }, + }, + }, + }, + expected: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + Conditions: []metav1.Condition{ + { + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonInstallationStatusUnknown, + Message: "install status unknown", + }, + }, + }, + }, + }, + } + + for _, tt := range testCases { + controllers.MapAppStatusToCondition(tt.app, tt.ext, tt.bundleImage) + for i := range tt.ext.Status.Conditions { + //unset transition time for comparison + tt.ext.Status.Conditions[i].LastTransitionTime = metav1.Time{} + } + assert.Equal(t, tt.expected, tt.ext, tt.name) + } +}