From be9f4a31ad30779f3bd451120103622095d5f943 Mon Sep 17 00:00:00 2001 From: Ankita Thomas Date: Thu, 7 Mar 2024 15:52:11 -0500 Subject: [PATCH 1/3] map kapp conditions to Extension Signed-off-by: Ankita Thomas --- api/v1alpha1/clusterextension_types.go | 4 + internal/controllers/common_controller.go | 24 +- internal/controllers/extension_controller.go | 82 +++-- .../controllers/extension_controller_test.go | 314 ++++++++++++++++++ 4 files changed, 395 insertions(+), 29 deletions(-) diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index 0d7f0b83d..72c4f4ec0 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 fd721f469..6306344d2 100644 --- a/internal/controllers/common_controller.go +++ b/internal/controllers/common_controller.go @@ -98,7 +98,29 @@ func setInstalledStatusConditionFailed(conditions *[]metav1.Condition, message s }) } -// setDeprecationStatusesUnknown sets the deprecation status conditions to unknown. +// 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{ ocv1alpha1.TypeDeprecated, diff --git a/internal/controllers/extension_controller.go b/internal/controllers/extension_controller.go index 53bc0c14b..be2876911 100644 --- a/internal/controllers/extension_controller.go +++ b/internal/controllers/extension_controller.go @@ -204,7 +204,8 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext return ctrl.Result{}, err } - mapAppStatusToInstalledCondition(existingTypedApp, ext, bundle) + version, _ := bundle.Version() + MapAppStatusToCondition(existingTypedApp, ext, &ocv1alpha1.BundleMetadata{Name: bundle.Name, Version: fmt.Sprintf("%v", version)}) SetDeprecationStatusInExtension(ext, bundle) return ctrl.Result{}, nil @@ -228,28 +229,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, 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, bundle *ocv1alpha1.BundleMetadata) { + if ext == nil { return } + message := "install status unknown" + ext.Status.InstalledBundle = nil - if appReady.Status != corev1.ConditionTrue { - ext.Status.InstalledBundle = nil - 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.InstalledBundle = bundle + } + appStatusMapFn[cond](&ext.Status.Conditions, message, ext.Generation) + return + } + } + if len(message) == 0 { + message = "install status unknown" + } + setInstalledStatusConditionUnknown(&ext.Status.Conditions, message, ext.Generation) +} - ext.Status.InstalledBundle = bundleMetadataFor(bundle) - 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 @@ -334,17 +371,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 9c72e0a47..0dcc32b25 100644 --- a/internal/controllers/extension_controller_test.go +++ b/internal/controllers/extension_controller_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" carvelv1alpha1 "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" @@ -15,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" @@ -238,3 +241,314 @@ 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 *carvelv1alpha1.App + ext *ocv1alpha1.Extension + bundleImage string + expected *ocv1alpha1.Extension + }{ + { + name: "preserve existing conditions on extension while reconciling", + app: &carvelv1alpha1.App{ + Status: carvelv1alpha1.AppStatus{ + GenericStatus: carvelv1alpha1.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: &carvelv1alpha1.App{ + Status: carvelv1alpha1.AppStatus{ + GenericStatus: carvelv1alpha1.GenericStatus{ + Conditions: []carvelv1alpha1.Condition{{ + Type: carvelv1alpha1.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{ + InstalledBundle: &ocv1alpha1.BundleMetadata{ + Name: "test-bundle", + Version: "0.0.1", + }, + 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: &carvelv1alpha1.App{ + Status: carvelv1alpha1.AppStatus{ + GenericStatus: carvelv1alpha1.GenericStatus{ + Conditions: []carvelv1alpha1.Condition{{ + Type: carvelv1alpha1.Reconciling, + Status: corev1.ConditionTrue, + Message: "Reconciling", + }}, + FriendlyDescription: "Reconciling", + }, + }, + }, + ext: &ocv1alpha1.Extension{ + Status: ocv1alpha1.ExtensionStatus{ + InstalledBundle: &ocv1alpha1.BundleMetadata{ + Name: "test-bundle", + Version: "0.0.1", + }, + 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: &carvelv1alpha1.App{ + Status: carvelv1alpha1.AppStatus{ + GenericStatus: carvelv1alpha1.GenericStatus{ + Conditions: []carvelv1alpha1.Condition{{ + Type: carvelv1alpha1.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{ + InstalledBundle: &ocv1alpha1.BundleMetadata{ + Name: "test-bundle", + Version: "0.0.1", + }, + 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: &carvelv1alpha1.App{ + Status: carvelv1alpha1.AppStatus{ + GenericStatus: carvelv1alpha1.GenericStatus{ + Conditions: []carvelv1alpha1.Condition{{ + Type: carvelv1alpha1.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: &carvelv1alpha1.App{ + Status: carvelv1alpha1.AppStatus{ + GenericStatus: carvelv1alpha1.GenericStatus{ + Conditions: []carvelv1alpha1.Condition{{ + Type: carvelv1alpha1.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: &carvelv1alpha1.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) + } +} From ce3a00591afe1a14313cb22b848fcea43d6d773a Mon Sep 17 00:00:00 2001 From: Ankita Thomas Date: Mon, 25 Mar 2024 10:20:40 -0400 Subject: [PATCH 2/3] add Progressing condition Signed-off-by: Ankita Thomas --- api/v1alpha1/clusterextension_types.go | 4 - api/v1alpha1/clusterextension_types_test.go | 14 +- api/v1alpha1/extension_types.go | 20 ++ internal/conditionsets/conditionsets.go | 4 + internal/controllers/common_controller.go | 68 ++-- internal/controllers/extension_controller.go | 122 +++---- .../controllers/extension_controller_test.go | 314 ------------------ 7 files changed, 131 insertions(+), 415 deletions(-) diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index 72c4f4ec0..0d7f0b83d 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/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, ) } 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..de91d8d63 100644 --- a/api/v1alpha1/extension_types.go +++ b/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, + } } 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 6306344d2..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{ @@ -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, + }) +} diff --git a/internal/controllers/extension_controller.go b/internal/controllers/extension_controller.go index be2876911..4b36a2a59 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()) @@ -162,28 +164,39 @@ 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 } @@ -191,6 +204,7 @@ 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()) + 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 { diff --git a/internal/controllers/extension_controller_test.go b/internal/controllers/extension_controller_test.go index 0dcc32b25..9c72e0a47 100644 --- a/internal/controllers/extension_controller_test.go +++ b/internal/controllers/extension_controller_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" carvelv1alpha1 "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" @@ -16,8 +15,6 @@ 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" @@ -241,314 +238,3 @@ 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 *carvelv1alpha1.App - ext *ocv1alpha1.Extension - bundleImage string - expected *ocv1alpha1.Extension - }{ - { - name: "preserve existing conditions on extension while reconciling", - app: &carvelv1alpha1.App{ - Status: carvelv1alpha1.AppStatus{ - GenericStatus: carvelv1alpha1.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: &carvelv1alpha1.App{ - Status: carvelv1alpha1.AppStatus{ - GenericStatus: carvelv1alpha1.GenericStatus{ - Conditions: []carvelv1alpha1.Condition{{ - Type: carvelv1alpha1.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{ - InstalledBundle: &ocv1alpha1.BundleMetadata{ - Name: "test-bundle", - Version: "0.0.1", - }, - 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: &carvelv1alpha1.App{ - Status: carvelv1alpha1.AppStatus{ - GenericStatus: carvelv1alpha1.GenericStatus{ - Conditions: []carvelv1alpha1.Condition{{ - Type: carvelv1alpha1.Reconciling, - Status: corev1.ConditionTrue, - Message: "Reconciling", - }}, - FriendlyDescription: "Reconciling", - }, - }, - }, - ext: &ocv1alpha1.Extension{ - Status: ocv1alpha1.ExtensionStatus{ - InstalledBundle: &ocv1alpha1.BundleMetadata{ - Name: "test-bundle", - Version: "0.0.1", - }, - 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: &carvelv1alpha1.App{ - Status: carvelv1alpha1.AppStatus{ - GenericStatus: carvelv1alpha1.GenericStatus{ - Conditions: []carvelv1alpha1.Condition{{ - Type: carvelv1alpha1.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{ - InstalledBundle: &ocv1alpha1.BundleMetadata{ - Name: "test-bundle", - Version: "0.0.1", - }, - 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: &carvelv1alpha1.App{ - Status: carvelv1alpha1.AppStatus{ - GenericStatus: carvelv1alpha1.GenericStatus{ - Conditions: []carvelv1alpha1.Condition{{ - Type: carvelv1alpha1.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: &carvelv1alpha1.App{ - Status: carvelv1alpha1.AppStatus{ - GenericStatus: carvelv1alpha1.GenericStatus{ - Conditions: []carvelv1alpha1.Condition{{ - Type: carvelv1alpha1.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: &carvelv1alpha1.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) - } -} From 27a6d327769fb492c26093d2c188efd607807903 Mon Sep 17 00:00:00 2001 From: Ankita Thomas Date: Fri, 12 Apr 2024 16:30:07 -0400 Subject: [PATCH 3/3] remove changes to deprecation statuses Signed-off-by: Ankita Thomas --- api/v1alpha1/extension_types.go | 3 ++- internal/controllers/extension_controller.go | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/extension_types.go b/api/v1alpha1/extension_types.go index de91d8d63..6109c2cc8 100644 --- a/api/v1alpha1/extension_types.go +++ b/api/v1alpha1/extension_types.go @@ -17,8 +17,9 @@ limitations under the License. package v1alpha1 import ( - "github.com/operator-framework/operator-controller/internal/conditionsets" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/operator-framework/operator-controller/internal/conditionsets" ) const ( diff --git a/internal/controllers/extension_controller.go b/internal/controllers/extension_controller.go index 4b36a2a59..57226e101 100644 --- a/internal/controllers/extension_controller.go +++ b/internal/controllers/extension_controller.go @@ -164,14 +164,12 @@ 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 { 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 @@ -185,6 +183,7 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext // 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 @@ -195,6 +194,7 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext 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 @@ -204,6 +204,7 @@ 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 } @@ -214,12 +215,14 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext // originally Reason: ocv1alpha1.ReasonInstallationStatusUnknown 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 } 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)