Skip to content
Closed
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
34 changes: 21 additions & 13 deletions pkg/cvo/sync_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,12 @@ func (w *SyncWorker) syncOnce(ctx context.Context, work *SyncWork, maxWorkers in
payloadUpdate.VerifiedImage = info.Verified
payloadUpdate.LoadedAt = time.Now()

// need to make sure the payload is only set when the preconditions have been successfull
if !info.Local && len(w.preconditions) > 0 {
// need to make sure the payload is only set when the preconditions have been successful
if len(w.preconditions) == 0 {
klog.V(4).Info("No preconditions configured.")
} else if info.Local {
klog.V(4).Info("Skipping preconditions for a local operator image payload.")
} else {
reporter.Report(SyncWorkerStatus{
Generation: work.Generation,
Step: "PreconditionChecks",
Expand All @@ -521,17 +525,21 @@ func (w *SyncWorker) syncOnce(ctx context.Context, work *SyncWork, maxWorkers in
Actual: update,
Verified: info.Verified,
})
if err := precondition.Summarize(w.preconditions.RunAll(ctx, precondition.ReleaseContext{DesiredVersion: payloadUpdate.ReleaseVersion})); err != nil && !update.Force {
reporter.Report(SyncWorkerStatus{
Generation: work.Generation,
Failure: err,
Step: "PreconditionChecks",
Initial: work.State.Initializing(),
Reconciling: work.State.Reconciling(),
Actual: update,
Verified: info.Verified,
})
return err
if err := precondition.Summarize(w.preconditions.RunAll(ctx, precondition.ReleaseContext{DesiredVersion: payloadUpdate.ReleaseVersion})); err != nil {
if update.Force {
klog.V(4).Infof("Forcing past precondition failures: %s", err)
} else {
reporter.Report(SyncWorkerStatus{
Generation: work.Generation,
Failure: err,
Step: "PreconditionChecks",
Initial: work.State.Initializing(),
Reconciling: work.State.Reconciling(),
Actual: update,
Verified: info.Verified,
})
return err
}
}
}

Expand Down
10 changes: 9 additions & 1 deletion pkg/payload/precondition/clusterversion/upgradeable.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
configv1listers "github.com/openshift/client-go/config/listers/config/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/klog"

"github.com/openshift/cluster-version-operator/lib/resourcemerge"
precondition "github.com/openshift/cluster-version-operator/pkg/payload/precondition"
Expand Down Expand Up @@ -46,12 +47,18 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele

// if we are upgradeable==true we can always upgrade
Copy link
Contributor

Choose a reason for hiding this comment

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

I always found this comment confusing, or at least overly simplistic, even before your changes. Can it be removed or enhanced.

Copy link
Member Author

Choose a reason for hiding this comment

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

This comment is from #291, so orthogonal to my change. Since this PR is a already a backport, I'd rather not pick it up here. Feel free to file a separate master PR adjusting the wording.

up := resourcemerge.FindOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorUpgradeable)
if up == nil || up.Status != configv1.ConditionFalse {
if up == nil {
klog.V(4).Infof("Precondition %s passed: no Upgradeable condition on ClusterVersion.", pf.Name())
return nil
}
if up.Status != configv1.ConditionFalse {
klog.V(4).Infof("Precondition %s passed: Upgradeable %s since %v: %s: %s", pf.Name(), up.Status, up.LastTransitionTime, up.Reason, up.Message)
return nil
}

// we can always allow the upgrade if there isn't a version already installed
if len(cv.Status.History) == 0 {
klog.V(4).Infof("Precondition %s passed: no release history.", pf.Name())
return nil
}

Expand All @@ -60,6 +67,7 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele

// if there is no difference in the minor version (4.y.z where 4.y is the same for current and desired), then we can still upgrade
if currentMinor == desiredMinor {
klog.V(4).Infof("Precondition %q passed: minor from the current %s matches minor from the target %s (both %s).", pf.Name(), cv.Status.History[0].Version, releaseContext.DesiredVersion, currentMinor)
return nil
}

Expand Down