Skip to content

ci: modify bpftrace script to ignore destinations outside pod CIDRs #36364

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 3 commits into from
Jan 10, 2025
Merged

ci: modify bpftrace script to ignore destinations outside pod CIDRs #36364

merged 3 commits into from
Jan 10, 2025

Conversation

smagnani96
Copy link
Contributor

@smagnani96 smagnani96 commented Dec 4, 2024 •
edited
Loading

With this PR, the bpftrace script that we run in CI is now able to ignore TCP traffic with destionation address outside pod CIDRs. This is particularly useful in egress-gateway tests, for which pod-to-world and pod-to-node traffic is sent.
Prior to this, in conformance-ipsec-e2e we used to keep tests separate, and run bpftrace only on non egress-gateway tests.
This PR (re)unifies tests while also running bpftrace in background for all of them.

Modify bpftrace script in CI to ignore proxy traffic if destination is outside pod CIDRs.

@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 Dec 4, 2024
@smagnani96
Copy link
Contributor Author

/ci-ipsec-e2e

@smagnani96
Copy link
Contributor Author

1st run of /ci-ipsec-e2e failed as for the known flake #35485:

Error: bpftrace output is not empty
[15:13:52:633004] fd00:10:244:2::8dcc:42975 -> fd00:10:244:1::80f:8080 (proto: 6, TCP flags: ...R, encap: 0, ifindex: 9, netns: f0000098, override: 0)

Also here:

  • fd00:10:244:2::8dcc -> kind-worker IP (ingress: ipv6)
  • fd00:10:244:1::80f -> echo-same-node-799fbb4f76-zh55t

Attaching sysdump for debugging in the other issue cilium-sysdumps.zip.

Re-triggering test, the flake is not introduced in this PR.

@smagnani96
Copy link
Contributor Author

smagnani96 commented Dec 4, 2024 •
edited
Loading

2nd run all 🟢
That should be the only place related to this PR changes afaiu, let me know in case we need other CI checks.
Marking it as ready for review.

@smagnani96 smagnani96 added kind/enhancement This would improve or streamline existing functionality. area/CI Continuous Integration testing issue or flake area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. release-note/ci This PR makes changes to the CI. feature/ipsec Relates to Cilium's IPsec feature labels Dec 4, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 4, 2024
@smagnani96 smagnani96 marked this pull request as ready for review December 4, 2024 18:02
@smagnani96 smagnani96 requested review from a team as code owners December 4, 2024 18:03
@smagnani96 smagnani96 requested a review from pchaigno December 4, 2024 18:03
This was referenced Dec 4, 2024
Closed
Merged
@smagnani96 smagnani96 added the dont-merge/waiting-for-review Requires further review before merging. label Dec 4, 2024
@smagnani96 smagnani96 mentioned this pull request Dec 5, 2024
Closed
@smagnani96
Copy link
Contributor Author

/test

@smagnani96 smagnani96 force-pushed the pr/smagnani96/bpftrace-ci-ignore-non-cidrs branch from 3ac02f0 to 6a4a1d1 Compare December 10, 2024 10:37
@smagnani96
Copy link
Contributor Author

3rd run was green, but need to rebase due to other CI panics.

@smagnani96
Copy link
Contributor Author

/test

pchaigno
pchaigno reviewed Dec 11, 2024
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Nice!

One question below. I'm not very familiar with the bpftrace script, so trying to make sure I understand things correctly.

@smagnani96 smagnani96 force-pushed the pr/smagnani96/bpftrace-ci-ignore-non-cidrs branch from 6a4a1d1 to 5124e01 Compare January 7, 2025 10:27
@smagnani96 smagnani96 removed the dont-merge/waiting-for-review Requires further review before merging. label Jan 7, 2025
@smagnani96 smagnani96 force-pushed the pr/smagnani96/bpftrace-ci-ignore-non-cidrs branch from 5124e01 to a83da4b Compare January 8, 2025 21:08
@smagnani96
Copy link
Contributor Author

/test

This was referenced Jan 11, 2025
Merged
Merged
Closed
@julianwiedmann julianwiedmann mentioned this pull request Jan 20, 2025
Merged
@julianwiedmann julianwiedmann added needs-backport/1.14 needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Jan 20, 2025
@rastislavs rastislavs mentioned this pull request Jan 21, 2025
Merged
45 tasks
@rastislavs rastislavs added backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Jan 21, 2025
@github-actions github-actions bot added backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. and removed backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. labels Jan 22, 2025
@rastislavs rastislavs mentioned this pull request Jan 22, 2025
Merged
19 tasks
@rastislavs rastislavs added backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Jan 22, 2025
@rastislavs rastislavs mentioned this pull request Jan 22, 2025
Merged
6 tasks
@rastislavs rastislavs added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 labels Jan 22, 2025
@rastislavs rastislavs mentioned this pull request Jan 22, 2025
Merged
4 tasks
@rastislavs rastislavs added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 labels Jan 22, 2025
@github-actions github-actions bot added 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. backport-done/1.16 The backport for Cilium 1.16.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. backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. labels Jan 22, 2025
@cilium-release-bot cilium-release-bot bot mentioned this pull request Jan 24, 2025
Merged
@smagnani96 smagnani96 mentioned this pull request Feb 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchaigno pchaigno pchaigno approved these changes

@jschwinger233 jschwinger233 jschwinger233 approved these changes

Assignees
No one assigned
Labels
area/CI Continuous Integration testing issue or flake area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. 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. backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. feature/ipsec Relates to Cilium's IPsec feature kind/enhancement This would improve or streamline existing functionality. 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
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@smagnani96 @pchaigno @jschwinger233 @rastislavs @julianwiedmann