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

fix: return error on interface not found and always clean up snat ep #3226

Merged
merged 3 commits into from
Dec 4, 2024

Conversation

QxBytes
Copy link
Contributor

@QxBytes QxBytes commented Dec 2, 2024

Reason for Change:

If the interface is not found when querying for it in AllowHostToNC or vice versa, the error is ignored, leading to a potential panic if we try to access the interface (which would be nil). This PR checks the error and returns it if it is not found, avoiding the panic. Additionally, if there is a failure in deleting one of the interfaces for an endpoint, this PR will still attempt to clean up associated snat interface (previously an error during the cleanup process would cause the snat endpoint not to be cleaned up).

Issue Fixed:

Requirements:

Notes:
Green run: https://msazure.visualstudio.com/One/_build/results?buildId=109439035&view=results

@QxBytes QxBytes added cni Related to CNI. fix Fixes something. linux multitenancy labels Dec 2, 2024
@QxBytes QxBytes self-assigned this Dec 2, 2024
@QxBytes QxBytes marked this pull request as ready for review December 3, 2024 18:40
@QxBytes QxBytes requested a review from a team as a code owner December 3, 2024 18:40
@QxBytes QxBytes requested a review from sharifnasser December 3, 2024 18:40
@QxBytes
Copy link
Contributor Author

QxBytes commented Dec 3, 2024

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@QxBytes QxBytes added this pull request to the merge queue Dec 4, 2024
Merged via the queue into master with commit 41b7122 Dec 4, 2024
97 checks passed
@QxBytes QxBytes deleted the alew/fix-snat-panic branch December 4, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cni Related to CNI. fix Fixes something. linux multitenancy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants