Skip to content

Commit

Permalink
roachtest: preserve vmodule settings for upgraded servers
Browse files Browse the repository at this point in the history
Without this patch upgraded servers in a roachtest are using the default
settings. For example, the rebalance roachtest is upgrading servers in
order to run with mixed version binaries, and those servers are running
without the right settings for logging (don't use vmodule).

This small fix makes sure that we pass the right options to the upgraded
binary, so that for the rebalance roachtest we will maintain the 'vmodule'
settings.

Release note: None
  • Loading branch information
lidorcarmel committed Jul 25, 2022
1 parent 69e5187 commit 2c40767
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 6 deletions.
7 changes: 4 additions & 3 deletions pkg/cmd/roachtest/tests/follower_reads.go
Original file line number Diff line number Diff line change
Expand Up @@ -877,15 +877,16 @@ func runFollowerReadsMixedVersionSingleRegionTest(
// Start the cluster at the old version.
settings := install.MakeClusterSettings()
settings.Binary = uploadVersion(ctx, t, c, c.All(), predecessorVersion)
c.Start(ctx, t.L(), option.DefaultStartOpts(), settings, c.All())
startOpts := option.DefaultStartOpts()
c.Start(ctx, t.L(), startOpts, settings, c.All())
topology := topologySpec{multiRegion: false}
data := initFollowerReadsDB(ctx, t, c, topology)

// Upgrade one node to the new version and run the test.
randNode := 1 + rand.Intn(c.Spec().NodeCount)
t.L().Printf("upgrading n%d to current version", randNode)
nodeToUpgrade := c.Node(randNode)
upgradeNodes(ctx, nodeToUpgrade, curVersion, t, c)
upgradeNodes(ctx, nodeToUpgrade, startOpts, curVersion, t, c)
runFollowerReadsTest(ctx, t, c, topologySpec{multiRegion: false}, exactStaleness, data)

// Upgrade the remaining nodes to the new version and run the test.
Expand All @@ -897,6 +898,6 @@ func runFollowerReadsMixedVersionSingleRegionTest(
remainingNodes = remainingNodes.Merge(c.Node(i + 1))
}
t.L().Printf("upgrading nodes %s to current version", remainingNodes)
upgradeNodes(ctx, remainingNodes, curVersion, t, c)
upgradeNodes(ctx, remainingNodes, startOpts, curVersion, t, c)
runFollowerReadsTest(ctx, t, c, topologySpec{multiRegion: false}, exactStaleness, data)
}
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/rebalance_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func registerRebalanceLoad(r registry.Registry) {
// `cockroach` flag will be used.
const newVersion = ""
c.Start(ctx, t.L(), startOpts, settings, roachNodes)
upgradeNodes(ctx, nodesToUpgrade, newVersion, t, c)
upgradeNodes(ctx, nodesToUpgrade, startOpts, newVersion, t, c)
} else {
c.Put(ctx, t.Cockroach(), "./cockroach", roachNodes)
c.Start(ctx, t.L(), startOpts, settings, roachNodes)
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/roachtest/tests/versionupgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ func uploadAndStart(nodes option.NodeListOption, v string) versionStep {
// Use a waitForUpgradeStep() for that.
func binaryUpgradeStep(nodes option.NodeListOption, newVersion string) versionStep {
return func(ctx context.Context, t test.Test, u *versionUpgradeTest) {
upgradeNodes(ctx, nodes, newVersion, t, u.c)
upgradeNodes(ctx, nodes, option.DefaultStartOpts(), newVersion, t, u.c)
// TODO(nvanbenschoten): add upgrade qualification step. What should we
// test? We could run logictests. We could add custom logic here. Maybe
// this should all be pushed to nightly migration tests instead.
Expand All @@ -383,6 +383,7 @@ func binaryUpgradeStep(nodes option.NodeListOption, newVersion string) versionSt
func upgradeNodes(
ctx context.Context,
nodes option.NodeListOption,
startOpts option.StartOpts,
newVersion string,
t test.Test,
c cluster.Cluster,
Expand Down Expand Up @@ -411,7 +412,6 @@ func upgradeNodes(

binary := uploadVersion(ctx, t, c, c.Node(node), newVersion)
settings := install.MakeClusterSettings(install.BinaryOption(binary))
startOpts := option.DefaultStartOpts()
c.Start(ctx, t.L(), startOpts, settings, c.Node(node))
}
}
Expand Down

0 comments on commit 2c40767

Please sign in to comment.