This repository has been archived by the owner on Jun 20, 2024. It is now read-only.
Claim existing IP addresses at startup with no race condition #2787
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrislovecnm
suggested changes
Feb 9, 2017
| @@ -0,0 +1,54 @@ | |||
| package main | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
bboreham
force-pushed
the
issues/2784-preclaim-addresses
branch
from
9b56239 to
0831f81
Compare
doing it from the script Note we need to run weaver in the host pid namespace, so it can search for existing container IPs. Also now we are firing off sub-processes it's better to not be PID 1 so Linux doesn't expect us to reap zombies.
bboreham
force-pushed
the
issues/2784-preclaim-addresses
branch
2 times, most recently
from
3552773 to
6ebf06e
Compare
bboreham
changed the title
bboreham
force-pushed
the
issues/2784-preclaim-addresses
branch
from
6ebf06e to
43ea755
Compare
The intention, per the comments, is that host1 should claim from host2. But host2 and host3 can carve up the space either way round. If host1 claims from host3 then there is a race whether anyone finds out about it. Adding a 'prime' on host2 ensures the space is all owned by host2 at that point.
bboreham
force-pushed
the
issues/2784-preclaim-addresses
branch
from
43ea755 to
293abd9
Compare
brb
reviewed
Feb 13, 2017
ipam/allocator.go
Outdated
| @@ -76,13 +76,20 @@ type Allocator struct { | |||
| now func() time.Time | |||
| } | |||
|
|
|||
| type PreClaim struct { | |||
| Ident string | |||
| IsContainer bool | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| } | ||
| preClaims, err := findExistingAddresses(dockerCli, allContainerIDs, weavenet.WeaveBridgeName) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| // Now iterate over all containers to see if they have a network | ||
| // namespace with an attached interface | ||
| if dockerCli != nil { | ||
| for _, cid := range containerIDs { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
brb
approved these changes
Feb 13, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2784
reclaim.gois a re-implementation of thelaunch.shcode to claim addresses in Go.We already had a
pendingClaimslist in IPAM, so we simply pre-populate this with entries for each existing address. The claims will be processed as soon as the ring is known.As it stands, this code is only good enough for
weave-kube, not forweave launchThe code to find existing IPs works through the process table, and thus we do not have the Docker container ID. To resolve that I propose to add another loop to go through Docker container IDs if we have a Docker API connection.