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.

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

iptables: Read CNI chaining mode from CNI config manager #30766

Merged
merged 1 commit into from Feb 29, 2024
Merged

iptables: Read CNI chaining mode from CNI config manager #30766

merged 1 commit into from Feb 29, 2024

Conversation

pippolo84
Copy link
Member

@pippolo84 pippolo84 commented Feb 14, 2024 •
edited

CNI chaining mode option has been moved to the CNI cell in commit 1254bf4.

Since it is not a global config option anymore, iptables manager will not see any change to that value, and its field CNIChainingMode will always be an empty string.
Thus, with the following config option values:

  • "enable-endpoint-routes": true
  • "cni-chaining-mode": "aws-cni"

the delivery interface referenced in the rules installed by the manager is "lxc+" instead of "eni+".

This commit fixes this adding a CNI config manager reference to the iptables manager parameters, in order to read the current setting for the chaining mode during rules installation.

Fixes: 1254bf4 ("daemon / cni: move to Cell, watch for changes")

Fix the referenced interface in iptables rules (`eni+` instead of `lxc+`) when `--enable-endpoint-routes=true` and `--cni-chaining-mode="aws-cni"`

@pippolo84 pippolo84 added kind/bug This is a bug in the Cilium logic. area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. release-note/misc This PR makes changes that have no direct user impact. backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. area/iptables Impacts how Cilium interacts with iptables. backport/1.15 This PR represents a backport for Cilium 1.15.x of a PR that was merged to main. labels Feb 14, 2024
@pippolo84
Copy link
Member Author

/test

@pippolo84 pippolo84 marked this pull request as ready for review February 14, 2024 15:19
@pippolo84 pippolo84 requested a review from a team as a code owner February 14, 2024 15:19
@pippolo84
Copy link
Member Author

Added @squeed as reviewer as original author of #24389

@pippolo84 pippolo84 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 and removed backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. backport/1.15 This PR represents a backport for Cilium 1.15.x of a PR that was merged to main. labels Feb 14, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.15.1 Feb 14, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.7 Feb 14, 2024
@michi-covalent michi-covalent added this to Needs backport from main in 1.15.2 Feb 14, 2024
@michi-covalent michi-covalent removed this from Needs backport from main in 1.15.1 Feb 14, 2024
@aanm aanm added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Feb 19, 2024
@pippolo84 pippolo84 force-pushed the pr/pippolo84/fix-iptables-chaining-mode branch from 50a8997 to 05f0b8f Compare February 19, 2024 15:47
@pippolo84
Copy link
Member Author

Force pushed after rebase

@pippolo84 pippolo84 removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Feb 19, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.8 Feb 19, 2024
ldelossa
ldelossa approved these changes Feb 19, 2024
View reviewed changes
Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

LGTM

squeed
squeed approved these changes Feb 27, 2024
View reviewed changes
Copy link
Contributor

@squeed squeed left a comment

Choose a reason for hiding this comment

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

Nice catch!

@pippolo84
Copy link
Member Author

/test

@pippolo84 pippolo84 mentioned this pull request Feb 28, 2024
Closed
@pippolo84
Copy link
Member Author

ci-clustermesh hit #30964, rerunning

@julianwiedmann julianwiedmann added this pull request to the merge queue Feb 29, 2024
@julianwiedmann julianwiedmann added the kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. label Feb 29, 2024
Merged via the queue into cilium:main with commit 77053ae Feb 29, 2024
61 of 62 checks passed
@pippolo84 pippolo84 added affects/v1.14 This issue affects v1.14 branch affects/v1.15 This issue affects v1.15 branch labels Feb 29, 2024
@pippolo84 pippolo84 mentioned this pull request Mar 5, 2024
Merged
13 tasks
@pippolo84 pippolo84 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 Mar 5, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.15 in 1.15.2 Mar 5, 2024
@pippolo84 pippolo84 added the backport/author The backport will be carried out by the author of the PR. label Mar 5, 2024
@pippolo84 pippolo84 mentioned this pull request Mar 5, 2024
Merged
7 tasks
@pippolo84 pippolo84 added backport/author The backport will be carried out by the author of the PR. and removed backport/author The backport will be carried out by the author of the PR. labels Mar 5, 2024
@pippolo84 pippolo84 mentioned this pull request Mar 9, 2024
Merged
@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 Mar 11, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport pending to v1.15 in 1.15.2 Mar 11, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport done to v1.15 in 1.15.2 Mar 11, 2024
@marseel marseel mentioned this pull request Mar 11, 2024
Closed
@jrajahalme jrajahalme mentioned this pull request Mar 13, 2024
Merged
@thorn3r thorn3r added this to Needs backport from main in 1.14.9 Mar 13, 2024
@thorn3r thorn3r removed this from Needs backport from main in 1.14.8 Mar 13, 2024
@julianwiedmann
Copy link
Member

The v1.14 backport landed via #31265.

@julianwiedmann julianwiedmann added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed affects/v1.14 This issue affects v1.14 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch affects/v1.15 This issue affects v1.15 branch labels Mar 14, 2024
@jrajahalme jrajahalme moved this from Needs backport from main to Backport done to v1.14 in 1.14.9 Mar 26, 2024
@aanm aanm moved this from Needs backport from main 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
Reviewers

@squeed squeed squeed approved these changes

@ldelossa ldelossa ldelossa approved these changes

Assignees
No one assigned
Labels
area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. area/iptables Impacts how Cilium interacts with iptables. backport/author The backport will be carried out by the author of the PR. 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. kind/bug This is a bug in the Cilium logic. kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. 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
No open projects
1.14.7
Backport done to v1.14
1.14.9
Backport done to v1.14
1.15.2
Backport done to v1.15
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@pippolo84 @julianwiedmann @squeed @ldelossa @aanm