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: delete endpoint on cni state save failure #2577

Merged
merged 5 commits into from
Feb 29, 2024
Merged

Conversation

QxBytes
Copy link
Contributor

@QxBytes QxBytes commented Feb 12, 2024

Reason for Change:

In windows, it is possible to successfully create an HNS endpoint and then subsequently fail to write to the CNI statefile. While the CNI call fails, the HNS endpoint is never deleted, causing retries to fail because the HNS endpoint has already been created. As it is not possible to retrieve the existing endpoint on a retried ADD call, we instead opt to delete the HNS endpoint if saving the CNI state fails.

Issue Fixed:

See above.

Requirements:

Notes:
The code change affects both the windows and linux scenarios (if saving fails, deleteEndpoint will call the platform specific version for deletion of the endpoint). On windows, tested triggering a save failure and then examining HNS endpoints-- the endpoint is cleaned up which is confirmed in the logs. Without the fix, the endpoint still shows and produces the endpoint already exists (ip address claimed) error. On linux transparent-vlan mode, tested triggering a save failure, which does call into deleting the endpoint. Subsequent delete/adds for that pod run without erroring.

@QxBytes QxBytes added cni Related to CNI. fix Fixes something. windows labels Feb 12, 2024
@QxBytes QxBytes marked this pull request as ready for review February 12, 2024 23:04
@QxBytes QxBytes requested a review from a team as a code owner February 12, 2024 23:04
@QxBytes QxBytes added this pull request to the merge queue Feb 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 22, 2024
@QxBytes QxBytes added this pull request to the merge queue Feb 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 22, 2024
@QxBytes QxBytes added this pull request to the merge queue Feb 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 26, 2024
@QxBytes QxBytes added this pull request to the merge queue Feb 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 27, 2024
@QxBytes QxBytes added this pull request to the merge queue Feb 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 28, 2024
@QxBytes QxBytes added this pull request to the merge queue Feb 29, 2024
Merged via the queue into master with commit 6b5ea17 Feb 29, 2024
15 checks passed
@QxBytes QxBytes deleted the alew/add-idemp-win branch February 29, 2024 06:07
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants