Skip to content

✨ add progressing condition #1302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions api/v1alpha1/clusterextension_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import (
"github.com/operator-framework/operator-controller/internal/conditionsets"
)

var (
ClusterExtensionKind = "ClusterExtension"
)
var ClusterExtensionKind = "ClusterExtension"

type UpgradeConstraintPolicy string
type CRDUpgradeSafetyPolicy string
type (
UpgradeConstraintPolicy string
CRDUpgradeSafetyPolicy string
)

const (
// The extension will only upgrade if the new version satisfies
Expand Down Expand Up @@ -412,8 +412,9 @@ type CRDUpgradeSafetyPreflightConfig struct {

const (
// TODO(user): add more Types, here and into init()
TypeInstalled = "Installed"
TypeResolved = "Resolved"
TypeInstalled = "Installed"
TypeResolved = "Resolved"
TypeProgressing = "Progressing"

// TypeDeprecated is a rollup condition that is present when
// any of the deprecated conditions are present.
Expand All @@ -426,12 +427,12 @@ const (
ReasonSuccess = "Succeeded"
ReasonDeprecated = "Deprecated"
ReasonFailed = "Failed"
ReasonBlocked = "Blocked"
ReasonRetrying = "Retrying"

ReasonErrorGettingClient = "ErrorGettingClient"
ReasonErrorGettingReleaseState = "ErrorGettingReleaseState"

ReasonUnverifiable = "Unverifiable"

CRDUpgradeSafetyPolicyEnabled CRDUpgradeSafetyPolicy = "Enabled"
CRDUpgradeSafetyPolicyDisabled CRDUpgradeSafetyPolicy = "Disabled"
)
Expand All @@ -446,6 +447,7 @@ func init() {
TypeChannelDeprecated,
TypeBundleDeprecated,
TypeUnpacked,
TypeProgressing,
)
// TODO(user): add Reasons from above
conditionsets.ConditionReasons = append(conditionsets.ConditionReasons,
Expand All @@ -454,7 +456,8 @@ func init() {
ReasonFailed,
ReasonErrorGettingClient,
ReasonErrorGettingReleaseState,
ReasonUnverifiable,
ReasonBlocked,
ReasonRetrying,
)
}

Expand Down
35 changes: 26 additions & 9 deletions internal/controllers/clusterextension_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
setInstallStatus(ext, nil)
setResolutionStatus(ext, nil)
setResolvedStatusConditionFailed(ext, err.Error())
setStatusProgressing(ext, err)
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
return ctrl.Result{}, err
}
Expand All @@ -216,6 +217,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
setInstallStatus(ext, nil)
// TODO: use Installed=Unknown
setInstalledStatusConditionFailed(ext, err.Error())
setStatusProgressing(ext, err)
return ctrl.Result{}, err
}

Expand All @@ -227,6 +229,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
setInstallStatus(ext, nil)
setResolutionStatus(ext, nil)
setResolvedStatusConditionFailed(ext, err.Error())
setStatusProgressing(ext, err)
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
return ctrl.Result{}, err
}
Expand All @@ -247,8 +250,9 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
// all catalogs?
SetDeprecationStatus(ext, resolvedBundle.Name, resolvedDeprecation)

resolvedBundleMetadata := bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion)
resStatus := &ocv1alpha1.ClusterExtensionResolutionStatus{
Bundle: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion),
Bundle: resolvedBundleMetadata,
}
setResolutionStatus(ext, resStatus)
setResolvedStatusConditionSuccess(ext, fmt.Sprintf("resolved to %q", resolvedBundle.Image))
Expand All @@ -264,20 +268,18 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
unpackResult, err := r.Unpacker.Unpack(ctx, bundleSource)
if err != nil {
setStatusUnpackFailed(ext, err.Error())
// Wrap the error passed to this with the resolution information until we have successfully
// installed since we intend for the progressing condition to replace the resolved condition
// and will be removing the .status.resolution field from the ClusterExtension status API
setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
return ctrl.Result{}, err
}

switch unpackResult.State {
case rukpaksource.StatePending:
setStatusUnpackFailed(ext, unpackResult.Message)
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, "unpack pending")
return ctrl.Result{}, nil
case rukpaksource.StateUnpacked:
setStatusUnpacked(ext, unpackResult.Message)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we setStatusProgressing here ? or just remove this completely (probably in the removed unpacked status pr) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would anticipate that we remove it completely in the unpacked status PR. There isn't really any value to setting it here since if there are no errors throughout reconciliation we set it to False, Succeeded

default:
setStatusUnpackFailed(ext, "unexpected unpack status")
// We previously exit with a failed status if error is not nil.
return ctrl.Result{}, fmt.Errorf("unexpected unpack status: %v", unpackResult.Message)
panic(fmt.Sprintf("unexpected unpack state %q", unpackResult.State))
}

objLbls := map[string]string{
Expand All @@ -304,25 +306,36 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
managedObjs, _, err := r.Applier.Apply(ctx, unpackResult.Bundle, ext, objLbls, storeLbls)
if err != nil {
setInstalledStatusConditionFailed(ext, err.Error())
setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
return ctrl.Result{}, err
}

installStatus := &ocv1alpha1.ClusterExtensionInstallStatus{
Bundle: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion),
Bundle: resolvedBundleMetadata,
}
setInstallStatus(ext, installStatus)
setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", resolvedBundle.Image))

l.V(1).Info("watching managed objects")
cache, err := r.Manager.Get(ctx, ext)
if err != nil {
// No need to wrap error with resolution information here (or beyond) since the
// bundle was successfully installed and the information will be present in
// the .status.installed field
setStatusProgressing(ext, err)
return ctrl.Result{}, err
}

if err := cache.Watch(ctx, r.controller, managedObjs...); err != nil {
setStatusProgressing(ext, err)
return ctrl.Result{}, err
}

// If we made it here, we have successfully reconciled the ClusterExtension
// and have reached the desired state. Since the Progressing status should reflect
// our progress towards the desired state, we also set it when we have reached
// the desired state by providing a nil error value.
setStatusProgressing(ext, nil)
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -438,6 +451,10 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error {
return nil
}

func wrapErrorWithResolutionInfo(resolved ocv1alpha1.BundleMetadata, err error) error {
return fmt.Errorf("%w for resolved bundle %q with version %q", err, resolved.Name, resolved.Version)
}

// Generate reconcile requests for all cluster extensions affected by a catalog change
func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) crhandler.MapFunc {
return func(ctx context.Context, _ client.Object) []reconcile.Request {
Expand Down
Loading
Loading