[3.5] member replace e2e test

[ZhouJianMS]

Follow up: #17079 (comment)

[k8s-ci-robot]

Hi @ZhouJianMS. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ahrtr]

can you please also backport the change for test case TestMemberReplace in #17119?

[ZhouJianMS]

can you please also backport the change for test case TestMemberReplace in #17119?

Sure, done.

[ahrtr]

@ZhouJianMS Reran the e2e test 4 times, always failed. Can you please download the log and take a look what's the issue?

[ZhouJianMS]

@ZhouJianMS Reran the e2e test 4 times, always failed. Can you please download the log and take a look what's the issue?

The test is stuck in waiting member killed after executing member remove command. @serathius Does member in etcd 3.5 automatically kill themselves or not?

After adding member.Kill(), the member can be successfully stopped. But I noticed that adding member back failed due to error "the member has been permanently removed from the cluster". Investigating why re-added member got the same ID.

[fuweid]

For 3.5 release, there is no background goroutine as zombie process's reaper.
I think we can just export Close() here. Otherwise, zombie process is running and member.IsRunning is true until timeout.

// Close waits for the expect process to exit.
// Close currently does not return error if process exited with !=0 status.
// TODO: Close should expose underlying proces failure by default.
func (ep *ExpectProcess) Close() error { return ep.close(false) }

ps -ef | grep 2975576
fuwei    2975576 2973518  0 15:35 pts/1    00:00:03 /tmp/go-build2055634301/b001/e2e.test -test.testlogfile=/tmp/go-build2055634301/b001/testlog.txt -test.paniconexit0 -test.v=true -test.timeout=30m0s -test.run=TestMemberReplace
fuwei    2975587 2975576  0 15:35 ?        00:00:00 [etcd] <defunct>
fuwei    2975588 2975576  0 15:35 pts/39   00:00:02 /home/fuwei/go/src/go.etcd.io/etcd/bin/etcd --name test-0 --listen-client-urls http://localhost:20000 --advertise-client-urls http://localhost:20000 --listen-peer-urls http://localhost:20001 --initial-advertise-peer-urls http://localhost:20001 --initial-cluster-token  --data-dir /tmp/TestMemberReplace206448044/002 --snapshot-count 100000 --experimental-corrupt-check-time 1s --initial-cluster test-0=http://localhost:20001,test-1=http://localhost:20006,test-2=http://localhost:20011
fuwei    2975589 2975576  0 15:35 pts/40   00:00:02 /home/fuwei/go/src/go.etcd.io/etcd/bin/etcd --name test-1 --listen-client-urls http://localhost:20005 --advertise-client-urls http://localhost:20005 --listen-peer-urls http://localhost:20006 --initial-advertise-peer-urls http://localhost:20006 --initial-cluster-token  --data-dir /tmp/TestMemberReplace206448044/003 --snapshot-count 100000 --experimental-corrupt-check-time 1s --initial-cluster test-0=http://localhost:20001,test-1=http://localhost:20006,test-2=http://localhost:20011
fuwei    2975630 2975576  0 15:35 ?        00:00:00 [etcdctl] <defunct>
fuwei    2975657 2975576  0 15:35 ?        00:00:00 [etcdctl] <defunct>
fuwei    2975667 2975576  0 15:35 ?        00:00:00 [etcdctl] <defunct>
fuwei    2982980 2975823  0 15:46 pts/44   00:00:00 grep --color=auto --exclude-dir=.bzr --exclude-dir=CVS --exclude-dir=.git --exclude-dir=.hg --exclude-dir=.svn --exclude-dir=.idea --exclude-dir=.tox 2975576

https://github.com/etcd-io/etcd/blob/5b572f15162d9b61979fb5eed65e65b026917464/pkg/expect/expect.go#L88C8-L88C23

[ahrtr]

Why the implementation is different from the main branch?

etcd/tests/e2e/utils.go

Lines 139 to 147 in 93530f6

	func patchArgs(args []string, flag, newValue string) error {
	for i, arg := range args {
	if strings.Contains(arg, flag) {
	args[i] = fmt.Sprintf("--%s=%s", flag, newValue)
	return nil
	}
	}
	return fmt.Errorf("--%s flag not found", flag)
	}

[ZhouJianMS]

3.5 does not have "initial-cluster-state" as a default argument in e2e framework. So, we need to append it in args.

etcd/tests/framework/e2e/cluster.go

Lines 468 to 473 in 93530f6

	func (cfg *EtcdProcessClusterConfig) SetInitialOrDiscovery(serverCfg *EtcdServerProcessConfig, initialCluster []string, initialClusterState string) {
	if cfg.Discovery == "" && len(cfg.ServerConfig.DiscoveryCfg.Endpoints) == 0 {
	serverCfg.InitialCluster = strings.Join(initialCluster, ",")
	serverCfg.Args = append(serverCfg.Args, "--initial-cluster="+serverCfg.InitialCluster)
	serverCfg.Args = append(serverCfg.Args, "--initial-cluster-state="+initialClusterState)
	}

First introduced in commit 6f63f4b

[ahrtr]

We should backport #16707 to 3.5 if possible.

Please add a comment to explain the difference for now.

[ahrtr]

lgtm

Thanks