Converts between experimental and non-experimental flags in downgrade & upgrade robustness & e2e test by ahrtr · Pull Request #20001 · etcd-io/etcd

ahrtr · 2025-05-21T13:56:07Z

followup to #19959 (comment)

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

serathius · 2025-05-21T14:02:57Z

Think this has same issue as #19506 (comment)
It will not work with upgrade/downgrade tests. Flags are rendered just during cluster creation, not during server start.

codecov · 2025-05-21T14:29:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.08%. Comparing base (6e2be32) to head (8859f41).
Report is 2 commits behind head on main.

Additional details and impacted files

see 155 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #20001      +/-   ##
==========================================
- Coverage   68.82%   61.08%   -7.74%     
==========================================
  Files         424      410      -14     
  Lines       35762    34603    -1159     
==========================================
- Hits        24612    21137    -3475     
- Misses       9728    11879    +2151     
- Partials     1422     1587     +165

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 6e2be32...8859f41. Read the comment docs.

🚀 New features to boost your workflow:

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

tests/framework/e2e/config.go

serathius · 2025-05-22T14:15:57Z

tests/framework/e2e/downgrade.go

+
+		// When we downgrade or upgrade a member, we need to re-generate the flags, to convert some non-experimental
+		// flags to experimental flags, or vice verse.
+		if err := clus.UpdateProcOptions(memberID, t); err != nil {


This will not be called in robustness tests

My understanding is both robustness test and e2e test reuse the same DowngradeUpgradeMembersByID to perform downgrade & upgrade.

If it doesn't, then robustness test will fail.

But the robustness test on older version is periodically test, can't be verified immediately. So I raised kubernetes/test-infra#34834

serathius · 2025-05-23T09:45:05Z

FYI: I'm waiting for CI to pass before reviewing it again.

ahrtr · 2025-05-23T09:57:48Z

FYI: I'm waiting for CI to pass before reviewing it again.

I will rebase this PR once #20016 gets merged.

Also we can see e2e workflow result in this PR before being rebased.

ahrtr · 2025-05-23T10:41:32Z

ping @serathius It should be working now.

serathius · 2025-05-23T12:19:30Z

tests/framework/e2e/config.go

+		return fmt.Sprintf("--%s=%s", name, value)
+	}
+
+	// version < 3.6 and name is experimental


Nit, those comments are not informative. Recommend to either remove them, or rewrite to answer "why?" instead of "what?". For example "In v3.7 etcd removed experimental prefix from flags (link to issue)"

k8s-ci-robot · 2025-05-23T12:43:34Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [ahrtr,serathius]

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

ahrtr · 2025-05-23T12:49:17Z

Since the kubernetes/test-infra#34834 hasn't been merged, so the robustness test against previous versions can't be verified in this PR. If there is still any issues after merging this PR, we can fix in followup PR(s).

serathius · 2025-05-23T12:52:28Z

Since the kubernetes/test-infra#34834 hasn't been merged, so the robustness test against previous versions can't be verified in this PR. If there is still any issues after merging this PR, we can fix in followup PR(s).

Have you run the test locally?

Signed-off-by: Benjamin Wang <benjamin.ahrtr@gmail.com>

ahrtr · 2025-05-23T13:39:02Z

Have you run the test locally?

3.6 works.

3.5/3.4 have issues, but unrelated to flags.

test-robustness-release-3.5:

        logger.go:146: 2025-05-23T14:25:27.534+0100	ERROR	Failed to trigger failpoint	{"failpoint": "beforeSendWatchResponse=sleep", "error": "goFailpoint beforeSendWatchResponse=sleep setup failed, err:bad status code: 400, err: fail to set failpoint: failpoint: failpoint does not exist\n"}
        main_test.go:144: failed triggering failpoint, err: goFailpoint beforeSendWatchResponse=sleep setup failed, err:bad status code: 400, err: fail to set failpoint: failpoint: failpoint does not exist

So we should be good to merge this PR, and investigate the failure separately.

serathius · 2025-05-23T13:47:31Z

So we should be good to merge this PR, and investigate the failure separately.

SG

ahrtr · 2025-05-23T14:01:34Z

/retest

