Skip to content

Commit

Permalink
add Progressing condition
Browse files Browse the repository at this point in the history 
Signed-off-by: Ankita Thomas <ankithom@redhat.com>
  • Loading branch information
@ankitathomas
ankitathomas committed Apr 10, 2024
1 parent be9f4a3 commit 781f915
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 413 deletions.
20 changes: 15 additions & 5 deletions api/v1alpha1/clusterextension_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,16 @@ type ClusterExtensionSpec struct {

const (
// TODO(user): add more Types, here and into init()
// TypeInstalled indicates whether the install for the bundle
// referenced in the status was completed.
// It does not indicate whether the App created by the bundle is healthy.
TypeInstalled = "Installed"
TypeResolved = "Resolved"
// TypeProgressing indicates whether operator-controller is
// reconciling, installing, updating or deleting an extension.
TypeProgressing = "Progressing"
// TypeDeprecated is a rollup condition that is present when
// any of the deprecated conditions are present.
// any of the deprecated conditions are present for the resolved bundle.
TypeDeprecated = "Deprecated"
TypePackageDeprecated = "PackageDeprecated"
TypeChannelDeprecated = "ChannelDeprecated"
Expand All @@ -93,8 +99,8 @@ const (
ReasonResolutionUnknown = "ResolutionUnknown"
ReasonSuccess = "Success"
ReasonDeprecated = "Deprecated"
ReasonDeleteFailed = "DeleteFailed"
ReasonDeleting = "Deleting"
ReasonProgressing = "Progressing"
ReasonFailed = "Failed"
)

func init() {
Expand All @@ -118,9 +124,13 @@ func init() {
ReasonInvalidSpec,
ReasonSuccess,
ReasonDeprecated,
ReasonDeleteFailed,
ReasonDeleting,
ReasonProgressing,
ReasonFailed,
)

conditionsets.ExtensionConditionTypes = []string{
TypeProgressing,
}
}

// ClusterExtensionStatus defines the observed state of ClusterExtension
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/clusterextension_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
3 changes: 3 additions & 0 deletions internal/conditionsets/conditionsets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ package conditionsets
// NOTE: These are populated by init() in api/v1alpha1/clusterextension_types.go
var ConditionTypes []string
var ConditionReasons []string

// This is the set of Extension exclusive condition Types.

Check failure on line 26 in internal/conditionsets/conditionsets.go

View workflow job for this annotation

GitHub Actions / lint 

ST1022: comment on exported var ExtensionConditionTypes should be of the form "ExtensionConditionTypes ..." (stylecheck)
var ExtensionConditionTypes []string
68 changes: 34 additions & 34 deletions internal/controllers/common_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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,
Expand All @@ -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.ReasonSuccess,
Message: message,
ObservedGeneration: generation,
})
}

// setProgressingStatusConditionFailed sets the progressing status condition to true with an error message.
func setProgressingStatusConditionFailed(conditions *[]metav1.Condition, message string, generation int64) {
apimeta.SetStatusCondition(conditions, metav1.Condition{
Type: ocv1alpha1.TypeProgressing,
Status: metav1.ConditionTrue,
Reason: ocv1alpha1.ReasonFailed,
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,
})
}
115 changes: 56 additions & 59 deletions internal/controllers/extension_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())
Expand All @@ -162,21 +164,29 @@ 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
}

Expand All @@ -191,6 +201,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())
setProgressingStatusConditionFailed(&ext.Status.Conditions, "installation failed", ext.GetGeneration())
return ctrl.Result{}, err
}

Expand All @@ -199,14 +210,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())
setProgressingStatusConditionFailed(&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
}
Expand All @@ -230,63 +243,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 {
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 := "install status unknown"
ext.Status.InstalledBundle = nil

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.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"
message = "waiting for app"
}
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]
}
}
return nil
setProgressingStatusConditionProgressing(&ext.Status.Conditions, message, ext.Generation)
}

// setDeprecationStatus will set the appropriate deprecation statuses for a Extension
Expand Down Expand Up @@ -371,6 +357,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 {
Expand Down
Loading

0 comments on commit 781f915

Please sign in to comment.