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.

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

Program connected routes for borrowed VXLAN tunnel addresses #9662

Merged

Conversation

caseydavenport
Copy link
Member

@caseydavenport caseydavenport commented Dec 31, 2024

Description

Program routes to VXLAN tunnel IP addresses. This fixes an issue where nodes with borrowed IPs had broken connectivity. Previoiusly, we were relying on the route for the block to provide connectivity to the tunnel, but this isn't always the case.

Related issues/PRs

Fixes #6160

Todos

  • Tests
  • Documentation
  • Release note

Release Note

Fix that nodes with borrowed VXLAN tunnel addresses were not reachable by pods.

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@caseydavenport caseydavenport requested a review from a team as a code owner December 31, 2024 20:32
@marvin-tigera marvin-tigera added this to the Calico v3.30.0 milestone Dec 31, 2024
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Dec 31, 2024
@caseydavenport caseydavenport added the docs-not-required Docs not required for this change label Dec 31, 2024
@marvin-tigera marvin-tigera removed the docs-pr-required Change is not yet documented label Dec 31, 2024
@caseydavenport caseydavenport changed the title Program connected routes for VXLAN tunnel addresses Program connected routes for borrowed VXLAN tunnel addresses Jan 2, 2025
Copy link
Member

@fasaxc fasaxc left a comment

Choose a reason for hiding this comment

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

LGTM; would it be doable to add an FV test?

@caseydavenport
Copy link
Member Author

@fasaxc I'm looking into it... It doesn't look trivial based on my reading. The FV tests rely pretty heavily on algorithmic IP allocation - both for assigning VXLAN tunnel IPs to nodes, and then workload IPs. It makes it tricky to carve out exceptions.

I might still be able to thread something through...

@caseydavenport
Copy link
Member Author

I'm going to merge this for now, but raise a story into the sprint to keep looking at FV tests. So far, it's proven rather challenging to make it work and I think it might require a larger change to the way we configure VXLAN FV tests in general, so would like to bank this one as is.

@caseydavenport caseydavenport merged commit 342d8ec into projectcalico:master Jan 6, 2025
4 checks passed
@caseydavenport caseydavenport deleted the casey-vxlan-borrow branch January 6, 2025 19:43
@caseydavenport caseydavenport added cherry-pick-completed PR has been cherry-picked and removed cherry-pick-candidate labels Jan 8, 2025
caseydavenport added a commit to caseydavenport/calico that referenced this pull request Jan 8, 2025
…-borrow

Program connected routes for borrowed VXLAN tunnel addresses
caseydavenport added a commit that referenced this pull request Jan 8, 2025
Program connected routes for borrowed VXLAN tunnel addresses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-completed PR has been cherry-picked docs-not-required Docs not required for this change release-note-required Change has user-facing impact (no matter how small)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VXLAN not working when tunnel address is borrowed
4 participants