From 4d0a5fa2750eb374138845b04bf3d62549c78352 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Fri, 17 Jan 2025 12:17:36 +0100 Subject: [PATCH] Move experimental flag from embed.Config to configJSON to normalize the config. Signed-off-by: Marek Siarkowicz --- server/embed/config.go | 19 +++++++++++++------ server/etcdmain/config.go | 4 ---- tests/e2e/corrupt_test.go | 11 ++++------- tests/framework/e2e/cluster.go | 19 ++++++++++--------- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/server/embed/config.go b/server/embed/config.go index a66adf97c2b..375326ea8ba 100644 --- a/server/embed/config.go +++ b/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 diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go index 99cc279a1be..0a0aa80f2e4 100644 --- a/server/etcdmain/config.go +++ b/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 diff --git a/tests/e2e/corrupt_test.go b/tests/e2e/corrupt_test.go index 86535345e99..4b28681a7e0 100644 --- a/tests/e2e/corrupt_test.go +++ b/tests/e2e/corrupt_test.go @@ -329,11 +329,11 @@ 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...) @@ -341,10 +341,7 @@ func testCompactHashCheckDetectCorruptionInterrupt(t *testing.T, useFeatureGate 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) diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go index 3da0ff9a8f2..2f11238be4c 100644 --- a/tests/framework/e2e/cluster.go +++ b/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()))