Skip to content

Commit

Permalink
server,settings: assert tenant cluster version is initialized
Browse files Browse the repository at this point in the history
It is invalid to start a tenant server if the version cluster setting
is not initialized. However, it's fairly easy for test code to forget
to do this, usually by mistakenly using
`cluster.MakeClusterSettings()` without calling
`clusterversion.Initialize()`.

This change adds an assertion that errors out if the version setting
is not set correctly.

Release note: None
  • Loading branch information
knz committed Apr 5, 2023
1 parent b4f737c commit 4aa59bc
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 1 deletion.
7 changes: 7 additions & 0 deletions pkg/clusterversion/clusterversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ func Initialize(ctx context.Context, ver roachpb.Version, sv *settings.Values) e
return version.initialize(ctx, ver, sv)
}

// AssertInitialized checks whether Initialize() has been called yet. This
// is used in test code to assert that an initial cluster version has
// been set when that matters.
func AssertInitialized(ctx context.Context, sv *settings.Values) {
_ = version.activeVersion(ctx, sv)
}

// Handle is the interface through which callers access the active cluster
// version and this binary's version details.
type Handle interface {
Expand Down
12 changes: 12 additions & 0 deletions pkg/server/testserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,18 @@ func (ts *TestServer) StartTenant(
if st == nil {
st = cluster.MakeTestingClusterSettings()
}
// Verify that the settings object that was passed in has
// initialized the version setting. This is pretty much necessary
// for secondary tenants. See the comments at the beginning of
// `runStartSQL()` in cli/mt_start_sql.go and
// `makeSharedProcessTenantServerConfig()` in
// server_controller_new_server.go.
//
// The version is initialized in MakeTestingClusterSettings(). This
// assertion is there to prevent inadvertent changes to
// MakeTestingClusterSettings() and as a guardrail for tests that
// pass a custom params.Settings.
clusterversion.AssertInitialized(ctx, &st.SV)

// Needed for backward-compat on crdb_internal.ranges{_no_leases}.
// Remove in v23.2.
Expand Down
4 changes: 3 additions & 1 deletion pkg/settings/cluster/cluster_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ func MakeClusterSettings() *Settings {
// object is needed, but cluster settings don't play a crucial role.
func MakeTestingClusterSettings() *Settings {
return MakeTestingClusterSettingsWithVersions(
clusterversion.TestingBinaryVersion, clusterversion.TestingBinaryVersion, true /* initializeVersion */)
clusterversion.TestingBinaryVersion,
clusterversion.TestingBinaryVersion,
true /* initializeVersion */)
}

// MakeTestingClusterSettingsWithVersions returns a Settings object that has its
Expand Down

0 comments on commit 4aa59bc

Please sign in to comment.