k8s-ci-robot added area/testing approved size/M labels May 21, 2025

ahrtr force-pushed the robustness_test_20250521 branch from ef2ab60 to fb314b7 Compare May 21, 2025 14:07

ahrtr force-pushed the robustness_test_20250521 branch 3 times, most recently from f355334 to fa6dbc9 Compare May 21, 2025 14:46

ahrtr mentioned this pull request May 21, 2025

Add robustness test on 3.6, 3,5 and 3.4 kubernetes/test-infra#34834

Merged

serathius reviewed May 22, 2025

View reviewed changes

tests/framework/e2e/config.go Outdated Show resolved Hide resolved

ahrtr force-pushed the robustness_test_20250521 branch 2 times, most recently from d0cd0ef to 0d2e938 Compare May 22, 2025 13:37

k8s-ci-robot added size/L and removed size/M labels May 22, 2025

ahrtr force-pushed the robustness_test_20250521 branch 2 times, most recently from 4441c12 to cae3452 Compare May 22, 2025 13:59

serathius reviewed May 22, 2025

View reviewed changes

ahrtr force-pushed the robustness_test_20250521 branch 2 times, most recently from 4dd1c71 to 7f7a746 Compare May 23, 2025 09:38

ahrtr mentioned this pull request May 23, 2025

Move parseArgs into e2e framework #20016

Merged

ahrtr force-pushed the robustness_test_20250521 branch from 7f7a746 to a870bb7 Compare May 23, 2025 10:37

k8s-ci-robot added size/M and removed size/L labels May 23, 2025

ahrtr changed the title ~~Convert non experimental flags for etcd version older than 3.6~~ Convert the flags in downgrade & upgrade robustness & e2e test May 23, 2025

serathius reviewed May 23, 2025

View reviewed changes

ahrtr force-pushed the robustness_test_20250521 branch from a870bb7 to dc92de1 Compare May 23, 2025 12:29

ahrtr force-pushed the robustness_test_20250521 branch from dc92de1 to e098bd4 Compare May 23, 2025 12:30

ahrtr changed the title ~~Convert the flags in downgrade & upgrade robustness & e2e test~~ converts between experimental and non-experimental flags in downgrade & upgrade robustness & e2e test May 23, 2025

ahrtr force-pushed the robustness_test_20250521 branch from e098bd4 to f7d7ef2 Compare May 23, 2025 12:42

serathius approved these changes May 23, 2025

View reviewed changes

convert the flags in downgrade & upgrade e2e test

8859f41

Signed-off-by: Benjamin Wang <benjamin.ahrtr@gmail.com>

ahrtr force-pushed the robustness_test_20250521 branch from f7d7ef2 to 8859f41 Compare May 23, 2025 13:28

ahrtr changed the title ~~converts between experimental and non-experimental flags in downgrade & upgrade robustness & e2e test~~ Converts between experimental and non-experimental flags in downgrade & upgrade robustness & e2e test May 23, 2025

ahrtr merged commit 622a010 into etcd-io:main May 23, 2025
30 of 31 checks passed

ahrtr deleted the robustness_test_20250521 branch May 23, 2025 14:42

ahrtr mentioned this pull request May 24, 2025

Remove prefix --experimental- instead of --experimental from flag #20019

Merged

Conversation

ahrtr commented May 21, 2025

Uh oh!

serathius commented May 21, 2025

Uh oh!

codecov bot commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

serathius May 22, 2025

Choose a reason for hiding this comment

Uh oh!

ahrtr May 22, 2025

Choose a reason for hiding this comment

Uh oh!

serathius May 22, 2025

Choose a reason for hiding this comment

Uh oh!

serathius commented May 23, 2025

Uh oh!

ahrtr commented May 23, 2025

Uh oh!

ahrtr commented May 23, 2025

Uh oh!

serathius May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented May 23, 2025

Uh oh!

ahrtr commented May 23, 2025

Uh oh!

serathius commented May 23, 2025

Uh oh!

ahrtr commented May 23, 2025

Uh oh!

serathius commented May 23, 2025

Uh oh!

ahrtr commented May 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

codecov bot commented May 21, 2025 •

edited

Loading

serathius May 23, 2025 •

edited

Loading