Move experimental flag from embed.Config to configJSON to normalize t…

…he config.

Signed-off-by: Marek Siarkowicz <[email protected]>

server/embed/config.go

@@ -388,11 +388,7 @@ type Config struct {
 	// Deprecated in v3.6.
 	// TODO: Delete in v3.7
 	ExperimentalEnableLeaseCheckpointPersist bool `json:"experimental-enable-lease-checkpoint-persist"`
-	// ExperimentalCompactionBatchLimit Sets the maximum revisions deleted in each compaction batch.
-	// Deprecated in v3.6 and will be decommissioned in v3.7.
-	// TODO: Delete in v3.7
-	ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit"`
-	CompactionBatchLimit             int `json:"compaction-batch-limit"`
+	CompactionBatchLimit                     int  `json:"compaction-batch-limit"`
 	// ExperimentalCompactionSleepInterval is the sleep interval between every etcd compaction loop.
 	ExperimentalCompactionSleepInterval     time.Duration `json:"experimental-compaction-sleep-interval"`
 	ExperimentalWatchProgressNotifyInterval time.Duration `json:"experimental-watch-progress-notify-interval"`
@@ -517,6 +513,10 @@ type configJSON struct {
 	PeerSecurityJSON   securityConfig `json:"peer-transport-security"`
 
 	ServerFeatureGatesJSON string `json:"feature-gates"`
+	// ExperimentalCompactionBatchLimit Sets the maximum revisions deleted in each compaction batch.
+	// Deprecated in v3.6 and will be decommissioned in v3.7.
+	// TODO: Delete in v3.7
+	ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit"`
 }
 
 type securityConfig struct {
@@ -794,7 +794,7 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
 	// TODO: delete in v3.7
 	fs.BoolVar(&cfg.ExperimentalEnableLeaseCheckpointPersist, "experimental-enable-lease-checkpoint-persist", false, "Enable persisting remainingTTL to prevent indefinite auto-renewal of long lived leases. Always enabled in v3.6. Should be used to ensure smooth upgrade from v3.5 clusters with this feature enabled. Requires experimental-enable-lease-checkpoint to be enabled.")
 	// TODO: delete in v3.7
-	fs.IntVar(&cfg.ExperimentalCompactionBatchLimit, "experimental-compaction-batch-limit", cfg.ExperimentalCompactionBatchLimit, "Sets the maximum revisions deleted in each compaction batch. Deprecated in v3.6 and will be decommissioned in v3.7. Use --compaction-batch-limit instead.")
+	fs.IntVar(&cfg.CompactionBatchLimit, "experimental-compaction-batch-limit", cfg.CompactionBatchLimit, "Sets the maximum revisions deleted in each compaction batch. Deprecated in v3.6 and will be decommissioned in v3.7. Use --compaction-batch-limit instead.")
 	fs.IntVar(&cfg.CompactionBatchLimit, "compaction-batch-limit", cfg.CompactionBatchLimit, "Sets the maximum revisions deleted in each compaction batch.")
 	fs.DurationVar(&cfg.ExperimentalCompactionSleepInterval, "experimental-compaction-sleep-interval", cfg.ExperimentalCompactionSleepInterval, "Sets the sleep interval between each compaction batch.")
 	fs.DurationVar(&cfg.ExperimentalWatchProgressNotifyInterval, "experimental-watch-progress-notify-interval", cfg.ExperimentalWatchProgressNotifyInterval, "Duration of periodic watch progress notifications.")
@@ -941,6 +941,13 @@ func (cfg *configYAML) configFromFile(path string) error {
 		cfg.ClusterState = ClusterStateFlagNew
 	}
 
+	if cfg.ExperimentalCompactionBatchLimit != 0 {
+		if cfg.CompactionBatchLimit != 0 {
+			return fmt.Errorf(`cannot set "experimentental-compaction-batch-limit" and "compaction-batch-limit" at the same time, please use "compaction-batch-limit" only`)
+		}
+		cfg.CompactionBatchLimit = cfg.ExperimentalCompactionBatchLimit
+	}
+
 	copySecurityDetails := func(tls *transport.TLSInfo, ysc *securityConfig) {
 		tls.CertFile = ysc.CertFile
 		tls.KeyFile = ysc.KeyFile

server/etcdmain/config.go

@@ -180,10 +180,6 @@ func (cfg *config) parse(arguments []string) error {
 		cfg.ec.CorruptCheckTime = cfg.ec.ExperimentalCorruptCheckTime
 	}
 
-	if cfg.ec.FlagsExplicitlySet["experimental-compaction-batch-limit"] {
-		cfg.ec.CompactionBatchLimit = cfg.ec.ExperimentalCompactionBatchLimit
-	}
-
 	// `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
 

tests/e2e/corrupt_test.go

@@ -329,22 +329,19 @@ func testCompactHashCheckDetectCorruptionInterrupt(t *testing.T, useFeatureGate
 	} else {
 		opts = append(opts, e2e.WithCompactHashCheckEnabled(true))
 	}
-	var compactionBatchLimit e2e.EPClusterOption
+	var flag string
 	if useExperimentalFlag {
-		compactionBatchLimit = e2e.WithExperimentalCompactionBatchLimit(1)
+		flag = "--experimental-compaction-batch-limit=1"
 	} else {
-		compactionBatchLimit = e2e.WithCompactionBatchLimit(1)
+		flag = "--compaction-batch-limit=1"
 	}
 
 	cfg := e2e.NewConfig(opts...)
 	epc, err := e2e.InitEtcdProcessCluster(t, cfg)
 	require.NoError(t, err)
 
 	// Assign a node a very slow compaction speed, so that its compaction can be interrupted.
-	err = epc.UpdateProcOptions(slowCompactionNodeIndex, t,
-		compactionBatchLimit,
-		e2e.WithCompactionSleepInterval(1*time.Hour),
-	)
+	err = epc.UpdateProcOptions(slowCompactionNodeIndex, t, flag)
 	require.NoError(t, err)
 
 	epc, err = e2e.StartEtcdProcessCluster(ctx, t, epc, cfg)

tests/framework/e2e/cluster.go

@@ -28,6 +28,7 @@ import (
 	"testing"
 	"time"
 
+	"go.etcd.io/etcd/api/v3/version"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zaptest"
 
@@ -376,10 +377,6 @@ func WithCompactionBatchLimit(limit int) EPClusterOption {
 	return func(c *EtcdProcessClusterConfig) { c.ServerConfig.CompactionBatchLimit = limit }
 }
 
-func WithExperimentalCompactionBatchLimit(limit int) EPClusterOption {
-	return func(c *EtcdProcessClusterConfig) { c.ServerConfig.ExperimentalCompactionBatchLimit = limit }
-}
-
 func WithCompactionSleepInterval(time time.Duration) EPClusterOption {
 	return func(c *EtcdProcessClusterConfig) { c.ServerConfig.ExperimentalCompactionSleepInterval = time }
 }
@@ -630,6 +627,10 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in
 	default:
 		panic(fmt.Sprintf("Unknown cluster version %v", cfg.Version))
 	}
+	v, err := GetVersionFromBinary(execPath)
+	if err != nil {
+		panic(err)
+	}
 
 	defaultValues := values(*embed.NewConfig())
 	overrideValues := values(cfg.ServerConfig)
@@ -640,6 +641,9 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in
 		if flag == "experimental-snapshot-catchup-entries" && !CouldSetSnapshotCatchupEntries(execPath) {
 			continue
 		}
+		if flag == "compaction-batch-limit" && v.LessThan(version.V3_6) {
+			flag = "experimental-compaction-batch-limit"
+		}
 		args = append(args, fmt.Sprintf("--%s=%s", flag, value))
 	}
 	envVars := map[string]string{}
@@ -881,16 +885,13 @@ func (epc *EtcdProcessCluster) StartNewProcFromConfig(ctx context.Context, tb te
 
 // UpdateProcOptions updates the options for a specific process. If no opt is set, then the config is identical
 // to the cluster.
-func (epc *EtcdProcessCluster) UpdateProcOptions(i int, tb testing.TB, opts ...EPClusterOption) error {
+func (epc *EtcdProcessCluster) UpdateProcOptions(i int, tb testing.TB, newflags ...string) error {
 	if epc.Procs[i].IsRunning() {
 		return fmt.Errorf("process %d is still running, please close it before updating its options", i)
 	}
 	cfg := *epc.Cfg
-	for _, opt := range opts {
-		opt(&cfg)
-	}
 	serverCfg := cfg.EtcdServerProcessConfig(tb, i)
-
+	serverCfg.Args = append(serverCfg.Args, newflags...)
 	var initialCluster []string
 	for _, p := range epc.Procs {
 		initialCluster = append(initialCluster, fmt.Sprintf("%s=%s", p.Config().Name, p.Config().PeerURL.String()))