Skip to content

Conversation

ti-mo
Copy link
Contributor

@ti-mo ti-mo commented May 15, 2025

Since 7b92700 and e9438c2, egress policy programs are inserted unconditionally, even when the L7 proxy is disabled. Unfortunately, the teardown logic only runs when the proxy is enabled, causing all handle_policy_egress programs to leak.

Double unfortunately, since this program doesn't reference any Cilium maps in its attached-but-disabled state (it's an empty program), it doesn't show up in Cilium's maps metrics, making the issue hard to spot. The latter is being addressed in #39557.

This commit makes the removal unconditional, since the attachment is also unconditional. I've decided against making both attachment and detachment conditional, since that would still cause orphaned programs if the l7 proxy was disabled on a running cluster. Endpoint teardown should always attempt to remove all known policy programs for robustness.

Fix handle_policy_egress programs not being cleaned up during endpoint teardown

Since 7b92700 and e9438c2, egress policy programs are inserted
unconditionally, even when the L7 proxy is disabled. Unfortunately, the
teardown logic only runs when the proxy is enabled, causing all
handle_policy_egress programs to leak.

Double unfortunately, since this program doesn't reference any Cilium maps in
its attached-but-disabled state (it's an empty program), it doesn't show up in
Cilium's maps metrics, making the issue hard to spot. The latter is being
addressed in cilium#39557.

This commit makes the removal unconditional, since the attachment is
also unconditional. I've decided against making both attachment and detachment
conditional, since that would still cause orphaned programs if the l7 proxy
was disabled on a running cluster. Endpoint teardown should always attempt to
remove all known policy programs for robustness.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo requested review from a team as code owners May 15, 2025 15:20
@ti-mo ti-mo added the release-note/bug This PR fixes an issue in a previous release of Cilium. label May 15, 2025
@ti-mo ti-mo requested a review from gentoo-root May 15, 2025 15:20
@ti-mo ti-mo added needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch affects/v1.17 This issue affects v1.17 branch labels May 15, 2025
@ti-mo ti-mo requested a review from gandro May 15, 2025 15:20
@ti-mo
Copy link
Contributor Author

ti-mo commented May 15, 2025

/test

@ti-mo ti-mo enabled auto-merge May 19, 2025 08:17
@ti-mo ti-mo added this pull request to the merge queue May 20, 2025
@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 May 20, 2025
Merged via the queue into cilium:main with commit 2d8769d May 20, 2025
76 checks passed
@ti-mo ti-mo deleted the tb/remove-egress-prog branch May 20, 2025 15:12
@julianwiedmann julianwiedmann removed the affects/v1.17 This issue affects v1.17 branch label May 21, 2025
@joamaki joamaki mentioned this pull request May 23, 2025
6 tasks
@joamaki joamaki 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 May 23, 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 May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.17 The backport for Cilium 1.17.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/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants