roachtest,roachprod: detect tenant-scope certs via help, unskip tests

The version comparison code to detect tenant scoped certificates was
still incorrect because the "alpha" portion of semver versions is
compare lexicographically. Since our alpha versions contain a count of
commits, this broke when we hit commit 1000 since the last tag.

Here, we search the help on the command to see if it supports the
tenant-scoped certs flag.  If it does, we assume we will need them.

There is duplication between the multitenant tests and roachprod still
that we should clean up.

Release note: None

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 {

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,20 +81,26 @@ 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)
 	}
 	tn.createTenantCert(ctx, t, c)
 	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))

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",

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 = "----------------------------------------"