Refactor plugin to create veths in CreateEndpoint #2412
Conversation
| @@ -206,7 +211,7 @@ func (driver *driver) DiscoverDelete(disco *api.DiscoveryNotification) error { | |||
| } | |||
|
|
|||
| func vethPair(id string) (string, string) { | |||
| return "vethwl" + id[:5], "vethwg" + id[:5] | |||
| return "vethwl" + id[:8], "vethwg" + id[:8] | |||
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.
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.
|
Would this be safe & worthwhile for 1.6.1? |
No customer has asked for this feature; I just noticed it when trying to re-implement |
bboreham
force-pushed
the
1803-plugin-create
branch
from
8f45b9e
to
13801ce
Compare
| if err := netlink.LinkDel(veth); err != nil { | ||
| // Try again using the name construction from earlier plugin version, | ||
| // in case user has upgraded with endpoints still extant | ||
| veth.Name = "vethwl" + deleteReq.EndpointID[:5] |
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.
Import library to get max name length IFNAMSIZ
bboreham
force-pushed
the
1803-plugin-create
branch
from
13801ce
to
afe882c
Compare
|
The switch from Join/Leave to Create/Delete makes me nervous. Does this introduce any failure modes / races / leaks? |
Well, that was the whole reason this change was pulled the last time. Then the Docker Overlay plugin was changed from create in Join and delete in Leave to do the veth deletion in Delete, and I asked if it mattered and didn't get much of an answer. The Docker calling code calls As an alternative, we could choose the MAC using the bits from the endpoint ID. |
Re-doing #1808, which was pulled because we were uncertain about the semantics of create/delete vs join/leave. Subsequent Docker changes indicate that this is not something to worry about.
Fixes #1803.
By moving endpoint creation into CreateEndpoint we can return the MAC address at the time Docker expects. We find the endpoint later simply by looking it up with the same name, derived from the endpoint ID.
The second commit was also tagged onto #1808; changes from using 5 chars from the endpoint-ID to using 8, to reduce the chance of collision from ~1/1000 to ~1/64K. This is not backwards-compatible, e.g. if you stop/upgrade/start the plugin it will log a warning and leak the veth when asked to remove an endpoint.