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

[17.12] Fix IP overlap with empty EndpointSpec #2513

Merged

Conversation

thaJeztah
Copy link
Member

cherry-pick of #2505 for 17.12

git checkout -b 17.12-backport-fixendpointspec upstream/bump_v17.12
git cherry-pick -s -S -x bd4e923c241ff1f9a493eab02125edbf6ec0b901

(no conflicts)

Passing and empty EndpointSpec in the service spec
was correctly triggering the VIP allocation but
the leader election was erroneusly handling the IPAM
state restore trying to release the VIP.
The fix focuses on proper handling of the restart case.

Signed-off-by: Flavio Crisciani flavio.crisciani@docker.com
(cherry picked from commit bd4e923)
Signed-off-by: Sebastiaan van Stijn github@gone.nl

Passing and empty EndpointSpec in the service spec
was correctly triggering the VIP allocation but
the leader election was erroneusly handling the IPAM
state restore trying to release the VIP.
The fix focuses on proper handling of the restart case.

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
(cherry picked from commit bd4e923)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

ping @fcrisciani PTAL

Copy link

@fcrisciani fcrisciani left a comment

Choose a reason for hiding this comment

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

Code looks good but the CI failed on a test that is flacky, @nishanttotla @anshulpundir can you guys restart it?

Copy link
Contributor

@nishanttotla nishanttotla left a comment

Choose a reason for hiding this comment

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

LGTM.

Restarted CI.

@andrewhsu
Copy link
Member

@nishanttotla perhaps need another kick of the ci?

@nishanttotla
Copy link
Contributor

CI seems to keep failing @andrewhsu :(

@andrewhsu
Copy link
Member

i see it too:

time="2018-02-16T23:41:07Z" level=error msg="task allocation failure" error="failed to retrieve network testID3 while allocating task testTaskID3" 
time="2018-02-16T23:41:08Z" level=error msg="Failed allocation for network testID5" error="failed while allocating driver state for network testID5: could not obtain vxlan id for pool 10.0.4.0/24: requested bit is already allocated" 
time="2018-02-16T23:41:08Z" level=error msg="task allocation failure" error="network testID5 attached to task testTaskID6 not allocated yet" 
time="2018-02-16T23:41:08Z" level=error msg="Failed allocation for service testServiceID4" error="requested bit is already allocated" 
time="2018-02-16T23:41:08Z" level=error msg="task allocation failure" error="service testServiceID4 to which this task testTaskID7 belongs has pending allocations" 
--- FAIL: TestNodeAllocator (0.04s)
	allocator_test.go:1190: timed out before watchNode found expected node state

@codecov
Copy link

codecov bot commented Feb 20, 2018

Codecov Report

Merging #2513 into bump_v17.12 will decrease coverage by 5.28%.
The diff coverage is 50%.

@@               Coverage Diff               @@
##           bump_v17.12    #2513      +/-   ##
===============================================
- Coverage        66.92%   61.63%   -5.29%     
===============================================
  Files               76      129      +53     
  Lines            11158    21233   +10075     
===============================================
+ Hits              7467    13087    +5620     
- Misses            2929     6741    +3812     
- Partials           762     1405     +643

@anshulpundir anshulpundir merged commit a7680ec into moby:bump_v17.12 Feb 20, 2018
@thaJeztah thaJeztah deleted the 17.12-backport-fixendpointspec branch February 21, 2018 11:05
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