From 4f33d5bcaf6f6d9d05357b0988aed3d0c8c46604 Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Sun, 26 Jun 2022 16:20:17 +0800 Subject: [PATCH 1/3] [3.5]backport: pkg/expect: send SIGTERM to target expect process instead of SIGKILL Signed-off-by: Eval EXEC Signed-off-by: ArkaSaha30 --- pkg/expect/expect.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/expect/expect.go b/pkg/expect/expect.go index 95bc30823f2..dd4683d3958 100644 --- a/pkg/expect/expect.go +++ b/pkg/expect/expect.go @@ -42,7 +42,7 @@ type ExpectProcess struct { count int // increment whenever new line gets added err error - // StopSignal is the signal Stop sends to the process; defaults to SIGKILL. + // StopSignal is the signal Stop sends to the process; defaults to SIGTERM. StopSignal os.Signal } @@ -58,7 +58,7 @@ func NewExpectWithEnv(name string, args []string, env []string) (ep *ExpectProce cmd.Env = env ep = &ExpectProcess{ cmd: cmd, - StopSignal: syscall.SIGKILL, + StopSignal: syscall.SIGTERM, } ep.cmd.Stderr = ep.cmd.Stdout ep.cmd.Stdin = nil From ea94399e3f42db77adec05402dbf77102d0a320f Mon Sep 17 00:00:00 2001 From: Thomas Jungblut Date: Thu, 28 Jul 2022 14:38:44 +0200 Subject: [PATCH 2/3] [3.5]backport: Reduce ExpectFunc polling interval. Fixes issue 14275, flakes close to timeout like TestKVDelete. Signed-off-by: Thomas Jungblut Signed-off-by: ArkaSaha30 --- pkg/expect/expect.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/expect/expect.go b/pkg/expect/expect.go index dd4683d3958..1d51eda6b8c 100644 --- a/pkg/expect/expect.go +++ b/pkg/expect/expect.go @@ -114,7 +114,7 @@ func (ep *ExpectProcess) ExpectFunc(f func(string) bool) (string, error) { break } ep.mu.Unlock() - time.Sleep(time.Millisecond * 100) + time.Sleep(time.Millisecond * 10) } ep.mu.Lock() lastLinesIndex := len(ep.lines) - DEBUG_LINES_TAIL From a37b082967edc7730b4dc0d69fa7d56ec814f325 Mon Sep 17 00:00:00 2001 From: Thomas Jungblut Date: Mon, 1 Aug 2022 15:53:30 +0200 Subject: [PATCH 3/3] [3.5]backport: Add test name to e2e cluster members This should aid in debugging test flakes, especially in tests where the process is restarted very often and thus changes its pid. Now it's a lot easier to grep for different members, also when different tests fail at the same time. The test TestDowngradeUpgradeClusterOf3 as mentioned in #13167 is a good example for that. Signed-off-by: Thomas Jungblut --- pkg/expect/expect.go | 15 +++-- tests/framework/e2e/cluster.go | 74 +++++++++++++------------ tests/framework/e2e/cluster_proxy.go | 8 ++- tests/framework/e2e/etcd_process.go | 15 ++--- tests/framework/e2e/etcd_spawn_nocov.go | 25 +++++---- tests/framework/e2e/v2v3.go | 16 +++++- 6 files changed, 94 insertions(+), 59 deletions(-) diff --git a/pkg/expect/expect.go b/pkg/expect/expect.go index 1d51eda6b8c..1827270b5a1 100644 --- a/pkg/expect/expect.go +++ b/pkg/expect/expect.go @@ -33,6 +33,8 @@ import ( const DEBUG_LINES_TAIL = 40 type ExpectProcess struct { + name string + cmd *exec.Cmd fpty *os.File wg sync.WaitGroup @@ -48,15 +50,16 @@ type ExpectProcess struct { // NewExpect creates a new process for expect testing. func NewExpect(name string, arg ...string) (ep *ExpectProcess, err error) { - // if env[] is nil, use current system env - return NewExpectWithEnv(name, arg, nil) + // if env[] is nil, use current system env and the default command as name + return NewExpectWithEnv(name, arg, nil, name) } // NewExpectWithEnv creates a new process with user defined env variables for expect testing. -func NewExpectWithEnv(name string, args []string, env []string) (ep *ExpectProcess, err error) { +func NewExpectWithEnv(name string, args []string, env []string, serverProcessConfigName string) (ep *ExpectProcess, err error) { cmd := exec.Command(name, args...) cmd.Env = env ep = &ExpectProcess{ + name: serverProcessConfigName, cmd: cmd, StopSignal: syscall.SIGTERM, } @@ -72,6 +75,10 @@ func NewExpectWithEnv(name string, args []string, env []string) (ep *ExpectProce return ep, nil } +func (ep *ExpectProcess) Pid() int { + return ep.cmd.Process.Pid +} + func (ep *ExpectProcess) read() { defer ep.wg.Done() printDebugLines := os.Getenv("EXPECT_DEBUG") != "" @@ -81,7 +88,7 @@ func (ep *ExpectProcess) read() { ep.mu.Lock() if l != "" { if printDebugLines { - fmt.Printf("%s-%d: %s", ep.cmd.Path, ep.cmd.Process.Pid, l) + fmt.Printf("%s (%s) (%d): %s", ep.cmd.Path, ep.name, ep.cmd.Process.Pid, l) } ep.lines = append(ep.lines, l) ep.count++ diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go index 250900bb380..de3028b94ff 100644 --- a/tests/framework/e2e/cluster.go +++ b/tests/framework/e2e/cluster.go @@ -20,14 +20,15 @@ import ( "net/url" "os" "path" + "regexp" "strings" "testing" "time" - "go.etcd.io/etcd/pkg/v3/proxy" - "go.etcd.io/etcd/server/v3/etcdserver" "go.uber.org/zap" "go.uber.org/zap/zaptest" + + "go.etcd.io/etcd/server/v3/etcdserver" ) const EtcdProcessBasePort = 20000 @@ -40,6 +41,9 @@ const ( ClientTLSAndNonTLS ) +// allow alphanumerics, underscores and dashes +var testNameCleanRegex = regexp.MustCompile(`[^a-zA-Z0-9 \-_]+`) + func NewConfigNoTLS() *EtcdProcessClusterConfig { return &EtcdProcessClusterConfig{ClusterSize: 3, InitialToken: "new", @@ -260,43 +264,45 @@ func (cfg *EtcdProcessClusterConfig) PeerScheme() string { return setupScheme(cfg.BasePeerScheme, cfg.IsPeerTLS) } -func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i int) *EtcdServerProcessConfig { - var curls []string - var curl string - port := cfg.BasePort + 5*i - clientPort := port - peerPort := port + 1 - peer2Port := port + 3 - clientHttpPort := port + 4 +func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfigs(tb testing.TB) []*EtcdServerProcessConfig { + lg := zaptest.NewLogger(tb) - if cfg.ClientTLS == ClientTLSAndNonTLS { - curl = clientURL(cfg.ClientScheme(), clientPort, ClientNonTLS) - curls = []string{curl, clientURL(cfg.ClientScheme(), clientPort, ClientTLS)} - } else { - curl = clientURL(cfg.ClientScheme(), clientPort, cfg.ClientTLS) - curls = []string{curl} + if cfg.BasePort == 0 { + cfg.BasePort = EtcdProcessBasePort + } + if cfg.ExecPath == "" { + cfg.ExecPath = BinPath + } + if cfg.SnapshotCount == 0 { + cfg.SnapshotCount = etcdserver.DefaultSnapshotCount } - purl := url.URL{Scheme: cfg.PeerScheme(), Host: fmt.Sprintf("localhost:%d", peerPort)} - peerAdvertiseUrl := url.URL{Scheme: cfg.PeerScheme(), Host: fmt.Sprintf("localhost:%d", peerPort)} - var proxyCfg *proxy.ServerConfig - if cfg.PeerProxy { - if !cfg.IsPeerTLS { - panic("Can't use peer proxy without peer TLS as it can result in malformed packets") - } - peerAdvertiseUrl.Host = fmt.Sprintf("localhost:%d", peer2Port) - proxyCfg = &proxy.ServerConfig{ - Logger: zap.NewNop(), - To: purl, - From: peerAdvertiseUrl, + etcdCfgs := make([]*EtcdServerProcessConfig, cfg.ClusterSize) + initialCluster := make([]string, cfg.ClusterSize) + for i := 0; i < cfg.ClusterSize; i++ { + var curls []string + var curl, curltls string + port := cfg.BasePort + 5*i + curlHost := fmt.Sprintf("localhost:%d", port) + + switch cfg.ClientTLS { + case ClientNonTLS, ClientTLS: + curl = (&url.URL{Scheme: cfg.ClientScheme(), Host: curlHost}).String() + curls = []string{curl} + case ClientTLSAndNonTLS: + curl = (&url.URL{Scheme: "http", Host: curlHost}).String() + curltls = (&url.URL{Scheme: "https", Host: curlHost}).String() + curls = []string{curl, curltls} } - } - name := fmt.Sprintf("test-%d", i) - dataDirPath := cfg.DataDirPath - if cfg.DataDirPath == "" { - dataDirPath = tb.TempDir() - } + purl := url.URL{Scheme: cfg.PeerScheme(), Host: fmt.Sprintf("localhost:%d", port+1)} + + name := fmt.Sprintf("%s-test-%d", testNameCleanRegex.ReplaceAllString(tb.Name(), ""), i) + dataDirPath := cfg.DataDirPath + if cfg.DataDirPath == "" { + dataDirPath = tb.TempDir() + } + initialCluster[i] = fmt.Sprintf("%s=%s", name, purl.String()) args := []string{ "--name", name, diff --git a/tests/framework/e2e/cluster_proxy.go b/tests/framework/e2e/cluster_proxy.go index 41452a412ee..74681805c5c 100644 --- a/tests/framework/e2e/cluster_proxy.go +++ b/tests/framework/e2e/cluster_proxy.go @@ -26,8 +26,9 @@ import ( "strconv" "strings" - "go.etcd.io/etcd/pkg/v3/expect" "go.uber.org/zap" + + "go.etcd.io/etcd/pkg/v3/expect" ) type proxyEtcdProcess struct { @@ -123,6 +124,7 @@ func (p *proxyEtcdProcess) Logs() LogsExpect { type proxyProc struct { lg *zap.Logger + name string execPath string args []string ep string @@ -138,7 +140,7 @@ func (pp *proxyProc) start() error { if pp.proc != nil { panic("already started") } - proc, err := SpawnCmdWithLogger(pp.lg, append([]string{pp.execPath}, pp.args...), nil) + proc, err := SpawnCmdWithLogger(pp.lg, append([]string{pp.execPath}, pp.args...), nil, pp.name) if err != nil { return err } @@ -202,6 +204,7 @@ func newProxyV2Proc(cfg *EtcdServerProcessConfig) *proxyV2Proc { } return &proxyV2Proc{ proxyProc: proxyProc{ + name: cfg.Name, lg: cfg.lg, execPath: cfg.ExecPath, args: append(args, cfg.TlsArgs...), @@ -288,6 +291,7 @@ func newProxyV3Proc(cfg *EtcdServerProcessConfig) *proxyV3Proc { } return &proxyV3Proc{ proxyProc{ + name: cfg.Name, lg: cfg.lg, execPath: cfg.ExecPath, args: append(args, tlsArgs...), diff --git a/tests/framework/e2e/etcd_process.go b/tests/framework/e2e/etcd_process.go index bb464e4db8f..b3e04aa90dd 100644 --- a/tests/framework/e2e/etcd_process.go +++ b/tests/framework/e2e/etcd_process.go @@ -27,11 +27,12 @@ import ( "testing" "time" + "go.uber.org/zap" + "github.com/coreos/go-semver/semver" "go.etcd.io/etcd/client/pkg/v3/fileutil" "go.etcd.io/etcd/pkg/v3/expect" "go.etcd.io/etcd/pkg/v3/proxy" - "go.uber.org/zap" ) var ( @@ -148,33 +149,33 @@ func (ep *EtcdServerProcess) Start() error { } } ep.cfg.lg.Info("starting server...", zap.String("name", ep.cfg.Name)) - proc, err := SpawnCmdWithLogger(ep.cfg.lg, append([]string{ep.cfg.ExecPath}, ep.cfg.Args...), ep.cfg.EnvVars) + proc, err := SpawnCmdWithLogger(ep.cfg.lg, append([]string{ep.cfg.ExecPath}, ep.cfg.Args...), ep.cfg.EnvVars, ep.cfg.Name) if err != nil { return err } ep.proc = proc err = ep.waitReady() if err == nil { - ep.cfg.lg.Info("started server.", zap.String("name", ep.cfg.Name)) + ep.cfg.lg.Info("started server.", zap.String("name", ep.cfg.Name), zap.Int("pid", ep.proc.Pid())) } return err } func (ep *EtcdServerProcess) Restart() error { - ep.cfg.lg.Info("restaring server...", zap.String("name", ep.cfg.Name)) + ep.cfg.lg.Info("restarting server...", zap.String("name", ep.cfg.Name)) if err := ep.Stop(); err != nil { return err } ep.donec = make(chan struct{}) err := ep.Start() if err == nil { - ep.cfg.lg.Info("restared server", zap.String("name", ep.cfg.Name)) + ep.cfg.lg.Info("restarted server", zap.String("name", ep.cfg.Name)) } return err } func (ep *EtcdServerProcess) Stop() (err error) { - ep.cfg.lg.Info("stoping server...", zap.String("name", ep.cfg.Name)) + ep.cfg.lg.Info("stopping server...", zap.String("name", ep.cfg.Name)) if ep == nil || ep.proc == nil { return nil } @@ -230,7 +231,7 @@ func (ep *EtcdServerProcess) Config() *EtcdServerProcessConfig { return ep.cfg } func (ep *EtcdServerProcess) Logs() LogsExpect { if ep.proc == nil { - ep.cfg.lg.Panic("Please grap logs before process is stopped") + ep.cfg.lg.Panic("Please grab logs before process is stopped") } return ep.proc } diff --git a/tests/framework/e2e/etcd_spawn_nocov.go b/tests/framework/e2e/etcd_spawn_nocov.go index a9343ccea2b..3bbb2010db6 100644 --- a/tests/framework/e2e/etcd_spawn_nocov.go +++ b/tests/framework/e2e/etcd_spawn_nocov.go @@ -22,17 +22,14 @@ import ( "os" "strings" - "go.etcd.io/etcd/pkg/v3/expect" "go.uber.org/zap" + + "go.etcd.io/etcd/pkg/v3/expect" ) const noOutputLineCount = 0 // regular binaries emit no extra lines -func SpawnCmd(args []string, envVars map[string]string) (*expect.ExpectProcess, error) { - return SpawnCmdWithLogger(zap.NewNop(), args, envVars) -} - -func SpawnCmdWithLogger(lg *zap.Logger, args []string, envVars map[string]string) (*expect.ExpectProcess, error) { +func SpawnCmdWithLogger(lg *zap.Logger, args []string, envVars map[string]string, name string) (*expect.ExpectProcess, error) { wd, err := os.Getwd() if err != nil { return nil, err @@ -40,11 +37,19 @@ func SpawnCmdWithLogger(lg *zap.Logger, args []string, envVars map[string]string env := mergeEnvVariables(envVars) if strings.HasSuffix(args[0], "/etcdctl3") { env = append(env, "ETCDCTL_API=3") - lg.Info("spawning process with ETCDCTL_API=3", zap.Strings("args", args), zap.String("working-dir", wd), zap.Strings("environment-variables", env)) - return expect.NewExpectWithEnv(CtlBinPath, args[1:], env) + lg.Info("spawning process with ETCDCTL_API=3", + zap.Strings("args", args), + zap.String("working-dir", wd), + zap.String("name", name), + zap.Strings("environment-variables", env)) + return expect.NewExpectWithEnv(CtlBinPath, args[1:], env, name) } - lg.Info("spawning process", zap.Strings("args", args), zap.String("working-dir", wd), zap.Strings("environment-variables", env)) - return expect.NewExpectWithEnv(args[0], args[1:], env) + lg.Info("spawning process", + zap.Strings("args", args), + zap.String("working-dir", wd), + zap.String("name", name), + zap.Strings("environment-variables", env)) + return expect.NewExpectWithEnv(args[0], args[1:], env, name) } func mergeEnvVariables(envVars map[string]string) []string { diff --git a/tests/framework/e2e/v2v3.go b/tests/framework/e2e/v2v3.go index a42bb936d0f..9a694a12fe7 100644 --- a/tests/framework/e2e/v2v3.go +++ b/tests/framework/e2e/v2v3.go @@ -17,6 +17,18 @@ package e2e -func AddV2Args(args []string) []string { - return append(args, "--experimental-enable-v2v3", "v2/") +import ( + "strings" + + "go.uber.org/zap" + + "go.etcd.io/etcd/pkg/v3/expect" +) + +func SpawnCmd(args []string, envVars map[string]string) (*expect.ExpectProcess, error) { + return SpawnNamedCmd(strings.Join(args, "_"), args, envVars) +} + +func SpawnNamedCmd(processName string, args []string, envVars map[string]string) (*expect.ExpectProcess, error) { + return SpawnCmdWithLogger(zap.NewNop(), args, envVars, processName) }