diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index fda11e5a3..1c6bca50b 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -121,6 +121,11 @@ type Operator struct { upgradeable *upgradeable upgradeableChecks []upgradeableCheck + // minimumUpgradeableCheckInterval is the minimum duration before another + // synchronization of the upgradeable status can happen during failing + // precondition checks. + minimumUpgradeableCheckInterval time.Duration + // verifier, if provided, will be used to check an update before it is executed. // Any error will prevent an update payload from being accessed. verifier verify.Interface @@ -179,10 +184,11 @@ func New( Image: releaseImage, }, - statusInterval: 15 * time.Second, - minimumUpdateCheckInterval: minimumInterval, - payloadDir: overridePayloadDir, - defaultUpstreamServer: "https://api.openshift.com/api/upgrades_info/v1/graph", + statusInterval: 15 * time.Second, + minimumUpdateCheckInterval: minimumInterval, + minimumUpgradeableCheckInterval: 15 * time.Second, + payloadDir: overridePayloadDir, + defaultUpstreamServer: "https://api.openshift.com/api/upgrades_info/v1/graph", client: client, kubeClient: kubeClient, @@ -627,7 +633,7 @@ func (optr *Operator) upgradeableSync(ctx context.Context, key string) error { return nil } - return optr.syncUpgradeable() + return optr.syncUpgradeable(config) } // isOlderThanLastUpdate returns true if the cluster version is older than diff --git a/pkg/cvo/upgradeable.go b/pkg/cvo/upgradeable.go index 465a875a2..d42b7f318 100644 --- a/pkg/cvo/upgradeable.go +++ b/pkg/cvo/upgradeable.go @@ -33,12 +33,13 @@ const ( var adminAckGateRegexp = regexp.MustCompile(adminAckGateFmt) -// syncUpgradeable. The status is only checked if it has been more than -// the minimumUpdateCheckInterval since the last check. -func (optr *Operator) syncUpgradeable() error { - // updates are only checked at most once per minimumUpdateCheckInterval or if the generation changes +// syncUpgradeable synchronizes the upgradeable status only if it has been more than +// the minimumUpdateCheckInterval since the last synchronization or the precondition +// checks on the payload are failing for less than minimumUpdateCheckInterval, and it has +// been more than the minimumUpgradeableCheckInterval since the last synchronization. +func (optr *Operator) syncUpgradeable(config *configv1.ClusterVersion) error { u := optr.getUpgradeable() - if u != nil && u.RecentlyChanged(optr.minimumUpdateCheckInterval) { + if u != nil && u.RecentlyChanged(optr.minimumUpdateCheckInterval) && !shouldSyncUpgradeableDueToPreconditionChecks(optr, config, u) { klog.V(2).Infof("Upgradeable conditions were recently checked, will try later.") return nil } @@ -92,8 +93,14 @@ type upgradeable struct { Conditions []configv1.ClusterOperatorStatusCondition } +// hasPassedDurationSinceTime checks if a certain amount of time specified by duration +// has passed since time. Returns true if the duration has passed, returns false otherwise. +func hasPassedDurationSinceTime(t time.Time, duration time.Duration) bool { + return time.Now().After(t.Add(duration)) +} + func (u *upgradeable) RecentlyChanged(interval time.Duration) bool { - return u.At.After(time.Now().Add(-interval)) + return !hasPassedDurationSinceTime(u.At, interval) } func (u *upgradeable) NeedsUpdate(original *configv1.ClusterVersion) *configv1.ClusterVersion { @@ -442,3 +449,22 @@ func (optr *Operator) adminGatesEventHandler() cache.ResourceEventHandler { DeleteFunc: optr.deleteFunc, } } + +// shouldSyncUpgradeableDueToPreconditionChecks checks if the upgradeable status should +// be synchronized due to the precondition checks. It checks whether the precondition +// checks on the payload are failing for less than minimumUpdateCheckInterval, and it has +// been more than the minimumUpgradeableCheckInterval since the last synchronization. +// This means, upon precondition failure, we will synchronize the upgradeable status +// more frequently at beginning of an upgrade. +// +// shouldSyncUpgradeableDueToPreconditionChecks expects the parameters not to be nil. +// +// Function returns true if the synchronization should happen, returns false otherwise. +func shouldSyncUpgradeableDueToPreconditionChecks(optr *Operator, config *configv1.ClusterVersion, u *upgradeable) bool { + cond := resourcemerge.FindOperatorStatusCondition(config.Status.Conditions, DesiredReleaseAccepted) + if cond != nil && cond.Reason == "PreconditionChecks" && cond.Status == configv1.ConditionFalse && + hasPassedDurationSinceTime(u.At, optr.minimumUpgradeableCheckInterval) && !hasPassedDurationSinceTime(cond.LastTransitionTime.Time, optr.minimumUpdateCheckInterval) { + return true + } + return false +}