Skip to content

ci-e2e-upgrade: Bring it on #29073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 28, 2023
Merged

ci-e2e-upgrade: Bring it on #29073

merged 1 commit into from
Nov 28, 2023

Conversation

brb
Copy link
Member

@brb brb commented Nov 9, 2023

No description provided.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 9, 2023
@brb
Copy link
Member Author

brb commented Nov 9, 2023

/ci-e2e-upgrade

2 similar comments
@brb
Copy link
Member Author

brb commented Nov 9, 2023

/ci-e2e-upgrade

@brb
Copy link
Member Author

brb commented Nov 9, 2023

/ci-e2e-upgrade

@brb brb force-pushed the pr/brb/ci-upgrade-vol-4 branch from d599317 to 2d583e2 Compare November 9, 2023 12:06
@brb
Copy link
Member Author

brb commented Nov 9, 2023

/ci-e2e-upgrade

5 similar comments
@brb
Copy link
Member Author

brb commented Nov 9, 2023

/ci-e2e-upgrade

@brb
Copy link
Member Author

brb commented Nov 9, 2023

/ci-e2e-upgrade

@brb
Copy link
Member Author

brb commented Nov 9, 2023

/ci-e2e-upgrade

@brb
Copy link
Member Author

brb commented Nov 10, 2023

/ci-e2e-upgrade

@brb
Copy link
Member Author

brb commented Nov 10, 2023

/ci-e2e-upgrade

@brb brb force-pushed the pr/brb/ci-upgrade-vol-4 branch from bb269b1 to 360e9da Compare November 10, 2023 12:16
@brb
Copy link
Member Author

brb commented Nov 10, 2023

/ci-e2e-upgrade

@brb brb mentioned this pull request Nov 10, 2023
Closed
@brb brb force-pushed the pr/brb/ci-upgrade-vol-4 branch from 360e9da to f3f53ad Compare November 15, 2023 15:07
@brb
Copy link
Member Author

brb commented Nov 15, 2023

/ci-e2e-upgrade

2 similar comments
@brb
Copy link
Member Author

brb commented Nov 16, 2023

/ci-e2e-upgrade

@brb
Copy link
Member Author

brb commented Nov 17, 2023

/ci-e2e-upgrade

@brb brb force-pushed the pr/brb/ci-upgrade-vol-4 branch from 4e21f35 to 876a52f Compare November 17, 2023 08:50
@brb
Copy link
Member Author

brb commented Nov 17, 2023

/ci-e2e-upgrade

1 similar comment
@brb
Copy link
Member Author

brb commented Nov 17, 2023

/ci-e2e-upgrade

@brb brb force-pushed the pr/brb/ci-upgrade-vol-4 branch from 12bb6df to 5c355d7 Compare November 17, 2023 10:54
@brb
Copy link
Member Author

brb commented Nov 17, 2023

/ci-e2e-upgrade

1 similar comment
@brb
Copy link
Member Author

brb commented Nov 17, 2023

/ci-e2e-upgrade

@brb brb force-pushed the pr/brb/ci-upgrade-vol-4 branch from 7642333 to 54ee628 Compare November 17, 2023 14:19
@brb
Copy link
Member Author

brb commented Nov 17, 2023

/ci-e2e-upgrade

2 similar comments
@brb
Copy link
Member Author

brb commented Nov 17, 2023

/ci-e2e-upgrade

@brb
Copy link
Member Author

brb commented Nov 20, 2023

/ci-e2e-upgrade

@brb brb force-pushed the pr/brb/ci-upgrade-vol-4 branch from 03f67d3 to ed421d5 Compare November 20, 2023 10:30
@brb
Copy link
Member Author

brb commented Nov 20, 2023

/ci-e2e-upgrade

@brb brb marked this pull request as ready for review November 24, 2023 18:04
@brb brb requested review from a team as code owners November 24, 2023 18:04
viktor-kurchenko
viktor-kurchenko approved these changes Nov 27, 2023
View reviewed changes
Copy link
Contributor

