roachtest: stop using ClusterSpec.Cloud in test code#111324
roachtest: stop using ClusterSpec.Cloud in test code#111324craig[bot] merged 5 commits intocockroachdb:masterfrom
Conversation
d863fa7 to
ed5bb3e
Compare
|
Updated (resolved conflicts). |
renatolabs
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde, @smg260, and @srosenberg)
build/release/teamcity-mark-build.sh line 31 at r5 (raw file):
log_into_gcloud gawscloud container images add-tag "${gcr_repository}:${TC_BUILD_BRANCH}" "${gcr_repository}:latest-${release_branch}-${build_label}-build"
This doesn't look right?
smg260
left a comment
There was a problem hiding this comment.
Are all the OnlyGCE actually only GCE? At least one example that worked locally is ledger/nodes=6/multi-az, whether intended to, or not.
Reviewed 8 of 18 files at r4, 20 of 28 files at r5, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde and @srosenberg)
ed5bb3e to
3633eb9
Compare
RaduBerinde
left a comment
There was a problem hiding this comment.
TFTRs!!
It's a multi AZ test, feels like there's no point to run locally (other than perhaps debugging the test). Note that you will always be able to run whatever you want by adding a flag to override the advertised compatibilities.
I will wait for #111633 to go in and address any conflicts.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @renatolabs, @smg260, and @srosenberg)
build/release/teamcity-mark-build.sh line 31 at r5 (raw file):
Previously, renatolabs (Renato Costa) wrote…
This doesn't look right?
Wow.. fat fingers. Fixed.
smg260
left a comment
There was a problem hiding this comment.
Reviewed 1 of 38 files at r6, 37 of 37 files at r7, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @renatolabs and @srosenberg)
This commit reduces boilerplate in the `Option` code by making it a func instead of an interface with an apply func. This way each option can define the function inline instead of having to define a type. Epic: none Release note: None
This commit moves GCE and AWS specific settings to their own inline structs and adds Options for them. Epic: none Release note: None
`ClusterSpec` now provides options for GCE and AWS machine types. Epic: none Release note: None
`ClusterSpec` now provides options for GCE and AWS zones specification. Epic: none Release note: None
This change removes all remaining uses of `ClusterSpec.Cloud` except those internal to roachtest. Code that is part of running a test now uses `Cluster.Cloud()` instead. Informs: cockroachdb#104029 Release note: None
3633eb9 to
22a79ff
Compare
|
bors r+ |
|
Build succeeded: |
In this PR we fix all test registration code which was depending on the flags passed to
roachtest(indirectly, through TestSpec). In the process, I tried to clean up various things I had to touch.This makes progress in the direction of #104029 and of making the registry and
TestSpecnot depend at all on the flags (the flags should come in later, when we create an actual cluster from the spec).roachtest: spec: simplify Option
This commit reduces boilerplate in the
Optioncode by making it afunc instead of an interface with an apply func. This way each option
can define the function inline instead of having to define a type.
Epic: none
Release note: None
roachtest: spec: provide options for GCE/AWS settings
This commit moves GCE and AWS specific settings to their own
inline structs and adds Options for them.
Epic: none
Release note: None
roachtest: clean up instance type specification
ClusterSpecnow provides options for GCE and AWS machine types.Epic: none
Release note: None
roachtest: clean up zones specification
ClusterSpecnow provides options for GCE and AWS zonesspecification.
Epic: none
Release note: None
roachtest: stop using ClusterSpec.Cloud in test code
This change removes all remaining uses of
ClusterSpec.Cloudexceptthose internal to roachtest. Code that is part of running a test now
uses
Cluster.Cloud()instead.Informs: #104029
Release note: None