From a4774e22b306dfa73dde1dd41cf3b25152decad3 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 18 Aug 2022 12:12:22 +0000 Subject: [PATCH] clusterversion: prevent upgrades from master versions This change defines a new "unstableVersionsAbove" point on the cluster version line, above which any cluster versions are considered unstable development-only versions which are still subject to change. Performing an upgrade to a version while it is still unstable leaves a cluster in a state where it persists a version that claims it has done that upgrade and all prior, however those upgrades are still subject to change by nature of being unstable. If it subsequently upgraded to a stable version, this could result in subtle and nearly impossible to detect issues, as being at or above a particular version is used to assume that all subsequent version upgrades _as released_ were run; on a cluster that ran an earlier iteration of an upgrade this does not hold. Thus to prevent clusters which upgrade to development versions from subsequently upgrading to a stable version, we offset all development versions -- those above the unstableVersionsAbove point -- into the far future by adding one million to their major version e.g. v22.x-y becomes 1000022.x-y. This means an attempt to subsequently "upgrade" to a stable version -- such as v22.2 -- will look like a downgrade and be forbidden. On the release branch, prior to starting to publish upgradable releases, the unstableVersionsAbove value should be set to invalidVersionKey to reflect that all version upgrades in that release branch are now considered to be stable, meaning they must be treated as immutable and append-only. Release note (ops change): clusters that are upgraded to an alpha or other manual build from the development branch will not be able to be subsequently upgraded to a release build. Release justification: high-priority change to existing functionality, to allow releasing alphas with known version upgrade bugs while ensuring they do not subsequently upgrade into stable version but silently corrupted clusters. --- .../settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- pkg/clusterversion/cockroach_versions.go | 40 +++++++++++++++++-- pkg/kv/kvserver/stores.go | 3 +- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index 69fe43009f8c..6a18a4f14919 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -290,4 +290,4 @@ trace.jaeger.agent string the address of a Jaeger agent to receive traces using trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collector string the address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -version version 22.1-58 set the active cluster version in the format '.' +version version 1000022.1-58 set the active cluster version in the format '.' diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 9aefdc75ff19..22b359588c44 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -221,6 +221,6 @@ trace.opentelemetry.collectorstringaddress of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabledbooleantrueif set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collectorstringthe address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -versionversion22.1-58set the active cluster version in the format '.' +versionversion1000022.1-58set the active cluster version in the format '.' diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index 8cb1fee9c52e..361893a7f08a 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -331,8 +331,8 @@ const TODOPreV21_2 = V21_2 // previously referenced a < 22.1 version until that check/gate can be removed. const TODOPreV22_1 = V22_1 -// versionsSingleton lists all historical versions here in chronological order, -// with comments describing what backwards-incompatible features were +// rawVersionsSingleton lists all historical versions here in chronological +// order, with comments describing what backwards-incompatible features were // introduced. // // A roachpb.Version has the colloquial form MAJOR.MINOR[.PATCH][-INTERNAL], @@ -348,7 +348,11 @@ const TODOPreV22_1 = V22_1 // Such clusters would need to be wiped. As a result, do not bump the major or // minor version until we are absolutely sure that no new migrations will need // to be added (i.e., when cutting the final release candidate). -var versionsSingleton = keyedVersions{ +// +// rawVersionsSingleton is converted to versionsSingleton below, by adding a +// large number to every major if building from master, so as to ensure that +// master builds cannot be upgraded to release-branch builds. +var rawVersionsSingleton = keyedVersions{ { // V21_2 is CockroachDB v21.2. It's used for all v21.2.x patch releases. Key: V21_2, @@ -536,6 +540,36 @@ var versionsSingleton = keyedVersions{ // ************************************************* } +const ( + // unstableVersionsAbove is a cluster version Key above which any upgrades in + // this version are considered unstable development-only versions if it is not + // negative, and upgrading to them should permanently move a cluster to + // development versions. On master it should be the minted version of the last + // release, while on release branches it can be set to invalidVersionKey to + // disable marking any versions as development versions. + unstableVersionsAbove = V22_1 + + // finalVersion should be set on a release branch to the minted final cluster + // version key, e.g. to V22_2 on the release-22.2 branch once it is minted. + // Setting it has the effect of ensuring no versions are subsequently added. + finalVersion = invalidVersionKey +) + +var versionsSingleton = func() keyedVersions { + if unstableVersionsAbove > invalidVersionKey { + const devOffset = 1000000 + // Throw every version above the last release (which will be none on a release + // branch) 1 million major versions into the future, so any "upgrade" to a + // release branch build will be a downgrade and thus blocked. + for i := range rawVersionsSingleton { + if rawVersionsSingleton[i].Key > unstableVersionsAbove { + rawVersionsSingleton[i].Major += devOffset + } + } + } + return rawVersionsSingleton +}() + // TODO(irfansharif): clusterversion.binary{,MinimumSupported}Version // feels out of place. A "cluster version" and a "binary version" are two // separate concepts. diff --git a/pkg/kv/kvserver/stores.go b/pkg/kv/kvserver/stores.go index 9dd1fd81d859..eb5520cb4f29 100644 --- a/pkg/kv/kvserver/stores.go +++ b/pkg/kv/kvserver/stores.go @@ -13,6 +13,7 @@ package kvserver import ( "context" "fmt" + math "math" "sync" "unsafe" @@ -364,7 +365,7 @@ func SynthesizeClusterVersionFromEngines( origin string } - maxPossibleVersion := roachpb.Version{Major: 999999} // Sort above any real version. + maxPossibleVersion := roachpb.Version{Major: math.MaxInt32} // Sort above any real version. minStoreVersion := originVersion{ Version: maxPossibleVersion, origin: "(no store)",