Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

roachtest: backport improvements to mixedversion framework (secure clusters, fixtures opt-out) #106545

Conversation

renatolabs
Copy link
Contributor

Backports:

Please see individual PRs for details.

/cc @cockroachdb/release

Epic: none

Release note: None

Release justification: test-only changes.

This commit fixes a number of small bugs related to using secure
clusters in roachtests, which probably stayed dormant up to this point
because we don't use secure clusters enough, especially in tests where
nodes are restarted.

In this commit, we avoid refetching certs when restarting, fix the
checks for certificate existence on nodes, and fix the command to wipe
a cluster when certs are to be preserved.

Epic: none

Release note: None
This parameter already existed in the underlying `cluster_synced`
implemenation, but the `cluster` interface exposed to roachtsts forced
that argument to `false`.

This commit changes the interface to expose the parameter, allowing
tests to wipe clusters while keeping the certs intact.

Epic: none

Release note: None
Secure clusters are closer to production deployments and also allow
us to tests features that we couldn't before, like creating new users
with passwords during a test, and then performing SQL operations with
those users.

Epic: CRDB-19321

Release note: None
The `loadTPCHDataset` function was unconditionally removing certs when
the cluster was wiped. In this commit, we add a parameter to that
function to indicate if the cluster being used is `secure`; if so, we
preserve the certs when wiping.

Fixes: cockroachdb#105880

Release note: None
This commit updates the `mixedversion` API with two main changes:

* cluster validation: previously, mixed-version tests would always
start from checked-in fixtures. Those fixtures have an implied cluster
topology and require the cluster to have 4 cockroach nodes -- if
that's not the case, the cluster fails to start up with non-obvious
error messages. To make this situation clearer, we now validate that
the test is being run with the correct number of nodes.

* to allow for larger-scale mixed-version tests (more than 4 nodes),
we now expose a `DisableFixtures` option that can be passed to
`NewTest`. With this option, nodes start with an empty store
directory, eliminating any restrictions on cluster size.

Epic: CRDB-19321

Release note: None
@renatolabs renatolabs requested a review from a team as a code owner July 10, 2023 19:52
@renatolabs renatolabs requested review from herkolategan and smg260 and removed request for a team July 10, 2023 19:52
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@renatolabs
Copy link
Contributor Author

@srosenberg srosenberg self-requested a review July 11, 2023 03:59
It needed changes to work with secure clusters, including
authenticating with the server before performing the `curl` call for
the endpoint being tested.

Epic: none

Release note: None
@renatolabs
Copy link
Contributor Author

tenant-span-stats/mixed-version needed fixing to work with secure clusters. Pushed the fix as a separate commit since this test only exists on release-23.1.

Test is running here, if everything goes well I'll proceed with the merge: https://teamcity.cockroachdb.com/viewQueued.html?itemId=10867137&tab=queuedBuildOverviewTab.

FYI @THardy98.

@renatolabs renatolabs merged commit e38f7bb into cockroachdb:release-23.1 Jul 11, 2023
@renatolabs renatolabs deleted the backport23.1-105454-105891-105455 branch July 11, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants