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.

Sign up for GitHub

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

Jump to bottom

roachtest: use tags to resolve test target by cloud provider #96655

Closed
msbutler opened this issue Feb 6, 2023 · 2 comments · Fixed by #99402
Closed

roachtest: use tags to resolve test target by cloud provider #96655

msbutler opened this issue Feb 6, 2023 · 2 comments · Fixed by #99402
Assignees
@smg260
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. T-testeng TestEng Team

Comments

@msbutler
Copy link
Collaborator

msbutler commented Feb 6, 2023 •
edited by cockroach-jira-scripts
Loading

Currently, the (hard to find) roachtest_util.sh script decides which roachtests run on which cloud, rather than testspec.tags, which leads to a small amount of wasted cloud resources and a poorer developer experience.

As the script suggests, all roachtests run on gce by default. To prevent a roachtest from running on GCE then, the roachtest writer must skip the test within the TestSpec.Run callback, after gce vms have been provisioned for the test. Example here.

The script also decides which roachtests run on the aws nightly using a brittle regex.

testspec.tags could be used instead to filter which tests run on which cloud at test registration, similar to their use for the weekly / aws-weekly test suite. Tags would obviate the regex filter, as the roachtest run command used in the aws nightly job could simply add an aws tag, similar the aws-weekly job. To prevent a roachtest from running in the default gce nightly job, we could add a aws-only tag as well.

Lastly, the use of tags would make it easy to see which tests run on which cloud, i.e. roachtest list tag:aws

Jira issue: CRDB-24267
@msbutler msbutler added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. T-dev-inf T-testeng TestEng Team labels Feb 6, 2023
@blathers-crl
Copy link

blathers-crl bot commented Feb 6, 2023

cc @cockroachdb/test-eng

@srosenberg
Copy link
Member

It looks like we could easily extend NewTestFilter to support a conjunction of tags, e.g., roachtest list tag:aws tag:weekly. Currently, it supports only a single tag, e.g., roachtest list tag:aws. We probably don't need disjunction or negation until a future use-case arises.

renatolabs added a commit to renatolabs/cockroach that referenced this issue Mar 20, 2023
@renatolabs
build: update restore regexp in roachtest AWS suite
0a52511 
In cockroachdb#98259, the concept of `Low` memory per vCPU was introduced in
roachtest's `ClusterSpec`. That setting is currently only supported in
GCE; if a test that uses that setting is run on any other cloud,
roachtest will panic.

The only roachtest using the low-memory setting is currently a restore
test that is meant to run on GCE. However, the check for the appropriate
cloud is done by that test during runtime (necessarily so), meaning a
cluster will need to be created for the check to happen. By that
point, the test runner will panic due to the unsupported memory
setting.

