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

v1.12 Backports 2023-06-13 (IPsec) #26161

Merged
merged 1 commit into from
Jun 13, 2023
Merged

v1.12 Backports 2023-06-13 (IPsec) #26161

merged 1 commit into from
Jun 13, 2023

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Jun 13, 2023 •
edited by pchaigno

Once this PR is merged, you can update the PR labels via:

for pr in 26093; do contrib/backporting/set-labels.py $pr done 1.12; done

or with

make add-labels BRANCH=v1.12 ISSUES=26093

@pchaigno @qmonnet
ipsec: Don't attempt per-node route deletion when unexistant
ddeb4a1 
[ upstream commit 1e1e2f7 ]

Commit 3e59b68 ("ipsec: Per-node XFRM states & policies for EKS &
AKS") changed the XFRM config to have one state and policy per remote
node in IPAM modes ENI and Azure. The IPsec cleanup logic was therefore
also updated to call deleteIPsec() whenever a remote node is deleted.

However, we missed that the cleanup logic also tries to remove the
per-node IP route. In case of IPAM modes ENI and Azure, the IP route
however stays as before: we have a single route for all remote nodes. We
therefore don't have anything to cleanup.

Because of this unnecessary IP route cleanup attempt, an error message
was printed for every remote node deletion:

    Unable to delete the IPsec route OUT from the host routing table

This commit fixes it to avoid attempting this unnecessary cleanup.

Fixes: 3e59b68 ("ipsec: Per-node XFRM states & policies for EKS & AKS")
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet added kind/backports This PR provides functionality previously merged into master. backport/1.12 This PR represents a backport for Cilium 1.12.x of a PR that was merged to main. labels Jun 13, 2023
@qmonnet qmonnet requested a review from pchaigno June 13, 2023 09:12
@qmonnet qmonnet marked this pull request as ready for review June 13, 2023 09:13
@qmonnet qmonnet requested a review from a team as a code owner June 13, 2023 09:13
@qmonnet
Copy link
Member Author

qmonnet commented Jun 13, 2023

/test-backport-1.12

@qmonnet qmonnet changed the title v1.12 Backports 2023-06-13 v1.12 Backports 2023-06-13 (IPsec) Jun 13, 2023
pchaigno
pchaigno approved these changes Jun 13, 2023
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.

My PR looks good. Thanks!

@gandro gandro added the release-blocker/1.12 This issue will prevent the release of the next version of Cilium. label Jun 13, 2023
@gandro
Copy link
Member

gandro commented Jun 13, 2023

  • ConformanceKind1.19 is failing due to 1.1.1.1 currently rejecting our traffic (this is happening on other PRs too at the moment). There is nothing we can do about this in this PR, so that failure can be ignored.

Luckily, we do have alternative coverage for IPSec with PodCIDR based IPAM, notably:

And more. So the change here is covered more than enough. Marking as ready to merge..

@gandro gandro added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 13, 2023
@gandro gandro merged commit 866d4bc into v1.12 Jun 13, 2023
56 of 57 checks passed
@gandro gandro deleted the pr/v1.12-backport-2023-06-13 branch June 13, 2023 16:53
@michi-covalent michi-covalent mentioned this pull request Jun 14, 2023
Merged
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

@michi-covalent michi-covalent michi-covalent approved these changes

Assignees
No one assigned
Labels
backport/1.12 This PR represents a backport for Cilium 1.12.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.12 This issue will prevent the release of the next version of Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@qmonnet @gandro @pchaigno @michi-covalent