tests: Add make-mirror test implementation

[ronaldngounou]

Context

This PR contributes to https://github.com/etcd-io/etcd/issues/20612 . The goal is to migrate tests/e2e/ctl_v3_make_mirror_test.go to the common testing framework

co-author: @yagikota

While we were able to migrate the e2e tests, we would like to have a discussion with the maintenairs on how we can migrate the integration tests as providing guidance would require more context about the source code. cc @ahrtr @serathius

Testing

go test -v -count=1 -tags=e2e . -run TestMakeMirror

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 69.33%. Comparing base (1b2ed5a) to head (87ec3f0). :warning: Report is 89 commits behind head on main.

Additional details and impacted files

see 28 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #20908      +/-   ##
==========================================
+ Coverage   69.21%   69.33%   +0.11%     
==========================================
  Files         422      422              
  Lines       34831    34841      +10     
==========================================
+ Hits        24108    24156      +48     
+ Misses       9327     9296      -31     
+ Partials     1396     1389       -7     

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1b2ed5a...87ec3f0. Read the comment docs.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[serathius]

Please fix

common/make_mirror_test.go:130:2: undefined: configureMirrorDestTLS

[ronaldngounou]

Fixed, thanks.

[serathius]

Can you fix pull-etcd-verify? Just run make fix.

../tests/framework/e2e/config.go:44:1: File is not properly formatted (gci)
	Version ClusterVersion
^
../tests/common/make_mirror_test.go:134:10: testMirror$1 - opts is unused (unparam)
	go func(opts config.MakeMirrorOptions) {
	        ^
../tests/common/make_mirror_test.go:32:96: unnecessary leading newline (whitespace)
			t.Run(fmt.Sprintf("Source=%s/Destination=%s", srcTC.name, destTC.name), func(t *testing.T) {
			                                                                                            ^
../tests/common/make_mirror_test.go:51:46: unnecessary leading newline (whitespace)
		for _, destTC := range clusterTestCases() {
		                                           ^
../tests/common/make_mirror_test.go:82:4: unnecessary trailing newline (whitespace)
			})
			^
../tests/common/make_mirror_test.go:102:4: unnecessary trailing newline (whitespace)
			})
			^
../tests/framework/e2e/etcdctl.go:749:115: unnecessary leading newline (whitespace)
func (ctl *EtcdctlV3) MakeMirror(ctx context.Context, destEndpoints string, opts config.MakeMirrorOptions) error {
                                                                                                                  ^
../tests/framework/e2e/etcdctl.go:794:1: unnecessary trailing newline (whitespace)

[serathius]

/retest

[ronaldngounou]

/retest

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ronaldngounou, serathius

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [serathius]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[serathius]

/cc @fuweid @ahrtr

[yagikota]

@fuweid @ronaldngounou

This test case only works for e2e. I think we should move that into e2e folder instead of common.

The motivation of this PR is to migrate the following tests to common

• e2e: https://github.com/etcd-io/etcd/blob/main/tests/e2e/ctl_v3_make_mirror_test.go
• integration
  • https://github.com/etcd-io/etcd/blob/main/tests/integration/clientv3/mirror_auth_test.go
  • https://github.com/etcd-io/etcd/blob/main/tests/integration/clientv3/mirror_test.go

As you say, the current tests only work for e2e since MakeMirror implementation for integration test is just a mock.

https://github.com/etcd-io/etcd/blob/952c177c6f288deb1945d29da1a85b538a6bec57/tests/framework/integration/integration.go#L480-L482

To make the tests work for integration, we need to complete the MakeMirror function. The logic to be implemented is here:

https://github.com/etcd-io/etcd/blob/952c177c6f288deb1945d29da1a85b538a6bec57/etcdctl/ctlv3/command/make_mirror_command.go#L142-L240

But I'm not sure whether this approach is good or not...

[fuweid]

Hi @yagikota yeah. It's ok to move that into common. But that change on option doesn't make senses to me, like WithTCPClient. I think we revert that change. Other than that, It looks good.

[yagikota]

Hi @yagikota yeah. It's ok to move that into common. But that change on option doesn't make senses to me, like WithTCPClient. I think we revert that change. Other than that, It looks good.

Thanks for sharing your thoughts. Let's wait for updates from @ronaldngounou.

[ronaldngounou]

/test pull-etcd-e2e-386

[ronaldngounou]

/retest

[ronaldngounou]

@fuweid, the remaining comments are addressed. Waiting for a review please

[ronaldngounou]

/retest

[k8s-ci-robot]

@ronaldngounou: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-etcd-e2e-386	87ec3f0198fb7902822962e8e39bee514501ba70	link	true	/test pull-etcd-e2e-386
pull-etcd-e2e-arm64	87ec3f0198fb7902822962e8e39bee514501ba70	link	true	/test pull-etcd-e2e-arm64
pull-etcd-e2e-amd64	87ec3f0198fb7902822962e8e39bee514501ba70	link	true	/test pull-etcd-e2e-amd64

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-sigs/prow repository. I understand the commands that are listed here.

[ronaldngounou]

@fuweid Could you please review? I addressed the comments.