Skip to content
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.

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

gha: additionally cover BPF masquerade in clustermesh E2E tests #30321

Merged

Conversation

giorio94
Copy link
Member

Currently, BPF masquerade was always disabled in the clustermesh E2E tests due to unintended interactions with Docker iptables rules breaking DNS resolution [1]. Instead, let's explicitly configure external upstream DNS servers for coredns, so that we can also enable this feature when KPR is enabled.

While being there, let's also make the KPR setting explicit, instead of relying on the Cilium CLI configuration (which is based on whether the kube-proxy daemonset is present or not).

[1]: #23283

@giorio94 giorio94 added area/CI Continuous Integration testing issue or flake area/clustermesh Relates to multi-cluster routing functionality in Cilium. release-note/ci This PR makes changes to the CI. labels Jan 18, 2024
@giorio94 giorio94 force-pushed the pr/giorio94/main/gha-clustermesh-bpf-masquerade branch from ce1c7e8 to 90db4a4 Compare January 18, 2024 17:08
@giorio94
Copy link
Member Author

/ci-clustermesh

@giorio94 giorio94 added needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Jan 18, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in v1.15.0-rc.1 Jan 18, 2024
@giorio94
Copy link
Member Author

Opening for review to start gathering feedback. Marking as blocked as well, given that it depends on cilium/cilium-cli#2242.

@giorio94 giorio94 marked this pull request as ready for review January 19, 2024 14:27
@giorio94 giorio94 requested review from a team as code owners January 19, 2024 14:27
@giorio94 giorio94 added the dont-merge/blocked Another PR must be merged before this one. label Jan 19, 2024
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.

Copy link
Contributor

@marseel marseel left a comment

Choose a reason for hiding this comment

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

LGTM.

@aanm aanm added this to Needs backport from main in 1.15.1 Jan 31, 2024
@aanm aanm removed this from Needs backport from main in v1.15.0-rc.1 Jan 31, 2024
Currently, BPF masquerade was always disabled in the clustermesh
E2E tests due to unintended interactions with Docker iptables
rules breaking DNS resolution [1]. Instead, let's explicitly
configure external upstream DNS servers for coredns, so that we
can also enable this feature when KPR is enabled.

While being there, let's also make the KPR setting explicit,
instead of relying on the Cilium CLI configuration (which is based
on whether the kube-proxy daemonset is present or not).

[1]: #23283

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 force-pushed the pr/giorio94/main/gha-clustermesh-bpf-masquerade branch from 90db4a4 to cf6beef Compare February 2, 2024 07:51
@giorio94
Copy link
Member Author

giorio94 commented Feb 2, 2024

Rebased onto main to drop the temporary commit, as a new version of the Cilium CLI has been released.

@giorio94 giorio94 removed the dont-merge/blocked Another PR must be merged before this one. label Feb 2, 2024
@giorio94
Copy link
Member Author

giorio94 commented Feb 2, 2024

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.7 Feb 2, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 2, 2024
@julianwiedmann julianwiedmann added this pull request to the merge queue Feb 5, 2024
Merged via the queue into main with commit a1089a7 Feb 5, 2024
217 checks passed
@julianwiedmann julianwiedmann deleted the pr/giorio94/main/gha-clustermesh-bpf-masquerade branch February 5, 2024 07:48
@nbusseneau nbusseneau mentioned this pull request Feb 8, 2024
9 tasks
@nbusseneau nbusseneau added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Feb 8, 2024
@nbusseneau nbusseneau mentioned this pull request Feb 8, 2024
12 tasks
@nbusseneau nbusseneau added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Feb 8, 2024
@github-actions github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Feb 9, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from main in 1.15.1 Feb 9, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.7 Feb 9, 2024
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Feb 9, 2024
@aanm aanm moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.7 Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake area/clustermesh Relates to multi-cluster routing functionality in Cilium. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. 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.14.7
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

None yet

5 participants