Fix a data race with rollbackPeriod. (#17387)

builtin/logical/pki/crl_test.go

@@ -905,11 +905,13 @@ func TestAutoRebuild(t *testing.T) {
 		},
 		// See notes below about usage of /sys/raw for reading cluster
 		// storage without barrier encryption.
-		EnableRaw: true,
+		EnableRaw:      true,
+		RollbackPeriod: newPeriod,
 	}
-	cluster := vault.CreateTestClusterWithRollbackPeriod(t, newPeriod, coreConfig, &vault.TestClusterOptions{
+	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
 		HandlerFunc: vaulthttp.Handler,
 	})
+	cluster.Start()
 	defer cluster.Cleanup()
 	client := cluster.Cores[0].Client
 

builtin/logical/pki/path_tidy_test.go

@@ -33,11 +33,13 @@ func TestAutoTidy(t *testing.T) {
 		},
 		// See notes below about usage of /sys/raw for reading cluster
 		// storage without barrier encryption.
-		EnableRaw: true,
+		EnableRaw:      true,
+		RollbackPeriod: newPeriod,
 	}
-	cluster := vault.CreateTestClusterWithRollbackPeriod(t, newPeriod, coreConfig, &vault.TestClusterOptions{
+	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
 		HandlerFunc: vaulthttp.Handler,
 	})
+	cluster.Start()
 	defer cluster.Cleanup()
 	client := cluster.Cores[0].Client
 

vault/core.go

@@ -637,6 +637,8 @@ type Core struct {
 	// effectiveSDKVersion contains the SDK version that standby nodes should use when
 	// heartbeating with the active node. Default to the current SDK version.
 	effectiveSDKVersion string
+
+	rollbackPeriod time.Duration
 }
 
 func (c *Core) HAState() consts.HAState {
@@ -769,6 +771,8 @@ type CoreConfig struct {
 	DisableSSCTokens bool
 
 	EffectiveSDKVersion string
+
+	RollbackPeriod time.Duration
 }
 
 // GetServiceRegistration returns the config's ServiceRegistration, or nil if it does

vault/core_util.go

@@ -5,6 +5,7 @@ package vault
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/helper/namespace"
@@ -53,6 +54,11 @@ func coreInit(c *Core, conf *CoreConfig) error {
 	if !conf.DisableKeyEncodingChecks {
 		c.physical = physical.NewStorageEncoding(c.physical)
 	}
+
+	c.rollbackPeriod = conf.RollbackPeriod
+	if conf.RollbackPeriod == 0 {
+		c.rollbackPeriod = time.Minute
+	}
 	return nil
 }
 

vault/rollback.go

@@ -9,17 +9,10 @@ import (
 
 	metrics "github.com/armon/go-metrics"
 	log "github.com/hashicorp/go-hclog"
-
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/sdk/logical"
 )
 
-// rollbackPeriod is how often we attempt rollbacks for all the backends.
-//
-// This is turned into a variable to allow test to check behavior without
-// waiting the full minute. See CreateTestClusterWithRollbackPeriod(...).
-var rollbackPeriod = time.Minute
-
 // RollbackManager is responsible for performing rollbacks of partial
 // secrets within logical backends.
 //
@@ -70,7 +63,7 @@ func NewRollbackManager(ctx context.Context, logger log.Logger, backendsFunc fun
 		logger:      logger,
 		backends:    backendsFunc,
 		router:      router,
-		period:      rollbackPeriod,
+		period:      core.rollbackPeriod,
 		inflight:    make(map[string]*rollbackState),
 		doneCh:      make(chan struct{}),
 		shutdownCh:  make(chan struct{}),

vault/testing.go

@@ -1645,6 +1645,8 @@ func NewTestCluster(t testing.T, base *CoreConfig, opts *TestClusterOptions) *Te
 		coreConfig.EnableResponseHeaderHostname = base.EnableResponseHeaderHostname
 		coreConfig.EnableResponseHeaderRaftNodeID = base.EnableResponseHeaderRaftNodeID
 
+		coreConfig.RollbackPeriod = base.RollbackPeriod
+
 		testApplyEntBaseConfig(coreConfig, base)
 	}
 	if coreConfig.ClusterName == "" {
@@ -2441,33 +2443,6 @@ func RetryUntil(t testing.T, timeout time.Duration, f func() error) {
 	t.Fatalf("did not complete before deadline, err: %v", err)
 }
 
-// CreateTestClusterWithRollbackPeriod lets us modify the periodic func
-// invocation time period to some other value.
-//
-// Because multiple tests in the PKI mount use this helper, we've added
-// a lock around it and created the cluster immediately in this helper.
-// This ensures the tests don't race against each other.
-var rollbackPeriodLock sync.Mutex
-
-func CreateTestClusterWithRollbackPeriod(t testing.T, newPeriod time.Duration, base *CoreConfig, opts *TestClusterOptions) *TestCluster {
-	rollbackPeriodLock.Lock()
-	defer rollbackPeriodLock.Unlock()
-
-	// Set the period
-	oldPeriod := rollbackPeriod
-
-	// Create and start a new cluster.
-	rollbackPeriod = newPeriod
-	cluster := NewTestCluster(t, base, opts)
-	cluster.Start()
-
-	// Reset the period
-	rollbackPeriod = oldPeriod
-
-	// Return the cluster.
-	return cluster
-}
-
 // MakeTestPluginDir creates a temporary directory suitable for holding plugins.
 // This helper also resolves symlinks to make tests happy on OS X.
 func MakeTestPluginDir(t testing.T) (string, func(t testing.T)) {