roachtest: rename clusterImpl to roachprodCluster#169384
Open
golgeek wants to merge 6 commits intocockroachdb:masterfrom
Open
roachtest: rename clusterImpl to roachprodCluster#169384golgeek wants to merge 6 commits intocockroachdb:masterfrom
golgeek wants to merge 6 commits intocockroachdb:masterfrom
Conversation
Introduce `testCluster`, an internal interface that captures the runner's view of a cluster. The runner currently accesses many `clusterImpl` fields directly; the interface forces these accesses through methods so that alternative cluster backends can be plugged in without touching runner internals. The interface embeds `cluster.Cluster` (the public test API) and adds the lifecycle and configuration methods the runner needs: destroy, save, wipe, cockroach staging, labels, and architecture/encryption knobs. `clusterImpl` satisfies the new interface; no behavior changes. Epic: none Release note: None
Replace `*clusterImpl` with `testCluster` throughout the runner, cluster registry, github issue reporting, and test monitor. All direct field accesses (e.g. `c.arch`, `c.encAtRest`, `c.clusterSettings`) are replaced with the corresponding interface methods introduced in the previous commit. A few methods that were missing from the interface surface are added here: `OS()`, `SetUseDRPC()`, `GetLiveMigrationVMs()`, `Extend()`, `Saved()`, `status()`, and `SetClusterSetting()`. In `github.go`, a nil-interface guard (`isNilTestCluster`) is added because the cluster may be nil when constructing issue info for tests that failed before cluster creation. Epic: none Release note: None
golgeek
requested review from
cpj2195 and
srosenberg
and removed request for
a team
Contributor
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Member
|
This change is |
|
Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link) |
Remove `assertConsistentReplicas` entirely: drop it from the `testCluster` interface and delete the implementation on `clusterImpl`. The method's only caller in `postTestAssertions` now uses `roachtestutil.CheckReplicaDivergenceOnDB` directly, wrapped in a 20-minute timeout. Behavior is unchanged. Epic: none Release note: None
Separate the cluster factory's public entry point (`newCluster`) from the roachprod-specific implementation (`newRoachprodCluster`). This lets a future managed-service backend provide its own `newCluster` without modifying the roachprod path. Introduce the type alias `roachprodCluster = clusterImpl` so that the roachprod backend can be referenced by a self-documenting name. Rename the `roachprodCluster` enum constant to `roachprodClusterType` to avoid the name collision. Epic: none Release note: None
Replace a `c.(*clusterImpl).encAtRest` type assertion in the test
helpers with an interface-based accessor
(`c.(interface{ EncryptedAtRest() bool })`). This removes the last
direct reference to `*clusterImpl` outside the cluster package,
allowing tests to work with any `testCluster` implementation.
Epic: none
Release note: None
Remove the type alias (`roachprodCluster = clusterImpl`) introduced in the previous PR and rename the struct itself. This gives the roachprod-backed cluster implementation a self-documenting name, making it clear which backend is in use and leaving room for future alternative implementations. Purely mechanical rename; no behavior changes. Epic: none Release note: None Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
golgeek
force-pushed
the
ludo/rt-rename-clusterImpl-to-roachprodCluster
branch
from
ae14db1 to
8467193
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #169382.
Remove the type alias (
roachprodCluster = clusterImpl) introduced in the previous refactoring PR and rename the struct itself.This gives the
roachprod-backed cluster implementation a self-documenting name, making it clear which backend is in use and leaving room for future alternative implementations.Purely mechanical rename; no behavior changes.
Epic: none
Release note: None