Merge pull request #111233 from RaduBerinde/backport22.2-111137

release-22.2: roachtest: expand tag functionality to allow filtering by conjunctive…

build/teamcity/cockroach/nightlies/roachtest_nightly_impl.sh

@@ -25,4 +25,4 @@ build/teamcity-roachtest-invoke.sh \
   --artifacts=/artifacts \
   --artifacts-literal="${LITERAL_ARTIFACTS_DIR:-}" \
   --slack-token="${SLACK_TOKEN}" \
-  "${TESTS}"
+  "${TESTS}" ${FILTER}

build/teamcity/util/roachtest_util.sh

@@ -57,16 +57,22 @@ trap upload_stats EXIT
 PARALLELISM=16
 CPUQUOTA=1024
 TESTS="${TESTS-}"
+FILTER="${FILTER-}"
 case "${CLOUD}" in
   gce)
+      # Confusing due to how we've handled tags in the past where it has been assumed that all tests should
+      # be run on GCE. Now with refactoring of how tags are handled, we need:
+      # - "default" to ensure we select tests that don't have any user specified tags (preserve old behavior)
+      # - "aws" to ensure we select tests that now no longer have "default" because they have the "aws" tag
+      # Ideally, refactor the tags themselves to be explicit about what cloud they are for and when they can run.
+      # https://github.com/cockroachdb/cockroach/issues/100605
+      FILTER="tag:aws tag:default"
     ;;
   aws)
     PARALLELISM=3
     CPUQUOTA=384
-    if [ -z "${TESTS}" ]; then
-      # NB: anchor ycsb to beginning of line to avoid matching `zfs/ycsb/*` which
-      # isn't supported on AWS at time of writing.
-      TESTS="awsdms|kv(0|95)|^ycsb|tpcc/(headroom/n4cpu16)|tpccbench/(nodes=3/cpu=16)|scbench/randomload/(nodes=3/ops=2000/conc=1)|backup/(KMS/AWS/n3cpu4)"
+    if [ -z "${FILTER}" ]; then
+      FILTER="tag:aws"
     fi
     ;;
   *)

pkg/cmd/roachtest/README.md

@@ -27,6 +27,15 @@ acceptance/cli/node-status [server]
 [...]
 ```
 
+The list can be filtered by passing a regular expression to the `list` command which will match against the test name.
+Multiple `tag:` prefixed args can be specified to further narrow by which tags are present for a test. The following 
+will list all tests with name containing `admission` where test tags match `(weekly && aws) || my-tag`
+
+```
+roachtest list admission tag:weekly,aws tag:my-tag
+```
+
+
 ## Getting the binaries
 
 To run a test, the `roachtest run` command is used. Since a test typically

pkg/cmd/roachtest/main.go

@@ -210,15 +210,25 @@ Use --bench to list benchmarks instead of tests.
 
 Each test has a set of tags. The tags are used to skip tests which don't match
 the tag filter. The tag filter is specified by specifying a pattern with the
-"tag:" prefix. The default tag filter is "tag:default" which matches any test
-that has the "default" tag. Note that tests are selected based on their name,
-and skipped based on their tag.
+"tag:" prefix. 
+
+If multiple "tag:" patterns are specified, the test must match at
+least one of them.
+
+Within a single "tag:" pattern, multiple tags can be specified by separating them
+with a comma. In this case, the test must match all of the tags.
 
 Examples:
 
    roachtest list acceptance copy/bank/.*false
-   roachtest list tag:acceptance
+   roachtest list tag:owner-kv
    roachtest list tag:weekly
+
+   # match weekly kv owned tests
+   roachtest list tag:owner-kv,weekly
+
+   # match weekly kv owner tests or aws tests
+   roachtest list tag:owner-kv,weekly tag:aws
 `,
 		RunE: func(_ *cobra.Command, args []string) error {
 			r, err := makeTestRegistry(cloud, instanceType, zonesF, localSSDArg, listBench)
@@ -240,6 +250,8 @@ Examples:
 	}
 	listCmd.Flags().BoolVar(
 		&listBench, "bench", false, "list benchmarks instead of tests")
