Skip to content

Commit

Permalink
roachprod,roachtest: make mt roachtests run secure
Browse files Browse the repository at this point in the history
This PR updates the `acceptance/multitenant` and `multitenant-upgrade`
roachtests to run in secure mode, now that the necessary infra. to run
multitenant roachtests in secure mode has been added to roachprod.

It also updates the roachprod setup to detect whether or not the
cockroach binary supports tenant-scoped client certificates via the
`--help` command's output.

Release note: none
  • Loading branch information
abarganier committed Aug 7, 2022
1 parent f843d36 commit 570056f
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 33 deletions.
1 change: 0 additions & 1 deletion pkg/cmd/roachtest/tests/acceptance.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ func registerAcceptance(r registry.Registry) {
registry.OwnerMultiTenant: {
{
name: "multitenant",
skip: "https://github.com/cockroachdb/cockroach/issues/81506",
fn: runAcceptanceMultitenant,
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/multitenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
func runAcceptanceMultitenant(ctx context.Context, t test.Test, c cluster.Cluster) {
c.Put(ctx, t.Cockroach(), "./cockroach")

c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.All())
c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(install.SecureOption(true)), c.All())

const tenantID = 123
{
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/multitenant_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func runMultiTenantUpgrade(ctx context.Context, t test.Test, c cluster.Cluster,

kvNodes := c.Node(1)

settings := install.MakeClusterSettings(install.BinaryOption(predecessorBinary))
settings := install.MakeClusterSettings(install.BinaryOption(predecessorBinary), install.SecureOption(true))
c.Start(ctx, t.L(), option.DefaultStartOpts(), settings, kvNodes)
tenantStartOpt := createTenantOtherTenantIDs([]int{11, 12, 13, 14})

Expand Down
49 changes: 19 additions & 30 deletions pkg/roachprod/install/cluster_synced.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/log"
"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"
)
Expand Down Expand Up @@ -1138,17 +1137,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)
}

Expand Down Expand Up @@ -1184,6 +1174,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.
Expand Down Expand Up @@ -1331,25 +1339,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 = "----------------------------------------"

Expand Down

0 comments on commit 570056f

Please sign in to comment.