Merge pull request #19156 from gangli113/experimentalFlag

migrate flag experimental-corrupt-check-time to use corrupt-check-time

Author: serathius

server/embed/config.go

@@ -134,6 +134,7 @@ var (
 	// TODO: delete in v3.7
 	experimentalNonBoolFlagMigrationMap = map[string]string{
 		"experimental-compact-hash-check-time": "compact-hash-check-time",
+		"experimental-corrupt-check-time":      "corrupt-check-time",
 	}
 )
 
@@ -363,8 +364,12 @@ type Config struct {
 	// AuthTokenTTL in seconds of the simple token
 	AuthTokenTTL uint `json:"auth-token-ttl"`
 
-	ExperimentalInitialCorruptCheck bool          `json:"experimental-initial-corrupt-check"`
-	ExperimentalCorruptCheckTime    time.Duration `json:"experimental-corrupt-check-time"`
+	ExperimentalInitialCorruptCheck bool `json:"experimental-initial-corrupt-check"`
+	// ExperimentalCorruptCheckTime is the duration of time between cluster corruption check passes.
+	// Deprecated in v3.6 and will be decommissioned in v3.7.
+	// TODO: delete in v3.7
+	ExperimentalCorruptCheckTime time.Duration `json:"experimental-corrupt-check-time"`
+	CorruptCheckTime             time.Duration `json:"corrupt-check-time"`
 	// ExperimentalCompactHashCheckEnabled enables leader to periodically check followers compaction hashes.
 	// Deprecated in v3.6 and will be decommissioned in v3.7.
 	// TODO: delete in v3.7
@@ -771,7 +776,9 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
 
 	// experimental
 	fs.BoolVar(&cfg.ExperimentalInitialCorruptCheck, "experimental-initial-corrupt-check", cfg.ExperimentalInitialCorruptCheck, "Enable to check data corruption before serving any client/peer traffic.")
-	fs.DurationVar(&cfg.ExperimentalCorruptCheckTime, "experimental-corrupt-check-time", cfg.ExperimentalCorruptCheckTime, "Duration of time between cluster corruption check passes.")
+	// TODO: delete in v3.7
+	fs.DurationVar(&cfg.ExperimentalCorruptCheckTime, "experimental-corrupt-check-time", cfg.ExperimentalCorruptCheckTime, "Duration of time between cluster corruption check passes. Deprecated in v3.6 and will be decommissioned in v3.7. Use --corrupt-check-time instead")
+	fs.DurationVar(&cfg.CorruptCheckTime, "corrupt-check-time", cfg.CorruptCheckTime, "Duration of time between cluster corruption check passes.")
 	// TODO: delete in v3.7
 	fs.BoolVar(&cfg.ExperimentalCompactHashCheckEnabled, "experimental-compact-hash-check-enabled", cfg.ExperimentalCompactHashCheckEnabled, "Enable leader to periodically check followers compaction hashes. Deprecated in v3.6 and will be decommissioned in v3.7. Use '--feature-gates=CompactHashCheck=true' instead")
 	fs.DurationVar(&cfg.ExperimentalCompactHashCheckTime, "experimental-compact-hash-check-time", cfg.ExperimentalCompactHashCheckTime, "Duration of time between leader checks followers compaction hashes. Deprecated in v3.6 and will be decommissioned in v3.7. Use --compact-hash-check-time instead.")

server/embed/etcd.go

@@ -205,7 +205,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
 		TokenTTL:                             cfg.AuthTokenTTL,
 		CORS:                                 cfg.CORS,
 		HostWhitelist:                        cfg.HostWhitelist,
-		CorruptCheckTime:                     cfg.ExperimentalCorruptCheckTime,
+		CorruptCheckTime:                     cfg.CorruptCheckTime,
 		CompactHashCheckTime:                 cfg.CompactHashCheckTime,
 		PreVote:                              cfg.PreVote,
 		Logger:                               cfg.logger,

server/etcdmain/config.go

@@ -65,6 +65,7 @@ var (
 		"experimental-compact-hash-check-enabled":        "--experimental-compact-hash-check-enabled is deprecated in 3.6 and will be decommissioned in 3.7. Use '--feature-gates=CompactHashCheck=true' instead.",
 		"experimental-compact-hash-check-time":           "--experimental-compact-hash-check-time is deprecated in 3.6 and will be decommissioned in 3.7. Use '--compact-hash-check-time' instead.",
 		"experimental-txn-mode-write-with-shared-buffer": "--experimental-txn-mode-write-with-shared-buffer is deprecated in v3.6 and will be decommissioned in v3.7. Use '--feature-gates=TxnModeWriteWithSharedBuffer=true' instead.",
+		"experimental-corrupt-check-time":                "--experimental-corrupt-check-time is deprecated in v3.6 and will be decommissioned in v3.7. Use '--corrupt-check-time' instead.",
 	}
 )
 
