Minor refacotr on the e2e test framework

Also resolved the review comment in etcd-io#19352 Signed-off-by: Benjamin Wang <benjamin.ahrtr@gmail.com>
ahrtr · Feb 11, 2025 · e969f58 · e969f58
1 parent 53d44f0
commit e969f58
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 26 deletions.
diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go
@@ -457,10 +457,6 @@ func InitEtcdProcessCluster(t testing.TB, cfg *EtcdProcessClusterConfig) (*EtcdP
 		if cfg.Version == LastVersion && !CouldSetSnapshotCatchupEntries(BinPath.EtcdLastRelease) {
 			return nil, fmt.Errorf("cannot set SnapshotCatchUpEntries for last etcd version: %s", BinPath.EtcdLastRelease)
 		}
-		if cfg.Version != CurrentVersion && UsesExperimentalSnapshotCatchupEntriesFlag(BinPath.EtcdLastRelease) {
-			cfg.ServerConfig.ExperimentalSnapshotCatchUpEntries = cfg.ServerConfig.SnapshotCatchUpEntries
-			cfg.ServerConfig.SnapshotCatchUpEntries = etcdserver.DefaultSnapshotCatchUpEntries
-		}
 	}
 
 	etcdCfgs := cfg.EtcdAllServerProcessConfigs(t)
@@ -639,26 +635,13 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in
 		args = append(args, "--discovery="+cfg.Discovery)
 	}
 
-	var execPath string
-	switch cfg.Version {
-	case CurrentVersion:
-		execPath = BinPath.Etcd
-	case MinorityLastVersion:
-		if i <= cfg.ClusterSize/2 {
-			execPath = BinPath.Etcd
-		} else {
-			execPath = BinPath.EtcdLastRelease
-		}
-	case QuorumLastVersion:
-		if i <= cfg.ClusterSize/2 {
-			execPath = BinPath.EtcdLastRelease
-		} else {
-			execPath = BinPath.Etcd
+	execPath := cfg.binaryPath(i)
+
+	if cfg.ServerConfig.SnapshotCatchUpEntries != etcdserver.DefaultSnapshotCatchUpEntries {
+		if !IsSnapshotCatchupEntriesFlagAvailable(execPath) {
+			cfg.ServerConfig.ExperimentalSnapshotCatchUpEntries = cfg.ServerConfig.SnapshotCatchUpEntries
+			cfg.ServerConfig.SnapshotCatchUpEntries = etcdserver.DefaultSnapshotCatchUpEntries
 		}
-	case LastVersion:
-		execPath = BinPath.EtcdLastRelease
-	default:
-		panic(fmt.Sprintf("Unknown cluster version %v", cfg.Version))
 	}
 
 	defaultValues := values(*embed.NewConfig())
@@ -702,6 +685,32 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in
 	}
 }
 
+func (cfg *EtcdProcessClusterConfig) binaryPath(i int) string {
+	var execPath string
+	switch cfg.Version {
+	case CurrentVersion:
+		execPath = BinPath.Etcd
+	case MinorityLastVersion:
+		if i <= cfg.ClusterSize/2 {
+			execPath = BinPath.Etcd
+		} else {
+			execPath = BinPath.EtcdLastRelease
+		}
+	case QuorumLastVersion:
+		if i <= cfg.ClusterSize/2 {
+			execPath = BinPath.EtcdLastRelease
+		} else {
+			execPath = BinPath.Etcd
+		}
+	case LastVersion:
+		execPath = BinPath.EtcdLastRelease
+	default:
+		panic(fmt.Sprintf("Unknown cluster version %v", cfg.Version))
+	}
+
+	return execPath
+}
+
 func values(cfg embed.Config) map[string]string {
 	fs := flag.NewFlagSet("etcd", flag.ContinueOnError)
 	cfg.AddFlags(fs)

diff --git a/tests/framework/e2e/etcd_process.go b/tests/framework/e2e/etcd_process.go
@@ -31,6 +31,7 @@ import (
 	"github.com/coreos/go-semver/semver"
 	"go.uber.org/zap"
 
+	"go.etcd.io/etcd/api/v3/version"
 	"go.etcd.io/etcd/client/pkg/v3/fileutil"
 	"go.etcd.io/etcd/pkg/v3/expect"
 	"go.etcd.io/etcd/pkg/v3/proxy"
@@ -532,11 +533,10 @@ func CouldSetSnapshotCatchupEntries(execPath string) bool {
 	return v.Compare(v3_5_14) >= 0
 }
 
-func UsesExperimentalSnapshotCatchupEntriesFlag(execPath string) bool {
+func IsSnapshotCatchupEntriesFlagAvailable(execPath string) bool {
 	v, err := GetVersionFromBinary(execPath)
 	if err != nil {
 		return false
 	}
-	v3_6 := semver.Version{Major: 3, Minor: 6}
-	return v.LessThan(v3_6)
+	return !v.LessThan(version.V3_6)
 }