diff --git a/pkg/cmd/roachtest/tests/multitenant_fairness.go b/pkg/cmd/roachtest/tests/multitenant_fairness.go index d0d3b4495036..8a035dc2e8c1 100644 --- a/pkg/cmd/roachtest/tests/multitenant_fairness.go +++ b/pkg/cmd/roachtest/tests/multitenant_fairness.go @@ -64,9 +64,7 @@ func registerMultiTenantFairness(r registry.Registry) { s.maxLoadOps = 100_000 r.Add(registry.TestSpec{ - Name: fmt.Sprintf("multitenant/fairness/kv/%s/%s", s.name, acStr[s.acEnabled]), - // TODO(cucaroach): remove this once #82926 is resolved. - Skip: "#82926", + Name: fmt.Sprintf("multitenant/fairness/kv/%s/%s", s.name, acStr[s.acEnabled]), Cluster: r.MakeClusterSpec(5), Owner: registry.OwnerSQLQueries, NonReleaseBlocker: false, @@ -95,9 +93,7 @@ func registerMultiTenantFairness(r registry.Registry) { s.maxLoadOps = 1000 r.Add(registry.TestSpec{ - Name: fmt.Sprintf("multitenant/fairness/store/%s/%s", s.name, acStr[s.acEnabled]), - // TODO(cucaroach): remove this once #82926 is resolved. - Skip: "#82926", + Name: fmt.Sprintf("multitenant/fairness/store/%s/%s", s.name, acStr[s.acEnabled]), Cluster: r.MakeClusterSpec(5), Owner: registry.OwnerSQLQueries, NonReleaseBlocker: false, @@ -162,7 +158,7 @@ func runMultiTenantFairness( const ( tenantBaseID = 11 tenantBaseHTTPPort = 8081 - tenantBaseSQLPort = 26257 + tenantBaseSQLPort = 26259 ) tenantHTTPPort := func(offset int) int { diff --git a/pkg/cmd/roachtest/tests/multitenant_utils.go b/pkg/cmd/roachtest/tests/multitenant_utils.go index 6d81200e0117..2cf2fea5d8fc 100644 --- a/pkg/cmd/roachtest/tests/multitenant_utils.go +++ b/pkg/cmd/roachtest/tests/multitenant_utils.go @@ -25,7 +25,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachprod" "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/testutils" - "github.com/cockroachdb/cockroach/pkg/util/version" "github.com/stretchr/testify/require" ) @@ -82,13 +81,7 @@ func createTenantNode( node: node, sqlPort: sqlPort, } - n := c.Node(1) - versionStr, err := fetchCockroachVersion(ctx, t.L(), c, n[0]) - v := version.MustParse(versionStr) - require.NoError(t, err) - // Tenant scoped certificates were introduced in version 22.2. - tenantScopeRequiredVersion := version.MustParse("v22.2.0-alpha.00000000-746-gc030b8b6dc") - if v.AtLeast(tenantScopeRequiredVersion) { + if tn.cockroachBinSupportsTenantScope(ctx, c) { err := tn.recreateClientCertsWithTenantScope(ctx, c, createOptions.otherTenantIDs) require.NoError(t, err) } @@ -96,6 +89,18 @@ func createTenantNode( return tn } +// cockroachBinSupportsTenantScope is a hack to figure out if the version of +// cockroach on the node supports tenant scoped certificates. We can't use a +// version comparison here because we need to compare alpha build versions which +// are compared lexicographically. This is a problem because our alpha versions +// contain an integer count of commits, which does not sort correctly. Once +// this feature ships in a release, it will be easier to do a version comparison +// on whether this command line flag is supported. +func (tn *tenantNode) cockroachBinSupportsTenantScope(ctx context.Context, c cluster.Cluster) bool { + err := c.RunE(ctx, c.Node(tn.node), "./cockroach cert create-client --help | grep '\\--tenant-scope'") + return err == nil +} + func (tn *tenantNode) createTenantCert(ctx context.Context, t test.Test, c cluster.Cluster) { var names []string eips, err := c.ExternalIP(ctx, t.L(), c.Node(tn.node)) diff --git a/pkg/roachprod/install/BUILD.bazel b/pkg/roachprod/install/BUILD.bazel index ad25e3ca9d5c..6d599709c136 100644 --- a/pkg/roachprod/install/BUILD.bazel +++ b/pkg/roachprod/install/BUILD.bazel @@ -36,7 +36,6 @@ go_library( "//pkg/util/retry", "//pkg/util/syncutil", "//pkg/util/timeutil", - "//pkg/util/version", "@com_github_alessio_shellescape//:shellescape", "@com_github_cockroachdb_errors//:errors", "@org_golang_x_sync//errgroup", diff --git a/pkg/roachprod/install/cluster_synced.go b/pkg/roachprod/install/cluster_synced.go index ae75398735ab..3f09950faed7 100644 --- a/pkg/roachprod/install/cluster_synced.go +++ b/pkg/roachprod/install/cluster_synced.go @@ -44,7 +44,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/retry" "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" - "github.com/cockroachdb/cockroach/pkg/util/version" "github.com/cockroachdb/errors" "golang.org/x/sync/errgroup" ) @@ -1182,17 +1181,8 @@ func (c *SyncedCluster) createTenantCertBundle( } defer sess.Close() - // NB: We compare against the alpha version here because in semver v22.2.0 > v22.2.0-alpha. - tenantScopedCertsVersion := version.MustParse("v22.2.0-alpha.00000000-746-gc030b8b6dc") - var useTenantScopedCerts bool - if v, err := getCockroachVersion(ctx, c, node); err != nil { - l.Printf("unknown cockroach binary version on host cluster, using tenant scoped certs by default") - useTenantScopedCerts = true - } else { - useTenantScopedCerts = v.AtLeast(tenantScopedCertsVersion) - } var tenantScopeArg string - if useTenantScopedCerts { + if c.cockroachBinSupportsTenantScope(ctx, node) { tenantScopeArg = fmt.Sprintf("--tenant-scope %d", tenantID) } @@ -1228,6 +1218,24 @@ tar cvf %[5]s $CERT_DIR }) } +// cockroachBinSupportsTenantScope is a hack to figure out if the version of +// cockroach on the node supports tenant scoped certificates. We can't use a +// version comparison here because we need to compare alpha build versions which +// are compared lexicographically. This is a problem because our alpha versions +// contain an integer count of commits, which does not sort correctly. Once +// this feature ships in a release, it will be easier to do a version comparison +// on whether this command line flag is supported. +func (c *SyncedCluster) cockroachBinSupportsTenantScope(ctx context.Context, node Node) bool { + sess, err := c.newSession(node) + if err != nil { + return false + } + defer sess.Close() + + cmd := fmt.Sprintf("%s cert create-client --help | grep '\\--tenant-scope'", cockroachNodeBinary(c, node)) + return sess.Run(ctx, cmd) == nil +} + // getFile retrieves the given file from the first node in the cluster. The // filename is assumed to be relative from the home directory of the node's // user. @@ -1375,25 +1383,6 @@ func (c *SyncedCluster) distributeLocalCertsTar( }) } -func getCockroachVersion( - ctx context.Context, c *SyncedCluster, node Node, -) (*version.Version, error) { - sess, err := c.newSession(node) - if err != nil { - return nil, err - } - defer sess.Close() - - cmd := fmt.Sprintf("%s version --build-tag", cockroachNodeBinary(c, node)) - out, err := sess.CombinedOutput(ctx, cmd) - if err != nil { - return nil, errors.Wrapf(err, "~ %s\n%s", cmd, out) - } - - verString := strings.TrimSpace(string(out)) - return version.Parse(verString) -} - const progressDone = "=======================================>" const progressTodo = "----------------------------------------"