@@ -174,6 +175,10 @@ func (cfg *config) parse(arguments []string) error {
 		cfg.ec.CompactHashCheckTime = cfg.ec.ExperimentalCompactHashCheckTime
 	}
 
+	if cfg.ec.FlagsExplicitlySet["experimental-corrupt-check-time"] {
+		cfg.ec.CorruptCheckTime = cfg.ec.ExperimentalCorruptCheckTime
+	}
+
 	// `V2Deprecation` (--v2-deprecation) is deprecated and scheduled for removal in v3.8. The default value is enforced, ignoring user input.
 	cfg.ec.V2Deprecation = cconfig.V2DeprDefault
 

server/etcdmain/config_test.go

@@ -570,6 +570,95 @@ func TestCompactHashCheckTimeFlagMigration(t *testing.T) {
 	}
 }
 
+// TestCorruptCheckTimeFlagMigration tests the migration from
+// --experimental-corrupt-check-time to --corrupt-check-time
+// TODO: delete in v3.7
+func TestCorruptCheckTimeFlagMigration(t *testing.T) {
+	testCases := []struct {
+		name                         string
+		corruptCheckTime             string
+		experimentalCorruptCheckTime string
+		useConfigFile                bool
+		expectErr                    bool
+		expectedCorruptCheckTime     time.Duration
+	}{
+		{
+			name:                         "cannot set both experimental flag and non experimental flag",
+			corruptCheckTime:             "2m",
+			experimentalCorruptCheckTime: "3m",
+			expectErr:                    true,
+		},
+		{
+			name:                         "can set experimental flag",
+			experimentalCorruptCheckTime: "3m",
+			expectedCorruptCheckTime:     3 * time.Minute,
+		},
+		{
+			name:                     "can set non experimental flag",
+			corruptCheckTime:         "2m",
+			expectedCorruptCheckTime: 2 * time.Minute,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			cmdLineArgs := []string{}
+			yc := struct {
+				ExperimentalCorruptCheckTime time.Duration `json:"experimental-corrupt-check-time,omitempty"`
+				CorruptCheckTime             time.Duration `json:"corrupt-check-time,omitempty"`
+			}{}
+
+			if tc.corruptCheckTime != "" {
+				cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--corrupt-check-time=%s", tc.corruptCheckTime))
+				corruptCheckTime, err := time.ParseDuration(tc.corruptCheckTime)
+				if err != nil {
+					t.Fatal(err)
+				}
+				yc.CorruptCheckTime = corruptCheckTime
+			}
+
+			if tc.experimentalCorruptCheckTime != "" {
+				cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-corrupt-check-time=%s", tc.experimentalCorruptCheckTime))
+				experimentalCorruptCheckTime, err := time.ParseDuration(tc.experimentalCorruptCheckTime)
+				if err != nil {
+					t.Fatal(err)
+				}
+				yc.ExperimentalCorruptCheckTime = experimentalCorruptCheckTime
+			}
+
+			b, err := yaml.Marshal(&yc)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			tmpfile := mustCreateCfgFile(t, b)
+			defer os.Remove(tmpfile.Name())
+
+			cfgFromCmdLine := newConfig()
+			errFromCmdLine := cfgFromCmdLine.parse(cmdLineArgs)
+
+			cfgFromFile := newConfig()
+			errFromFile := cfgFromFile.parse([]string{fmt.Sprintf("--config-file=%s", tmpfile.Name())})
+
+			if tc.expectErr {
+				if errFromCmdLine == nil || errFromFile == nil {
+					t.Fatal("expect parse error")
+				}
+				return
+			}
+			if errFromCmdLine != nil || errFromFile != nil {
+				t.Fatal(err)
+			}
+
+			if cfgFromCmdLine.ec.CorruptCheckTime != tc.expectedCorruptCheckTime {
+				t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCorruptCheckTime, cfgFromCmdLine.ec.CorruptCheckTime)
+			}
+			if cfgFromFile.ec.CorruptCheckTime != tc.expectedCorruptCheckTime {
+				t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCorruptCheckTime, cfgFromFile.ec.CorruptCheckTime)
+			}
+		})
+	}
+}
+
 func mustCreateCfgFile(t *testing.T, b []byte) *os.File {
 	tmpfile, err := os.CreateTemp("", "servercfg")
 	if err != nil {
@@ -663,6 +752,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 		ExperimentalCompactHashCheckEnabled     bool          `json:"experimental-compact-hash-check-enabled,omitempty"`
 		ExperimentalCompactHashCheckTime        time.Duration `json:"experimental-compact-hash-check-time,omitempty"`
 		ExperimentalWarningUnaryRequestDuration time.Duration `json:"experimental-warning-unary-request-duration,omitempty"`
+		ExperimentalCorruptCheckTime            time.Duration `json:"experimental-corrupt-check-time,omitempty"`
 	}
 
 	testCases := []struct {
@@ -681,10 +771,12 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 				ExperimentalCompactHashCheckEnabled:     true,
 				ExperimentalCompactHashCheckTime:        2 * time.Minute,
 				ExperimentalWarningUnaryRequestDuration: time.Second,
+				ExperimentalCorruptCheckTime:            time.Minute,
 			},
 			expectedFlags: map[string]struct{}{
 				"experimental-compact-hash-check-enabled": {},
 				"experimental-compact-hash-check-time":    {},
+				"experimental-corrupt-check-time":         {},
 			},
 		},
 		{

server/etcdmain/help.go

@@ -276,6 +276,8 @@ Experimental feature:
   --experimental-initial-corrupt-check 'false'. It's deprecated, and will be decommissioned in v3.7. Use '--feature-gates=InitialCorruptCheck=true' instead.
     Enable to check data corruption before serving any client/peer traffic.
   --experimental-corrupt-check-time '0s'
+    Duration of time between cluster corruption check passes. Deprecated in v3.6 and will be decommissioned in v3.7. Use 'corrupt-check-time' instead.
+  --corrupt-check-time '0s'
     Duration of time between cluster corruption check passes.
   --experimental-compact-hash-check-enabled 'false'. Deprecated in v3.6 and will be decommissioned in v3.7. Use '--feature-gates=CompactHashCheck=true' instead.
     Enable leader to periodically check followers compaction hashes.

tests/e2e/corrupt_test.go

@@ -188,13 +188,27 @@ func TestInPlaceRecovery(t *testing.T) {
 }
 
 func TestPeriodicCheckDetectsCorruption(t *testing.T) {
+	testPeriodicCheckDetectsCorruption(t, false)
+}
+
+func TestPeriodicCheckDetectsCorruptionWithExperimentalFlag(t *testing.T) {
+	testPeriodicCheckDetectsCorruption(t, true)
+}
+
+func testPeriodicCheckDetectsCorruption(t *testing.T, useExperimentalFlag bool) {
 	checkTime := time.Second
 	e2e.BeforeTest(t)
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
+	var corruptCheckTime e2e.EPClusterOption
+	if useExperimentalFlag {
+		corruptCheckTime = e2e.WithExperimentalCorruptCheckTime(time.Second)
+	} else {
+		corruptCheckTime = e2e.WithCorruptCheckTime(time.Second)
+	}
 	epc, err := e2e.NewEtcdProcessCluster(ctx, t,
 		e2e.WithKeepDataDir(true),
-		e2e.WithCorruptCheckTime(time.Second),
+		corruptCheckTime,
 	)
 	if err != nil {
 		t.Fatalf("could not start etcd process cluster (%v)", err)

tests/framework/e2e/cluster.go

@@ -313,6 +313,10 @@ func WithLogLevel(level string) EPClusterOption {
 }
 
 func WithCorruptCheckTime(time time.Duration) EPClusterOption {
+	return func(c *EtcdProcessClusterConfig) { c.ServerConfig.CorruptCheckTime = time }
+}
+
+func WithExperimentalCorruptCheckTime(time time.Duration) EPClusterOption {
 	return func(c *EtcdProcessClusterConfig) { c.ServerConfig.ExperimentalCorruptCheckTime = time }
 }