@viktor-kurchenko viktor-kurchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

qmonnet
qmonnet approved these changes Nov 27, 2023
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, nice!

.github/workflows/conformance-e2e.yaml
@@ -74,7 +74,9 @@ jobs:
max-parallel: 16
matrix:
include:
# See https://github.com/cilium/cilium/issues/20606 for configuration table
# !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
# ! NOTE: keep tests-e2e-upgrade.yaml config in sync !
Copy link
Member

@qmonnet qmonnet Nov 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't suppose there would be a way to put the description of the jobs into a separate file and to pull them in the two workflows? (I'm not aware of this being doable with GHA, asking just in case you know of something).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or a CI check that parse the YAML and check that the two matrix.include are identical. However big you make this comment (:sweat_smile:), the list is long enough that people will add/change the end without noticing the comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, IRL it's impossible to keep them in sync :-( E.g., https://github.com/cilium/cilium/pull/29073/files#diff-6d64c043690537c26ae80866ccf018a7e9f0b2cdd9b1cffe5c40e6db3ae27c6cR129

@brb brb added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 27, 2023
@youngnick youngnick merged commit ad8f24c into main Nov 28, 2023
@youngnick youngnick deleted the pr/brb/ci-upgrade-vol-4 branch November 28, 2023 03:28
@brb brb added needs-backport/1.11 backport/author The backport will be carried out by the author of the PR. labels Nov 28, 2023
@brb brb mentioned this pull request Nov 28, 2023
Closed
@pchaigno
Copy link
Member

pchaigno commented Dec 7, 2023

@brb When you backport this, you'll have to make sure to apply #29623.

@julianwiedmann
Copy link
Member

Landed in the initial v1.15 branch, no backport needed.

joestringer
joestringer reviewed Oct 7, 2024
View reviewed changes
.github/workflows/tests-e2e-upgrade.yaml
Comment on lines +400 to +408
--flush-ct \
--sysdump-hubble-flows-count=1000000 --sysdump-hubble-flows-timeout=5m \
--sysdump-output-filename "cilium-sysdump-${{ matrix.name }}-<ts>" \
--junit-file "cilium-junits/${{ env.job_name }} (${{ join(matrix.*, ', ') }}).xml" \
--junit-property github_job_step="Run tests upgrade 2 (${{ join(matrix.*, ', ') }})" \
\$EXTRA

# --flush-ct interrupts the flows, so we need to set up again.
./cilium-cli connectivity test --include-conn-disrupt-test --conn-disrupt-test-setup
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the conntrack table being flushed in the first place?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because of the L7 connectivity tests, which we run before the conn-disrupt-tests. They can make subsequent tests flaky - cilium/cilium-cli@6d4434fb60.

@giorio94
Copy link
Member

👋 @brb Are you still planning on backporting this to v1.14?

@brb
Copy link
Member Author

brb commented Jan 28, 2025

Considering that once 1.17 is released, 1.14 is EOL, I don't plan to. Is there any reason for backporting it?

@giorio94
Copy link
Member

Considering that once 1.17 is released, 1.14 is EOL, I don't plan to. Is there any reason for backporting it?

Personally agree that it doesn't make sense to backport it given that 1.14 is almost EOL. Just double-checking to clean stale labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer left review comments

@pchaigno pchaigno pchaigno left review comments

@ti-mo ti-mo ti-mo approved these changes

@qmonnet qmonnet qmonnet approved these changes

@viktor-kurchenko viktor-kurchenko viktor-kurchenko approved these changes

Assignees
No one assigned
Labels
area/CI Continuous Integration testing issue or flake backport/author The backport will be carried out by the author of the PR. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.13.17
Status: Needs backport from main
1.14.12
Status: Needs backport from main
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@brb @pchaigno @julianwiedmann @giorio94 @joestringer @ti-mo @qmonnet @viktor-kurchenko @jrajahalme @youngnick