Skip to content

Commit aceb5bc

Browse files
committed
pkg: add precondition handlers and perform these checks before accepting Update
This adds an interface `Precondition` that defines preflight checks for an Update. It adds one types of checks: 1) ClusterVersion upgradeable * If the upgradeable condition is False, the update should be marked as not safe to proceed. Also adds a Summarize function, that can summarize the precondition checks failures to a single error that is more understandable by cvo sync_loops Adds `UpgradePreconditionCheckFailed` as a known reason for summary of UpdateErrors. Similar to the release-image verification, when an Update is required, * precondition checks are executed using `precondition.List.RunAll`, which runs all the precondition checks in order and returns errors. * If any of the precondition checks fails, the cluster version operators does not start the upgrade to the requested Update * similar to release-image-verification, the user will have to force an upgrade when the Update is rejected due to precondition check failure. Piggybacking on the `force`: > https://github.com/openshift/api/blob/58aab2885e38fd7f4fd57f2615f5f9fe38039e22/config/v1/types_cluster_version.go#L209-L218 > // force allows an administrator to update to an image that has failed > // verification, does not appear in the availableUpdates list, or otherwise > // would be blocked by normal protections on update. This option should only > // be used when the authenticity of the provided image has been verified out > // of band because the provided image will run with full administrative access > // to the cluster. Do not use this flag with images that comes from unknown > // or potentially malicious sources. > // > // This flag does not override other forms of consistency checking that are > // required before a new update is deployed. > not appear in the availableUpdates list, or otherwise would be blocked by normal protections on update. > This flag does not override other forms of consistency checking that are required before a new update is deployed. Sooo, with our current documentation, the following line allows admins to use force to override any protections on update and not consistency checks.. and I think the precondition checks like the `override protection`,`techpreview and custom noupgrade feature` fall more in to protections bucket rather than the consistencies. Also I would agree that force is a big hammer, but https://bugzilla.redhat.com/show_bug.cgi?id=1730401 is a 4.2 blocker and i would to like to **not** introduce any additions this late in the cycle. So putting this into force for now seems **an** option. But i agree that we should 4.2+ think about allowing users to skip only some protections _these become more important in auto upgrade_. like disconnected people would maybe like to switch off only verify content and while others might want to only skip feature-flags.. but the current proposal of force being the big hammer today does not block moving to future with smaller hammers..
1 parent e6a01b8 commit aceb5bc

File tree

8 files changed

+774
-1
lines changed

8 files changed

+774
-1
lines changed

pkg/cvo/cvo.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ import (
3737
"github.com/openshift/cluster-version-operator/pkg/cvo/internal/dynamicclient"
3838
"github.com/openshift/cluster-version-operator/pkg/internal"
3939
"github.com/openshift/cluster-version-operator/pkg/payload"
40+
"github.com/openshift/cluster-version-operator/pkg/payload/precondition"
41+
preconditioncv "github.com/openshift/cluster-version-operator/pkg/payload/precondition/clusterversion"
4042
"github.com/openshift/cluster-version-operator/pkg/verify"
4143
)
4244

@@ -234,9 +236,10 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
234236

235237
// after the verifier has been loaded, initialize the sync worker with a payload retriever
236238
// which will consume the verifier
237-
optr.configSync = NewSyncWorker(
239+
optr.configSync = NewSyncWorkerWithPreconditions(
238240
optr.defaultPayloadRetriever(),
239241
NewResourceBuilder(restConfig, burstRestConfig, optr.coLister),
242+
optr.defaultPreconditionChecks(),
240243
optr.minimumUpdateCheckInterval,
241244
wait.Backoff{
242245
Duration: time.Second * 10,
@@ -643,3 +646,9 @@ func hasReachedLevel(cv *configv1.ClusterVersion, desired configv1.Update) bool
643646
}
644647
return desired.Image == cv.Status.History[0].Image
645648
}
649+
650+
func (optr *Operator) defaultPreconditionChecks() precondition.List {
651+
return []precondition.Precondition{
652+
preconditioncv.NewUpgradeable(optr.cvLister),
653+
}
654+
}

0 commit comments

Comments
 (0)