From 938345cef4b39b791c5fbee737459a7d4404e93e Mon Sep 17 00:00:00 2001 From: Kaviraj Date: Sun, 2 Mar 2025 14:05:02 +0100 Subject: [PATCH] robustness-test: Migrate `experimental-compaction-batch-limit` flag Fixes: https://github.com/etcd-io/etcd/issues/19354 Robustness run different versions of binaries. This flag is experiemental in v3.4 and v3.5. But non-experiemental in v3.6+. This PR adds ability to run different binaries with it's canonical flag name. In robustness tests we pass these config via `embed.ServerConfig` struct and get converted into CLI flags before spawning the etcd server binary. This PR maps into right canonical flag based on versions. So even the experimental field is removed from the struct (in v3.7), the flags will be correct mapped. Signed-off-by: Kaviraj --- tests/framework/e2e/cluster.go | 26 ++++++++++++++++++++++---- tests/framework/e2e/cluster_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go index 5af56407239..1b8168a1036 100644 --- a/tests/framework/e2e/cluster.go +++ b/tests/framework/e2e/cluster.go @@ -29,6 +29,7 @@ import ( "time" "github.com/coreos/go-semver/semver" + "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zaptest" @@ -645,8 +646,11 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in } } - defaultValues := values(*embed.NewConfig()) - overrideValues := values(cfg.ServerConfig) + version, err := GetVersionFromBinary(execPath) + require.NoError(tb, err) + + defaultValues := values(version, *embed.NewConfig()) + overrideValues := values(version, cfg.ServerConfig) for flag, value := range overrideValues { if defaultValue := defaultValues[flag]; value == "" || value == defaultValue { continue @@ -727,16 +731,30 @@ func (epc *EtcdProcessCluster) MinServerVersion() (*semver.Version, error) { return minVersion, nil } -func values(cfg embed.Config) map[string]string { +// canonicalFlagName takes any flag name and returns it's canonical name +// handling the experimental flags in different versions +func canonicalFlagName(version *semver.Version, name string) string { + v3_6 := semver.Version{Major: 3, Minor: 6} + if name == "compaction-batch-limit" { + if version.Compare(v3_6) < 0 { + name = "experimental-compaction-batch-limit" + } + } + return name +} + +func values(version *semver.Version, cfg embed.Config) map[string]string { fs := flag.NewFlagSet("etcd", flag.ContinueOnError) + cfg.AddFlags(fs) values := map[string]string{} fs.VisitAll(func(f *flag.Flag) { + name := canonicalFlagName(version, f.Name) value := f.Value.String() if value == "false" || value == "0" { value = "" } - values[f.Name] = value + values[name] = value }) return values } diff --git a/tests/framework/e2e/cluster_test.go b/tests/framework/e2e/cluster_test.go index 24fa8a91bc6..de3faa3ae1d 100644 --- a/tests/framework/e2e/cluster_test.go +++ b/tests/framework/e2e/cluster_test.go @@ -25,6 +25,11 @@ import ( func TestEtcdServerProcessConfig(t *testing.T) { v3_5_12 := semver.Version{Major: 3, Minor: 5, Patch: 12} v3_5_14 := semver.Version{Major: 3, Minor: 5, Patch: 14} + + v3_4 := semver.Version{Major: 3, Minor: 4} + v3_5 := semver.Version{Major: 3, Minor: 5} + v3_6 := semver.Version{Major: 3, Minor: 6} + tcs := []struct { name string config *EtcdProcessClusterConfig @@ -107,6 +112,30 @@ func TestEtcdServerProcessConfig(t *testing.T) { "--listen-client-http-urls=http://localhost:4", }, }, + { + name: "CompactionBatchLimit_v3.6", + config: NewConfig(WithCompactionBatchLimit(10)), + expectArgsContain: []string{ + "--compaction-batch-limit", + }, + mockBinaryVersion: &v3_6, + }, + { + name: "CompactionBatchLimit_v3.5", + config: NewConfig(WithCompactionBatchLimit(10)), + expectArgsContain: []string{ + "--experimental-compaction-batch-limit", + }, + mockBinaryVersion: &v3_5, + }, + { + name: "CompactionBatchLimit_v3.4", + config: NewConfig(WithCompactionBatchLimit(10)), + expectArgsContain: []string{ + "--experimental-compaction-batch-limit", + }, + mockBinaryVersion: &v3_4, + }, { name: "ForceNewCluster", config: NewConfig(WithForceNewCluster(true)),