In the long term, we want to support ways for roachtests to indicate
supported clouds outside the `Run` function (some context in cockroachdb#96655).
For now, to stop a panic from happening in roachtest nightly runs on
AWS, we update the regexp used to choose tests to run on AWS. We
leverage the fact that `restore` tests include the cloud they are
supposed to run in their name.

Epic: none

Release note: None
@renatolabs renatolabs mentioned this issue Mar 20, 2023
Merged
craig bot pushed a commit that referenced this issue Mar 20, 2023
@renatolabs
Merge #99022
4e8e867 
99022: build: update restore regexp in roachtest AWS suite r=msbutler a=renatolabs

In #98259, the concept of `Low` memory per vCPU was introduced in roachtest's `ClusterSpec`. That setting is currently only supported in GCE; if a test that uses that setting is run on any other cloud, roachtest will panic.

The only roachtest using the low-memory setting is currently a restore test that is meant to run on GCE. However, the check for the appropriate cloud is done by that test during runtime (necessarily so), meaning a cluster will need to be created for the check to happen. By that point, the test runner will panic due to the unsupported memory setting.

In the long term, we want to support ways for roachtests to indicate supported clouds outside the `Run` function (some context in #96655). For now, to stop a panic from happening in roachtest nightly runs on AWS, we update the regexp used to choose tests to run on AWS. We leverage the fact that `restore` tests include the cloud they are supposed to run in their name.

Epic: none

Release note: None

Co-authored-by: Renato Costa <[email protected]>
@smg260 smg260 mentioned this issue Mar 23, 2023
Merged
smg260 pushed a commit to smg260/cockroach that referenced this issue Mar 23, 2023
roachtest: expand tag functionality to allow filtering by conjunctive…
849109d 
… tags.

oday, tests are matched if they have any of the specified tags. This patch introduces the ability
to match tests which have all specified tags, whilst attempting to maintain backward compatibility.

Epic: none
Fixes: cockroachdb#96655

Release note: None
smg260 pushed a commit to smg260/cockroach that referenced this issue Mar 24, 2023
roachtest: expand tag functionality to allow filtering by conjunctive…
04d1099 
… tags.

oday, tests are matched if they have any of the specified tags. This patch introduces the ability
to match tests which have all specified tags, whilst attempting to maintain backward compatibility.

Epic: none
Fixes: cockroachdb#96655

Release note: None
smg260 pushed a commit to smg260/cockroach that referenced this issue Mar 27, 2023
roachtest: expand tag functionality to allow filtering by conjunctive…
fc2c63f 
… tags.

oday, tests are matched if they have any of the specified tags. This patch introduces the ability
to match tests which have all specified tags, whilst attempting to maintain backward compatibility.

Epic: none
Fixes: cockroachdb#96655

Release note: None
smg260 pushed a commit to smg260/cockroach that referenced this issue Mar 28, 2023
roachtest: expand tag functionality to allow filtering by conjunctive…
09f2e7d 
… tags.

Today, tests are matched if they have any-of the specified tags.
This patch introduces the ability to also match tests which have "all-of"
specified tags, along with tag negation.

This example will list all tests tagged with "aws" or with both "weekly"
and "owner-test-eng" tags: "roachtest list tag:aws tag:weekly,owner-test-eng"

An example of tag negation which would list all of the tests with tag
"weekly" but not "aws": "roachtest list tag:weekly,!aws"

Epic: none
Fixes: cockroachdb#96655

Release note: None
msbutler added a commit to msbutler/cockroach that referenced this issue Mar 31, 2023
@msbutler
roachtest: prevent aws roachtest panic
9401a80 
After cockroachdb#99723 merged as a bandaid for cockroachdb#98783, the aws roachtest nightly began to
panic because of a different roachtest papercut cockroachdb#96655. Specifically, because
roachtest filters which tests run on which cloud within the evaluation of the
test closure, tests meant to run on gce will still get registered in an AWS
run. During the registration of the gce test
`restore/tpce/400GB/gce/nodes=4/cpus=8/lowmem` _on aws_, the aws test harness
panics because the aws roachprod implementation does not have a low memory cpu
configuration. This patch prevents this panic and should be reverted once
the pr cockroachdb#99402 merges.

Epic: None

Release note: None
@msbutler msbutler mentioned this issue Mar 31, 2023
Merged
craig bot pushed a commit that referenced this issue Mar 31, 2023
@mgartner @herkolategan
@tbg @msbutler @ajwerner @andrewbaptist
Merge #99312 #99348 #99835 #100267 #100286 #100294 #100296
dcac33e 
99312: sqlsmith: add DEFAULT expressions to newly added columns r=mgartner a=mgartner

Sqlsmith now builds `ALTER TABLE .. ADD COLUMN .. DEFAULT` statements
with default expressions that have different types than the column type.
This is allowed if the default expression type can be assignment-casted
to the column's type.

Fixes #98133

Release note: None


99348: testutils: move default test tenant message r=rharding6373 a=herkolategan

In order to reduce logging noise but still inform test authors of the default test tenant, the message has been moved to where there is a `testing.TB` interface.

Epic: CRDB-18499

99835: opt/execbuilder: add panic catching to buildRoutinePlanGenerator r=mgartner a=mgartner

This commit adds a panic catcher to callback functions created in
execbuilder and invoked during evaluation of UDFs and correlated
subqueries. It matches the panic catcher logic in `buildApplyJoin`.

Fixes #98786

Release note: None


100267: roachtest: own autoupgrade to TestEng r=renatolabs a=tbg

Discussed in #99479.

Epic: none
Release note: None


100286: roachtest: prevent aws roachtest panic r=rail a=msbutler

After #99723 merged as a bandaid for #98783, the aws roachtest nightly began to panic because of a different roachtest papercut #96655. Specifically, because roachtest filters which tests run on which cloud within the evaluation of the test closure, tests meant to run on gce will still get registered in an AWS run. During the registration of the gce test
`restore/tpce/400GB/gce/nodes=4/cpus=8/lowmem` _on aws_, the aws test harness panics because the aws roachprod implementation does not have a low memory cpu configuration. This patch prevents this panic and should be reverted once the pr #99402 merges.

Epic: None

Release note: None

100294: tenantcapabilitiestestutils: add a missing default case r=ajwerner a=ajwerner

The test should fail if we ever add a new type of capability and use it in the data driven test but don't update the test to handle it.

Epic: none

Follow-up from #100217 (review)

Release note: None

100296: rpc: correctly check for nil before cast r=ajwerner a=andrewbaptist

As part of the fix of #99104, a cast without a nil check was introduced. This PR addresses that by only casting if it is known to be not nil.

Epic: none
Fixes: #100275
Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: ajwerner <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
@blathers-crl blathers-crl bot mentioned this issue Mar 31, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Mar 31, 2023
@msbutler
roachtest: prevent aws roachtest panic
4a7aa9d 
After #99723 merged as a bandaid for #98783, the aws roachtest nightly began to
panic because of a different roachtest papercut #96655. Specifically, because
roachtest filters which tests run on which cloud within the evaluation of the
test closure, tests meant to run on gce will still get registered in an AWS
run. During the registration of the gce test
`restore/tpce/400GB/gce/nodes=4/cpus=8/lowmem` _on aws_, the aws test harness
panics because the aws roachprod implementation does not have a low memory cpu
configuration. This patch prevents this panic and should be reverted once
the pr #99402 merges.

Epic: None

Release note: None
smg260 pushed a commit to smg260/cockroach that referenced this issue Apr 3, 2023
roachtest: expand tag functionality to allow filtering by conjunctive…
cd6888d 
… tags.

Today, tests are matched if they have any-of the specified tags.
This patch introduces the ability to also match tests which have "all-of"
specified tags, along with tag negation.

This example will list all tests tagged with "aws" or with both "weekly"
and "owner-test-eng" tags: "roachtest list tag:aws tag:weekly,owner-test-eng"

An example of tag negation which would list all of the tests with tag
"weekly" but not "aws": "roachtest list tag:weekly,!aws"

Epic: none
Fixes: cockroachdb#96655

Release note: None
smg260 pushed a commit to smg260/cockroach that referenced this issue Apr 4, 2023
roachtest: expand tag functionality to allow filtering by conjunctive…
2086e3a 
… tags.

Today, tests are matched if they have any-of the specified tags.
This patch introduces the ability to also match tests which have "all-of"
specified tags, along with tag negation.

This example will list all tests tagged with "aws" or with both "weekly"
and "owner-test-eng" tags: "roachtest list tag:aws tag:weekly,owner-test-eng"

An example of tag negation which would list all of the tests with tag
"weekly" but not "aws": "roachtest list tag:weekly,!aws"

Epic: none
Fixes: cockroachdb#96655

Release note: None
craig bot pushed a commit that referenced this issue Apr 5, 2023
@srosenberg @erikgrinaker @rafiss
Merge #99402 #100679 #100693 #100695
c14ba56 
99402: roachtest: expand tag functionality to allow filtering by conjunctive… r=srosenberg,herkolategan a=smg260

… tags.

oday, tests are matched if they have any of the specified tags. This patch introduces the ability to match tests which have all specified tags, whilst attempting to maintain backward compatibility.

Commits
1 - expand tag filtering (AND, !)
2 - convert `TestSpec.Tags` to `map[string]struct{}`
3 - suppress TeamCity log statement


Epic: none
Fixes: #96655

Release note: None

100679: ci: artifacts of FIPS-enabled roachtests should be separate r=rail a=srosenberg

When running FIPS-enabled roachtests, we want to separate their artifacts from other roachtests. This removes the confounding factor for performance tests wherein FIPS may result in performance degradation.

Epic: none

Release note: None

100693: kvserver: skip `TestLeaseRenewer` r=erikgrinaker a=erikgrinaker

This is flaky under race/deadlock, but also had a couple of failures that indicate minor races in (de)activating extensions. I won't have time to look into this immediately, so skipping it for now.

Touches #100689.

Epic: none
Release note: None

100695: logictest: use skip_on_retry earlier in new_schema_changer r=rafiss a=rafiss

Epic: None
Release note: None

Co-authored-by: Miral Gadani <[email protected]>
Co-authored-by: Stan Rosenberg <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
@craig craig bot closed this as completed in fa586a2 Apr 5, 2023
This was referenced Sep 22, 2023
Merged
Merged
RaduBerinde pushed a commit that referenced this issue Sep 26, 2023
@RaduBerinde
roachtest: expand tag functionality to allow filtering by conjunctive…
0812032 
… tags.

Today, tests are matched if they have any-of the specified tags.
This patch introduces the ability to also match tests which have "all-of"
specified tags, along with tag negation.

This example will list all tests tagged with "aws" or with both "weekly"
and "owner-test-eng" tags: "roachtest list tag:aws tag:weekly,owner-test-eng"

An example of tag negation which would list all of the tests with tag
"weekly" but not "aws": "roachtest list tag:weekly,!aws"

Epic: none
Fixes: #96655

Release note: None
RaduBerinde pushed a commit that referenced this issue Sep 27, 2023
@RaduBerinde
roachtest: expand tag functionality to allow filtering by conjunctive…
3311ba9 
… tags.

Today, tests are matched if they have any-of the specified tags.
This patch introduces the ability to also match tests which have "all-of"
specified tags, along with tag negation.

This example will list all tests tagged with "aws" or with both "weekly"
and "owner-test-eng" tags: "roachtest list tag:aws tag:weekly,owner-test-eng"

An example of tag negation which would list all of the tests with tag
"weekly" but not "aws": "roachtest list tag:weekly,!aws"

Epic: none
Fixes: #96655

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smg260 smg260

Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. T-testeng TestEng Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

roachtest: expand tag functionality to allow filtering by conjunctive… smg260/cockroach
3 participants
@srosenberg @smg260 @msbutler