+	listCmd.Flags().StringVar(
+		&cloud, "cloud", cloud, "cloud provider to use (aws, azure, or gce)")
 
 	runFn := func(args []string, benchOnly bool) error {
 		if literalArtifacts == "" {

pkg/cmd/roachtest/registry/filter.go

@@ -19,9 +19,15 @@ import (
 // See NewTestFilter.
 type TestFilter struct {
 	Name *regexp.Regexp
-	Tag  *regexp.Regexp
-	// RawTag is the string representation of the regexps in tag.
-	RawTag     []string
+	// Multiple `tag:` parameters can be passed for which only one needs to match, but the
+	// value of a single `tag:` parameter can be a comma-separated list of tags which all need
+	// to match.
+	// e.g. `tag:foo,bar` matches tests with tags `foo` and `bar`, and `tag:foo tag:bar` matches
+	// tests with either tag `foo` or tag `bar`.
+	//
+	// This set contains each tag, so the above examples would be represented as `["foo,bar"]` and
+	// `["foo", "bar"]` respectively..
+	Tags       map[string]struct{}
 	RunSkipped bool
 }
 
@@ -31,22 +37,15 @@ type TestFilter struct {
 // name.
 func NewTestFilter(filter []string, runSkipped bool) *TestFilter {
 	var name []string
-	var tag []string
-	var rawTag []string
+	tags := make(map[string]struct{})
 	for _, v := range filter {
 		if strings.HasPrefix(v, "tag:") {
-			tag = append(tag, strings.TrimPrefix(v, "tag:"))
-			rawTag = append(rawTag, v)
+			tags[strings.TrimPrefix(v, "tag:")] = struct{}{}
 		} else {
 			name = append(name, v)
 		}
 	}
 
-	if len(tag) == 0 {
-		tag = []string{DefaultTag}
-		rawTag = []string{"tag:" + DefaultTag}
-	}
-
 	makeRE := func(strs []string) *regexp.Regexp {
 		switch len(strs) {
 		case 0:
@@ -63,8 +62,7 @@ func NewTestFilter(filter []string, runSkipped bool) *TestFilter {
 
 	return &TestFilter{
 		Name:       makeRE(name),
-		Tag:        makeRE(tag),
-		RawTag:     rawTag,
+		Tags:       tags,
 		RunSkipped: runSkipped,
 	}
 }

pkg/cmd/roachtest/registry/test_spec.go

@@ -12,6 +12,7 @@ package registry
 
 import (
 	"context"
+	"strings"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
@@ -50,7 +51,7 @@ type TestSpec struct {
 	// Tags is a set of tags associated with the test that allow grouping
 	// tests. If no tags are specified, the set ["default"] is automatically
 	// given.
-	Tags []string
+	Tags map[string]struct{}
 	// Cluster provides the specification for the cluster to use for the test.
 	Cluster spec.ClusterSpec
 	// NativeLibs specifies the native libraries required to be present on
@@ -111,16 +112,42 @@ func (t *TestSpec) Match(filter *TestFilter) MatchType {
 	if !filter.Name.MatchString(t.Name) {
 		return FailedFilter
 	}
-	if len(t.Tags) == 0 {
-		if !filter.Tag.MatchString("default") {
-			return FailedTags
-		}
+
+	if len(filter.Tags) == 0 {
 		return Matched
 	}
-	for _, t := range t.Tags {
-		if filter.Tag.MatchString(t) {
+
+	for tag := range filter.Tags {
+		// If the tag is a single CSV e.g. "foo,bar,baz", we match all the tags
+		if matchesAll(t.Tags, strings.Split(tag, ",")) {
 			return Matched
 		}
 	}
+
 	return FailedTags
 }
+
+func matchesAll(testTags map[string]struct{}, filterTags []string) bool {
+	for _, tag := range filterTags {
+		negate := false
+		if tag[0] == '!' {
+			negate = true
+			tag = tag[1:]
+		}
+		_, tagExists := testTags[tag]
+
+		if negate == tagExists {
+			return false
+		}
+	}
+	return true
+}
+
+// Tags returns a set of strings.
+func Tags(values ...string) map[string]struct{} {
+	set := make(map[string]struct{})
+	for _, s := range values {
+		set[s] = struct{}{}
+	}
+	return set
+}

pkg/cmd/roachtest/test_registry.go

@@ -135,9 +135,9 @@ func (r *testRegistryImpl) prepareSpec(spec *registry.TestSpec) error {
 		return fmt.Errorf(`%s: unknown owner [%s]`, spec.Name, spec.Owner)
 	}
 	if len(spec.Tags) == 0 {
-		spec.Tags = []string{registry.DefaultTag}
+		spec.Tags = registry.Tags(registry.DefaultTag)
 	}
-	spec.Tags = append(spec.Tags, "owner-"+string(spec.Owner))
+	spec.Tags["owner-"+string(spec.Owner)] = struct{}{}
 
 	// At the time of writing, we expect the roachtest job to finish within 24h
 	// and have corresponding timeouts set up in CI. Since each individual test
@@ -147,10 +147,8 @@ func (r *testRegistryImpl) prepareSpec(spec *registry.TestSpec) error {
 	const maxTimeout = 18 * time.Hour
 	if spec.Timeout > maxTimeout {
 		var weekly bool
-		for _, tag := range spec.Tags {
-			if tag == "weekly" {
-				weekly = true
-			}
+		if _, ok := spec.Tags["weekly"]; ok {
+			weekly = true
 		}
 		if !weekly {
 			return fmt.Errorf(

pkg/cmd/roachtest/test_test.go

@@ -55,19 +55,28 @@ func TestMatchOrSkip(t *testing.T) {
 	testCases := []struct {
 		filter   []string
 		name     string
-		tags     []string
+		tags     map[string]struct{}
 		expected registry.MatchType
 	}{
 		{nil, "foo", nil, registry.Matched},
-		{nil, "foo", []string{"bar"}, registry.FailedTags},
-		{[]string{"tag:b"}, "foo", []string{"bar"}, registry.Matched},
+		{nil, "foo", registry.Tags("bar"), registry.Matched},
+		{[]string{"tag:bar"}, "foo", registry.Tags("bar"), registry.Matched},
+		// Partial tag match is not supported
+		{[]string{"tag:b"}, "foo", registry.Tags("bar"), registry.FailedTags},
 		{[]string{"tag:b"}, "foo", nil, registry.FailedTags},
-		{[]string{"tag:default"}, "foo", nil, registry.Matched},
-		{[]string{"tag:f"}, "foo", []string{"bar"}, registry.FailedTags},
-		{[]string{"f"}, "foo", []string{"bar"}, registry.FailedTags},
-		{[]string{"f"}, "bar", []string{"bar"}, registry.FailedFilter},
-		{[]string{"f", "tag:b"}, "foo", []string{"bar"}, registry.Matched},
-		{[]string{"f", "tag:f"}, "foo", []string{"bar"}, registry.FailedTags},
+		{[]string{"tag:f"}, "foo", registry.Tags("bar"), registry.FailedTags},
+		// Specifying no tag filters matches all tags.
+		{[]string{"f"}, "foo", registry.Tags("bar"), registry.Matched},
+		{[]string{"f"}, "bar", registry.Tags("bar"), registry.FailedFilter},
+		{[]string{"f", "tag:bar"}, "foo", registry.Tags("bar"), registry.Matched},
+		{[]string{"f", "tag:b"}, "foo", registry.Tags("bar"), registry.FailedTags},
+		{[]string{"f", "tag:f"}, "foo", registry.Tags("bar"), registry.FailedTags},
+		// Match tests that have both tags 'abc' and 'bar'
+		{[]string{"f", "tag:abc,bar"}, "foo", registry.Tags("abc", "bar"), registry.Matched},
+		{[]string{"f", "tag:abc,bar"}, "foo", registry.Tags("abc"), registry.FailedTags},
+		// Match tests that have tag 'abc' but not 'bar'
+		{[]string{"f", "tag:abc,!bar"}, "foo", registry.Tags("abc"), registry.Matched},
+		{[]string{"f", "tag:abc,!bar"}, "foo", registry.Tags("abc", "bar"), registry.FailedTags},
 	}
 	for _, c := range testCases {
 		t.Run("", func(t *testing.T) {

pkg/cmd/roachtest/tests/acceptance.go

@@ -77,7 +77,6 @@ func registerAcceptance(r registry.Registry) {
 			},
 		},
 	}
-	tags := []string{"default", "quick"}
 	specTemplate := registry.TestSpec{
 		// NB: teamcity-post-failures.py relies on the acceptance tests
 		// being named acceptance/<testname> and will avoid posting a
@@ -87,7 +86,7 @@ func registerAcceptance(r registry.Registry) {
 		// will be posted.
 		Name:    "acceptance",
 		Timeout: 10 * time.Minute,
-		Tags:    tags,
+		Tags:    registry.Tags("default", "quick"),
 	}
 
 	for owner, tests := range testCases {

pkg/cmd/roachtest/tests/activerecord.go

@@ -250,7 +250,7 @@ func registerActiveRecord(r registry.Registry) {
 		Owner:      registry.OwnerSQLFoundations,
 		Cluster:    r.MakeClusterSpec(1),
 		NativeLibs: registry.LibGEOS,
-		Tags:       []string{`default`, `orm`},
+		Tags:       registry.Tags(`default`, `orm`),
 		Run:        runActiveRecord,
 	})
 }

pkg/cmd/roachtest/tests/asyncpg.go

@@ -141,7 +141,7 @@ func registerAsyncpg(r registry.Registry) {
 		Name:    "asyncpg",
 		Owner:   registry.OwnerSQLFoundations,
 		Cluster: r.MakeClusterSpec(1, spec.CPU(16)),
-		Tags:    []string{`default`, `orm`},
+		Tags:    registry.Tags(`default`, `orm`),
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			runAsyncpg(ctx, t, c)
 		},

pkg/cmd/roachtest/tests/awsdms.go

@@ -128,7 +128,7 @@ func registerAWSDMS(r registry.Registry) {
 		Name:    "awsdms",
 		Owner:   registry.OwnerMigrations,
 		Cluster: r.MakeClusterSpec(1),
-		Tags:    []string{`weekly`, `aws-weekly`},
+		Tags:    registry.Tags(`weekly`, `aws-weekly`),
 		Run:     runAWSDMS,
 	})
 }

pkg/cmd/roachtest/tests/backup.go

@@ -762,16 +762,18 @@ func registerBackup(r registry.Registry) {
 	for _, item := range []struct {
 		kmsProvider string
 		machine     string
+		tags        map[string]struct{}
 	}{
 		{kmsProvider: "GCS", machine: spec.GCE},
-		{kmsProvider: "AWS", machine: spec.AWS},
+		{kmsProvider: "AWS", machine: spec.AWS, tags: registry.Tags("aws")},
 	} {
 		item := item
 		r.Add(registry.TestSpec{
 			Name:              fmt.Sprintf("backup/KMS/%s/%s", item.kmsProvider, KMSSpec.String()),
 			Owner:             registry.OwnerDisasterRecovery,
 			Cluster:           KMSSpec,
 			EncryptionSupport: registry.EncryptionMetamorphic,
+			Tags:              item.tags,
 			Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 				if c.Spec().Cloud != item.machine {
 					t.Skip("backupKMS roachtest is only configured to run on "+item.machine, "")

pkg/cmd/roachtest/tests/cdc.go

@@ -707,7 +707,7 @@ func registerCDC(r registry.Registry) {
 		Owner:           registry.OwnerCDC,
 		Benchmark:       true,
 		Cluster:         r.MakeClusterSpec(4, spec.CPU(16)),
-		Tags:            []string{"manual"},
+		Tags:            registry.Tags("manual"),
 		RequiresLicense: true,
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			cdcBasicTest(ctx, t, c, cdcTestArgs{

pkg/cmd/roachtest/tests/django.go

@@ -217,7 +217,7 @@ func registerDjango(r registry.Registry) {
 		Name:    "django",
 		Owner:   registry.OwnerSQLFoundations,
 		Cluster: r.MakeClusterSpec(1, spec.CPU(16)),
-		Tags:    []string{`default`, `orm`},
+		Tags:    registry.Tags(`default`, `orm`),
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			runDjango(ctx, t, c)
 		},

pkg/cmd/roachtest/tests/fixtures.go

@@ -65,7 +65,7 @@ func registerFixtures(r registry.Registry) {
 	spec := registry.TestSpec{
 		Name:    "generate-fixtures",
 		Timeout: 30 * time.Minute,
-		Tags:    []string{"fixtures"},
+		Tags:    registry.Tags("fixtures"),
 		Owner:   registry.OwnerDevInf,
 		Cluster: r.MakeClusterSpec(4),
 		Run:     runFixtures,

pkg/cmd/roachtest/tests/gopg.go

@@ -166,7 +166,7 @@ func registerGopg(r registry.Registry) {
 		Name:    "gopg",
 		Owner:   registry.OwnerSQLFoundations,
 		Cluster: r.MakeClusterSpec(1),
-		Tags:    []string{`default`, `orm`},
+		Tags:    registry.Tags(`default`, `orm`),
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			runGopg(ctx, t, c)
 		},

pkg/cmd/roachtest/tests/gorm.go

@@ -136,7 +136,7 @@ func registerGORM(r registry.Registry) {
 		Name:    "gorm",
 		Owner:   registry.OwnerSQLFoundations,
 		Cluster: r.MakeClusterSpec(1),
-		Tags:    []string{`default`, `orm`},
+		Tags:    registry.Tags(`default`, `orm`),
 		Run:     runGORM,
 	})
 }

pkg/cmd/roachtest/tests/hibernate.go

@@ -238,7 +238,7 @@ func registerHibernate(r registry.Registry, opt hibernateOptions) {
 		Owner:      registry.OwnerSQLFoundations,
 		Cluster:    r.MakeClusterSpec(1),
 		NativeLibs: registry.LibGEOS,
-		Tags:       []string{`default`, `orm`},
+		Tags:       registry.Tags(`default`, `orm`),
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			runHibernate(ctx, t, c)
 		},

pkg/cmd/roachtest/tests/jasyncsql.go

@@ -150,7 +150,7 @@ func registerJasyncSQL(r registry.Registry) {
 		Name:    "jasync",
 		Owner:   registry.OwnerSQLFoundations,
 		Cluster: r.MakeClusterSpec(1),
-		Tags:    []string{`default`, `orm`},
+		Tags:    registry.Tags(`default`, `orm`),
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			runJasyncSQL(ctx, t, c)
 		},