Skip to content

Comments

ipallocator fixes#5

Merged
jessfraz merged 6 commits intogenuinetools:masterfrom
al1img:fix_ip_allocation
Jun 8, 2018
Merged

ipallocator fixes#5
jessfraz merged 6 commits intogenuinetools:masterfrom
al1img:fix_ip_allocation

Conversation

@al1img
Copy link
Contributor

@al1img al1img commented Jun 7, 2018

Current ipallocator has following issues:

  • due to wrong sorting order in db it doesn't go through all subnet address space;
  • multicast and broadcast addresses could be assigned;
  • cross subnet address limit is not handled properly.

This pull request contains commits which intend to solve above issues.

al1img added 6 commits June 6, 2018 16:15
Using string ip causes wrong sorting order and wrong last assigned ip
definition. Because in this case ip address 172.19.0.10 is put before
172.19.0.2. And after allocation of 9 addresses, netns always tries to allocate
172.19.0.10 even it is already allocated. Using ping to check if ip is in use
helps in this case but leads to race condition when same address can be assigned
to different containers.

In this case lastip variable is reset instead of ip variable.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
In case of ip overflows subnet address space, "ip" variable should be reset
instead of "lastip".

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
In current design last allocated ip is defined as last record in db. But when ip
address overflows subnet address space and starts from the begining, last
allocated ip will point to last db entry. The patch stores last allocated ip in
the dedicated row with key equals "0".

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Iterate through all subnet address space from last allocated ip.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Add skipping non unicast and already assigned to the bridge addresses.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
@codecov-io
Copy link

Codecov Report

Merging #5 into master will decrease coverage by 0.17%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #5      +/-   ##
========================================
- Coverage    2.98%   2.8%   -0.18%     
========================================
  Files           5      5              
  Lines         335    356      +21     
========================================
  Hits           10     10              
- Misses        324    345      +21     
  Partials        1      1
Impacted Files Coverage Δ
list.go 0% <0%> (ø) ⬆️
ipallocator/ipallocator.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 189efb8...198b75b. Read the comment docs.

@jessfraz
Copy link
Collaborator

jessfraz commented Jun 8, 2018

wow awesome thank you!

@jessfraz jessfraz merged commit aff5af8 into genuinetools:master Jun 8, 2018
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.

3 participants