Skip to content

fix: Add support for changing the vxlan tunnel port after initial installation #38583

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 1 commit into from
May 2, 2025
Merged

fix: Add support for changing the vxlan tunnel port after initial installation #38583

merged 1 commit into from
May 2, 2025

Conversation

rapour
Copy link
Contributor

@rapour rapour commented Mar 28, 2025

Fixes: #38581

The setupVxlanDevice runs ensureDevice before checking for any configuration change. One of the steps defined in ensureDevice is to set up the vxlan interface. If the vxlan interface conflicts with another interface like having the same destination port, setting a different port will not resolve the issue since the setupVxlanDevice still fails through ensureDevice before even checking the port change. This fix suggests setupVxlanDevice checks for destination port change and recreate the interface before trying to bring the interface up.

@maintainer-s-little-helper
Copy link

Commit e816afc does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Mar 28, 2025
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Mar 28, 2025
@rapour rapour force-pushed the fix-ensure-device-failure branch from e816afc to f42bca8 Compare March 28, 2025 15:50
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 28, 2025
@joestringer joestringer changed the title fix: agent cannot recreate a failed vxlan interface device fix: Add support for changing the vxlan tunnel port after initial installation Apr 2, 2025
@joestringer joestringer added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Apr 2, 2025
@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 Apr 2, 2025
@rapour rapour force-pushed the fix-ensure-device-failure branch from f42bca8 to 303f4fe Compare April 9, 2025 06:57
@rapour rapour marked this pull request as ready for review April 9, 2025 06:57
@rapour rapour requested a review from a team as a code owner April 9, 2025 06:57
@rapour rapour requested a review from rgo3 April 9, 2025 06:57
ti-mo
ti-mo requested changes Apr 17, 2025
View reviewed changes
@rapour rapour requested a review from ti-mo April 18, 2025 10:01
rgo3
rgo3 approved these changes Apr 22, 2025
View reviewed changes
Copy link
Contributor

@rgo3 rgo3 left a comment

Choose a reason for hiding this comment

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

Also double checked that the test is currently failing on main, thanks!

@rgo3
Copy link
Contributor

rgo3 commented Apr 22, 2025

/test

@maintainer-s-little-helper
Copy link

Commit 9ccd725 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 23, 2025
@rapour rapour force-pushed the fix-ensure-device-failure branch from 9ccd725 to 5a28307 Compare April 23, 2025 13:35
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 23, 2025
@rgo3
Copy link
Contributor

rgo3 commented Apr 23, 2025

Could you please remove the merge commit, squash the two commits with the actual changes and rebase onto main?

@rapour rapour force-pushed the fix-ensure-device-failure branch from 5a28307 to eca8546 Compare April 23, 2025 15:36
@rapour
Copy link
Contributor Author

rapour commented Apr 23, 2025

Done

@rapour rapour force-pushed the fix-ensure-device-failure branch from eca8546 to d5c86b0 Compare April 23, 2025 15:43
@rgo3
Copy link
Contributor

rgo3 commented Apr 24, 2025

/test

@rapour rapour force-pushed the fix-ensure-device-failure branch from d5c86b0 to e39ff93 Compare April 24, 2025 09:35
@rapour
Copy link
Contributor Author

rapour commented Apr 24, 2025

Shortened the commit message
the conformance failure was probably due to a pull error

@rapour rapour force-pushed the fix-ensure-device-failure branch from e39ff93 to 25f1a15 Compare April 24, 2025 11:10
ti-mo
ti-mo reviewed Apr 28, 2025
View reviewed changes
@ti-mo
Copy link
Contributor

ti-mo commented Apr 28, 2025

/test

ti-mo
ti-mo approved these changes Apr 28, 2025
View reviewed changes
Copy link
Contributor

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Thanks! Left one nit.

@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 Apr 28, 2025
@rapour rapour force-pushed the fix-ensure-device-failure branch from 25f1a15 to 8c1d327 Compare April 28, 2025 14:22
@rapour
Copy link
Contributor Author

rapour commented Apr 28, 2025

The nit is addresses, thanks a lot guys

@aanm aanm enabled auto-merge April 29, 2025 08:26
@aanm
Copy link
Member

aanm commented Apr 29, 2025

/test

@rapour
Copy link
Contributor Author

rapour commented May 1, 2025

Is there anything I can do to unblock this PR?

@ldelossa
Copy link
Contributor

ldelossa commented May 1, 2025

@rapour We seem to be getting a persistent failure in the following CI test: https://github.com/cilium/cilium/actions/runs/14726773148/job/41429620511

Can you rebase this PR off main and we will perform a new run of CI tests to determine if the failure continues to exist.

@rapour
fix: add support for changing vxlan port after initial installation
3bedbb4 
Signed-off-by: Reza Abbasalipour <reza.abbasalipour@canonical.com>
auto-merge was automatically disabled May 2, 2025 07:54

Head branch was pushed to by a user without write access

@rapour rapour force-pushed the fix-ensure-device-failure branch from 8c1d327 to 3bedbb4 Compare May 2, 2025 07:54
@rgo3
Copy link
Contributor

rgo3 commented May 2, 2025

/test

@ldelossa ldelossa added this pull request to the merge queue May 2, 2025
Merged via the queue into cilium:main with commit 9d13457 May 2, 2025
66 checks passed
@rapour rapour deleted the fix-ensure-device-failure branch May 3, 2025 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ti-mo ti-mo ti-mo approved these changes

@rgo3 rgo3 rgo3 approved these changes

Assignees
No one assigned
Labels
kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cilium cannot recover from a failed VXLAN interface device
6 participants
@rapour @rgo3 @ti-mo @aanm @ldelossa @joestringer