Skip to content

Conversation

@aegal
Copy link
Contributor

@aegal aegal commented Aug 5, 2020

Reason for Change:
This is to address the potential for IPAM to leak IP's and have the pool be stuck in a 'in use ' state.

Issue Fixed:
#633

This also reintroduces some tests that were last in previous commits.

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Aug 5, 2020

Codecov Report

Merging #640 into master will increase coverage by 0.08%.
The diff coverage is 84.37%.

@@            Coverage Diff             @@
##           master     #640      +/-   ##
==========================================
+ Coverage   42.00%   42.09%   +0.08%     
==========================================
  Files          71       72       +1     
  Lines       10229    10362     +133     
==========================================
+ Hits         4297     4362      +65     
- Misses       5459     5531      +72     
+ Partials      473      469       -4     

Copy link
Member

@tamilmani1989 tamilmani1989 left a comment

Choose a reason for hiding this comment

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

I didnt see in releaseaddress id is reset to empty? how is it handled?

@aegal
Copy link
Contributor Author

aegal commented Aug 31, 2020

I didnt see in releaseaddress id is reset to empty? how is it handled?

Discussed offline, it is already handled in the releaseaddress

@aegal aegal requested a review from tamilmani1989 August 31, 2020 20:22
Copy link
Member

@tamilmani1989 tamilmani1989 left a comment

Choose a reason for hiding this comment

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

lgtm

@aegal aegal dismissed ashvindeodhar’s stale review September 1, 2020 23:09

Dismissing this since you are on vacation, discussed offline, addressed all comments

@aegal aegal merged commit d8e5732 into master Sep 1, 2020
@EdmondAndy
Copy link

Hi,

see the change for "ipam pool leak fix" has been merged for CNI issues.
Can you please confirm which aks version will include this fix? thanks

Ed

@zhiweiv zhiweiv mentioned this pull request Jan 20, 2021
@rbtr rbtr deleted the ipamLeakFix branch November 30, 2021 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants