Merge pull request #16707 from serathius/dynamic-flags

Dynamically generate flags passed to etcd binary
etcd-io · Oct 12, 2023 · 6d68ab0 · 6d68ab0
2 parents 2c86103 + c34ccfb
commit 6d68ab0
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 72 deletions.
diff --git a/tests/e2e/cmux_test.go b/tests/e2e/cmux_test.go
@@ -70,8 +70,10 @@ func TestConnectionMultiplexing(t *testing.T) {
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			ctx := context.Background()
-			cfg := e2e.EtcdProcessClusterConfig{ClusterSize: 1, Client: e2e.ClientConfig{ConnectionType: tc.serverTLS}, ClientHttpSeparate: tc.separateHttpPort}
-			clus, err := e2e.NewEtcdProcessCluster(ctx, t, e2e.WithConfig(&cfg))
+			cfg := e2e.NewConfig(e2e.WithClusterSize(1))
+			cfg.Client.ConnectionType = tc.serverTLS
+			cfg.ClientHttpSeparate = tc.separateHttpPort
+			clus, err := e2e.NewEtcdProcessCluster(ctx, t, e2e.WithConfig(cfg))
 			require.NoError(t, err)
 			defer clus.Close()
 

diff --git a/tests/e2e/watch_delay_test.go b/tests/e2e/watch_delay_test.go
@@ -39,10 +39,11 @@ const (
 )
 
 type testCase struct {
-	name          string
-	config        e2e.EtcdProcessClusterConfig
-	maxWatchDelay time.Duration
-	dbSizeBytes   int
+	name                string
+	client              e2e.ClientConfig
+	clientHttpSerparate bool
+	maxWatchDelay       time.Duration
+	dbSizeBytes         int
 }
 
 const (
@@ -56,40 +57,44 @@ const (
 var tcs = []testCase{
 	{
 		name:          "NoTLS",
-		config:        e2e.EtcdProcessClusterConfig{ClusterSize: 1},
 		maxWatchDelay: 150 * time.Millisecond,
 		dbSizeBytes:   5 * Mega,
 	},
 	{
 		name:          "TLS",
-		config:        e2e.EtcdProcessClusterConfig{ClusterSize: 1, Client: e2e.ClientConfig{ConnectionType: e2e.ClientTLS}},
+		client:        e2e.ClientConfig{ConnectionType: e2e.ClientTLS},
 		maxWatchDelay: 150 * time.Millisecond,
 		dbSizeBytes:   5 * Mega,
 	},
 	{
-		name:          "SeparateHttpNoTLS",
-		config:        e2e.EtcdProcessClusterConfig{ClusterSize: 1, ClientHttpSeparate: true},
-		maxWatchDelay: 150 * time.Millisecond,
-		dbSizeBytes:   5 * Mega,
+		name:                "SeparateHttpNoTLS",
+		clientHttpSerparate: true,
+		maxWatchDelay:       150 * time.Millisecond,
+		dbSizeBytes:         5 * Mega,
 	},
 	{
-		name:          "SeparateHttpTLS",
-		config:        e2e.EtcdProcessClusterConfig{ClusterSize: 1, Client: e2e.ClientConfig{ConnectionType: e2e.ClientTLS}, ClientHttpSeparate: true},
-		maxWatchDelay: 150 * time.Millisecond,
-		dbSizeBytes:   5 * Mega,
+		name:                "SeparateHttpTLS",
+		client:              e2e.ClientConfig{ConnectionType: e2e.ClientTLS},
+		clientHttpSerparate: true,
+		maxWatchDelay:       150 * time.Millisecond,
+		dbSizeBytes:         5 * Mega,
 	},
 }
 
 func TestWatchDelayForPeriodicProgressNotification(t *testing.T) {
 	e2e.BeforeTest(t)
 	for _, tc := range tcs {
 		tc := tc
-		tc.config.ServerConfig.ExperimentalWatchProgressNotifyInterval = watchResponsePeriod
+		cfg := e2e.DefaultConfig()
+		cfg.ClusterSize = 1
+		cfg.ServerConfig.ExperimentalWatchProgressNotifyInterval = watchResponsePeriod
+		cfg.Client = tc.client
+		cfg.ClientHttpSeparate = tc.clientHttpSerparate
 		t.Run(tc.name, func(t *testing.T) {
-			clus, err := e2e.NewEtcdProcessCluster(context.Background(), t, e2e.WithConfig(&tc.config))
+			clus, err := e2e.NewEtcdProcessCluster(context.Background(), t, e2e.WithConfig(cfg))
 			require.NoError(t, err)
 			defer clus.Close()
-			c := newClient(t, clus.EndpointsGRPC(), tc.config.Client)
+			c := newClient(t, clus.EndpointsGRPC(), tc.client)
 			require.NoError(t, fillEtcdWithData(context.Background(), c, tc.dbSizeBytes))
 
 			ctx, cancel := context.WithTimeout(context.Background(), watchTestDuration)
@@ -105,11 +110,16 @@ func TestWatchDelayForPeriodicProgressNotification(t *testing.T) {
 func TestWatchDelayForManualProgressNotification(t *testing.T) {
 	e2e.BeforeTest(t)
 	for _, tc := range tcs {
+		tc := tc
+		cfg := e2e.DefaultConfig()
+		cfg.ClusterSize = 1
+		cfg.Client = tc.client
+		cfg.ClientHttpSeparate = tc.clientHttpSerparate
 		t.Run(tc.name, func(t *testing.T) {
-			clus, err := e2e.NewEtcdProcessCluster(context.Background(), t, e2e.WithConfig(&tc.config))
+			clus, err := e2e.NewEtcdProcessCluster(context.Background(), t, e2e.WithConfig(cfg))
 			require.NoError(t, err)
 			defer clus.Close()
-			c := newClient(t, clus.EndpointsGRPC(), tc.config.Client)
+			c := newClient(t, clus.EndpointsGRPC(), tc.client)
 			require.NoError(t, fillEtcdWithData(context.Background(), c, tc.dbSizeBytes))
 
 			ctx, cancel := context.WithTimeout(context.Background(), watchTestDuration)
@@ -137,11 +147,16 @@ func TestWatchDelayForManualProgressNotification(t *testing.T) {
 func TestWatchDelayForEvent(t *testing.T) {
 	e2e.BeforeTest(t)
 	for _, tc := range tcs {
+		tc := tc
+		cfg := e2e.DefaultConfig()
+		cfg.ClusterSize = 1
+		cfg.Client = tc.client
+		cfg.ClientHttpSeparate = tc.clientHttpSerparate
 		t.Run(tc.name, func(t *testing.T) {
-			clus, err := e2e.NewEtcdProcessCluster(context.Background(), t, e2e.WithConfig(&tc.config))
+			clus, err := e2e.NewEtcdProcessCluster(context.Background(), t, e2e.WithConfig(cfg))
 			require.NoError(t, err)
 			defer clus.Close()
-			c := newClient(t, clus.EndpointsGRPC(), tc.config.Client)
+			c := newClient(t, clus.EndpointsGRPC(), tc.client)
 			require.NoError(t, fillEtcdWithData(context.Background(), c, tc.dbSizeBytes))
 
 			ctx, cancel := context.WithTimeout(context.Background(), watchTestDuration)

diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go
@@ -17,6 +17,7 @@ package e2e
 import (
 	"context"
 	"errors"
+	"flag"
 	"fmt"
 	"net/url"
 	"path"
@@ -30,10 +31,8 @@ import (
 	"go.uber.org/zap/zaptest"
 
 	"go.etcd.io/etcd/api/v3/etcdserverpb"
-	"go.etcd.io/etcd/client/pkg/v3/logutil"
 	clientv3 "go.etcd.io/etcd/client/v3"
 	"go.etcd.io/etcd/pkg/v3/proxy"
-	config2 "go.etcd.io/etcd/server/v3/config"
 	"go.etcd.io/etcd/server/v3/embed"
 	"go.etcd.io/etcd/server/v3/etcdserver"
 	"go.etcd.io/etcd/tests/v3/framework/config"
@@ -538,9 +537,6 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in
 	if cfg.EnableV2 {
 		args = append(args, "--enable-v2")
 	}
-	if cfg.ServerConfig.ExperimentalInitialCorruptCheck {
-		args = append(args, "--experimental-initial-corrupt-check")
-	}
 	var murl string
 	if cfg.MetricsURLScheme != "" {
 		murl = (&url.URL{
@@ -552,54 +548,20 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in
 
 	args = append(args, cfg.TlsArgs()...)
 
-	if cfg.ServerConfig.AuthToken != "" && cfg.ServerConfig.AuthToken != embed.DefaultAuthToken {
-		args = append(args, "--auth-token="+cfg.ServerConfig.AuthToken)
-	}
-
-	if cfg.ServerConfig.V2Deprecation != "" && cfg.ServerConfig.V2Deprecation != config2.V2_DEPR_DEFAULT {
-		args = append(args, "--v2-deprecation="+string(cfg.ServerConfig.V2Deprecation))
-	}
-
 	if cfg.Discovery != "" {
 		args = append(args, "--discovery="+cfg.Discovery)
 	}
 
-	if cfg.ServerConfig.LogLevel != "" && cfg.ServerConfig.LogLevel != logutil.DefaultLogLevel {
-		args = append(args, "--log-level="+cfg.ServerConfig.LogLevel)
-	}
-
-	if cfg.ServerConfig.MaxConcurrentStreams != 0 && cfg.ServerConfig.MaxConcurrentStreams != embed.DefaultMaxConcurrentStreams {
-		args = append(args, "--max-concurrent-streams="+fmt.Sprintf("%d", cfg.ServerConfig.MaxConcurrentStreams))
-	}
-
-	if cfg.ServerConfig.ExperimentalCorruptCheckTime != 0 {
-		args = append(args, "--experimental-corrupt-check-time="+fmt.Sprintf("%s", cfg.ServerConfig.ExperimentalCorruptCheckTime))
-	}
-	if cfg.ServerConfig.ExperimentalCompactHashCheckEnabled {
-		args = append(args, "--experimental-compact-hash-check-enabled")
-	}
-	if cfg.ServerConfig.ExperimentalCompactHashCheckTime != 0 && cfg.ServerConfig.ExperimentalCompactHashCheckTime != embed.DefaultExperimentalCompactHashCheckTime {
-		args = append(args, "--experimental-compact-hash-check-time="+cfg.ServerConfig.ExperimentalCompactHashCheckTime.String())
-	}
-	if cfg.ServerConfig.ExperimentalCompactionBatchLimit != 0 {
-		args = append(args, "--experimental-compaction-batch-limit="+fmt.Sprintf("%d", cfg.ServerConfig.ExperimentalCompactionBatchLimit))
-	}
-	if cfg.ServerConfig.ExperimentalCompactionSleepInterval != 0 {
-		args = append(args, "--experimental-compaction-sleep-interval="+cfg.ServerConfig.ExperimentalCompactionSleepInterval.String())
-	}
-	if cfg.ServerConfig.WarningUnaryRequestDuration != 0 {
-		args = append(args, "--warning-unary-request-duration="+cfg.ServerConfig.WarningUnaryRequestDuration.String())
-	}
-	if cfg.ServerConfig.ExperimentalWarningUnaryRequestDuration != 0 {
-		args = append(args, "--experimental-warning-unary-request-duration="+cfg.ServerConfig.ExperimentalWarningUnaryRequestDuration.String())
-	}
-	if cfg.ServerConfig.ExperimentalWatchProgressNotifyInterval != 0 {
-		args = append(args, "--experimental-watch-progress-notify-interval="+cfg.ServerConfig.ExperimentalWatchProgressNotifyInterval.String())
-	}
-	if cfg.ServerConfig.SnapshotCatchUpEntries != 0 && cfg.ServerConfig.SnapshotCatchUpEntries != etcdserver.DefaultSnapshotCatchUpEntries {
-		if cfg.Version == CurrentVersion || (cfg.Version == MinorityLastVersion && i <= cfg.ClusterSize/2) || (cfg.Version == QuorumLastVersion && i > cfg.ClusterSize/2) {
-			args = append(args, "--experimental-snapshot-catchup-entries="+fmt.Sprintf("%d", cfg.ServerConfig.SnapshotCatchUpEntries))
+	defaultValues := values(*embed.NewConfig())
+	overrideValues := values(cfg.ServerConfig)
+	for flag, value := range overrideValues {
+		if defaultValue := defaultValues[flag]; value == "" || value == defaultValue {
+			continue
 		}
+		if flag == "experimental-snapshot-catchup-entries" && !(cfg.Version == CurrentVersion || (cfg.Version == MinorityLastVersion && i <= cfg.ClusterSize/2) || (cfg.Version == QuorumLastVersion && i > cfg.ClusterSize/2)) {
+			continue
+		}
+		args = append(args, fmt.Sprintf("--%s=%s", flag, value))
 	}
 	envVars := map[string]string{}
 	for key, value := range cfg.EnvVars {
@@ -654,6 +616,20 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in
 	}
 }
 
+func values(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) {
+		value := f.Value.String()
+		if value == "false" || value == "0" {
+			value = ""
+		}
+		values[f.Name] = value
+	})
+	return values
+}
+
 func clientURL(scheme string, port int, connType ClientConnType) string {
 	curlHost := fmt.Sprintf("localhost:%d", port)
 	switch connType {

diff --git a/tests/framework/e2e/cluster_test.go b/tests/framework/e2e/cluster_test.go
@@ -57,7 +57,7 @@ func TestEtcdServerProcessConfig(t *testing.T) {
 			name:   "CorruptCheck",
 			config: NewConfig(WithInitialCorruptCheck(true)),
 			expectArgsContain: []string{
-				"--experimental-initial-corrupt-check",
+				"--experimental-initial-corrupt-check=true",
 			},
 		},
 		{