Networkallocator must make use of plugin-v2 apis by mavenugo · Pull Request #1876 · moby/swarmkit

mavenugo · 2017-01-18T22:06:34Z

This is a followup PR for #1867 and its companion docker PR moby/moby#30145 to resolve moby/moby#30024.

This PR carries

the patch from @anusha-ragunathan (Add plugingetter to swarm node config. #1867)
changes required to pass on the plugingetter to networkallocator
vendoring docker/docker to consume the changes intrduced by plugingetter: Avoid all caps for constant declarations moby#29564

Signed-off-by: Madhu Venugopal <madhu@docker.com>

This is necessary for swarmkit to support cluster wide plugins, such as globally scoped network plugins. Signed-off-by: Anusha Ragunathan <anusha.ragunathan@docker.com>

mavenugo · 2017-01-18T22:07:30Z

Test result using weave managed plugin -

 $ sudo docker plugin install bboreham/weave2
Plugin "bboreham/weave2" is requesting the following privileges:
 - network: [host]
 - capabilities: [CAP_SYS_ADMIN CAP_NET_ADMIN]
Do you grant the above permissions? [y/N] y
latest: Pulling from bboreham/weave2
7718f575adf7: Download complete
Digest: sha256:2780330cc15644b60809637ee8bd68b4c85c893d973cb17f2981aabfadfb6d72
Status: Downloaded newer image for bboreham/weave2:latest
Installed plugin bboreham/weave2

 $ sudo docker network create --driver=bboreham/weave2:latest foo
gek8emqogme6dmdhjjqi9mchl

 $ sudo docker service create --network foo --name sw2 mrjana/simpleweb simpleweb
hjpzskl7zkkrgcaiykj31zmgj
 $ sudo docker service lsID            NAME  MODE        REPLICAS  IMAGE
hjpzskl7zkkr  sw2   replicated  1/1       mrjana/simpleweb:latest

 $ sudo docker service create --network foo --name sw3 mrjana/simpleweb simpleweb
q8mfk0e6h30vdid50g11rew5n
madhu@Ubuntu-vm swarmkit (plugin-v2) $ sudo docker service ls
ID            NAME  MODE        REPLICAS  IMAGE
hjpzskl7zkkr  sw2   replicated  1/1       mrjana/simpleweb:latest
q8mfk0e6h30v  sw3   replicated  1/1       mrjana/simpleweb:latest

madhu@Ubuntu-vm swarmkit (plugin-v2) $ sudo docker ps
CONTAINER ID        IMAGE                                                                                      COMMAND             CREATED             STATUS              PORTS               NAMES
62c3d78b376a        mrjana/simpleweb@sha256:317d7f221d68c86d503119b0ea12c29de42af0a22ca087d522646ad1069a47a4   "simpleweb"         8 seconds ago       Up 7 seconds                            sw3.1.0i9skstty1jauf9sny4o566zs
cc4794f73672        mrjana/simpleweb@sha256:317d7f221d68c86d503119b0ea12c29de42af0a22ca087d522646ad1069a47a4   "simpleweb"         4 hours ago         Up 5 hours                              sw2.1.nc9jk48x21uykwabmnxal3oz2

madhu@Ubuntu-vm swarmkit (plugin-v2) $ sudo docker exec -it cc4794f73672 sh
/ # ping sw3
PING sw3 (10.0.1.4): 56 data bytes
64 bytes from 10.0.1.4: seq=0 ttl=64 time=0.066 ms
64 bytes from 10.0.1.4: seq=1 ttl=64 time=0.145 ms
64 bytes from 10.0.1.4: seq=2 ttl=64 time=0.099 ms
^C
--- sw3 ping statistics ---
3 packets transmitted, 3 packets received, 0% packet loss
round-trip min/avg/max = 0.066/0.103/0.145 ms
/ #

## WITH ROUTING-MESH

$ sudo docker service create --network foo --name sw4 -p 7000:5000  mrjana/simpleweb simpleweb
ol45q4h9ojy1xjdyuaepxtpq7

$ curl 127.0.0.1:7000
Hitting backend 10.255.0.6 (count 1)
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.6 (count 2)
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.6 (count 3)

$ sudo docker service scale sw4=3
sw4 scaled to 3
$
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.8 (count 1)
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.7 (count 1)
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.6 (count 4)
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.8 (count 2)
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.7 (count 2)
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.6 (count 5)
$ curl 127.0.0.1:7000
Hitting backend 10.255.0.8 (count 3)

aaronlehmann · 2017-01-18T22:10:07Z

 	doneChan chan struct{}
+
+	// PluginGetter provides access to docker's plugin inventory.
+	PluginGetter plugingetter.PluginGetter


Does this need to be exported?

aaronlehmann · 2017-01-18T22:11:15Z

+		_, err = pg.Get(name, driverapi.NetworkPluginEndpointType, plugingetter.Lookup)
+	} else {
+		_, err = plugins.Get(name, driverapi.NetworkPluginEndpointType)
+	}


What are the semantics of doing it like this? Does it mean that if we have a PluginGetter, we only support v2 plugins, and without a PluginGetter, only v1 plugins are supported? Or is there some compatibility mechanism built into pg.Get?

Yes. that is the idea. But plugingetter actually does its work by querying the plugins package internally if it cannot find a corresponding managed plugin. I can actually remove the call to plugin-v1 package.

Can you also update the other instances of this pattern in libnetwork? Eg. https://github.com/docker/libnetwork/blob/4da563bb06cc7e488047ddf3a20cde2f9d9928cc/controller.go#L1089

FYI, the older pattern was necessary when plugins were experimental and both stable and experimental daemons had to work with plugins. Today, this is not necessary.

aaronlehmann · 2017-01-18T22:44:24Z

It looks like the unit test needs to be updated.

manager/allocator/allocator_test.go:24: not enough arguments in call to New

Signed-off-by: Madhu Venugopal <madhu@docker.com>

codecov-io · 2017-01-18T23:10:37Z

Current coverage is 54.85% (diff: 36.66%)

Merging #1876 into master will increase coverage by 0.12%

@@             master      #1876   diff @@
==========================================
  Files           107        107          
  Lines         17608      17611     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           9636       9660    +24   
+ Misses         6792       6783     -9   
+ Partials       1180       1168    -12

Powered by Codecov. Last update 5a1ea1f...f250807

mavenugo · 2017-01-18T23:11:16Z

@aaronlehmann addressed your comments and the CI is green now.

aaronlehmann · 2017-01-18T23:12:06Z

LGTM

anusha-ragunathan · 2017-01-19T19:31:04Z

LGTM

dperny · 2017-01-19T19:41:38Z

LGTM, nothing obviously wrong with it.

Cherry-picking #1876 and reverts #1856 in 1.13.1 branch

- backport of moby#1876 commit: 7541809 commit: c6aacc7 commit: f250807 To handle the type deletion happened in docker/docker and not updated in swarmkit. The docker vendoring was needed due to a dependency on a method requested by libnetwork vendoring Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>

mavenugo and others added 2 commits January 18, 2017 13:59

Vendoring docker/docker to carry docker#29564

c6aacc7

Signed-off-by: Madhu Venugopal <madhu@docker.com>

Add plugingetter to swarm node config.

7541809

This is necessary for swarmkit to support cluster wide plugins, such as globally scoped network plugins. Signed-off-by: Anusha Ragunathan <anusha.ragunathan@docker.com>

aaronlehmann reviewed Jan 18, 2017

View reviewed changes

NetworkAllocator must make use of plugin-v2

f250807

Signed-off-by: Madhu Venugopal <madhu@docker.com>

mavenugo force-pushed the plugin-v2 branch from 8c79ad4 to f250807 Compare January 18, 2017 23:01

aaronlehmann mentioned this pull request Jan 19, 2017

Add plugingetter to swarm node config. #1867

Closed

aaronlehmann added this to the 1.13.1 milestone Jan 19, 2017

aaronlehmann added the process/cherry-pick label Jan 19, 2017

aaronlehmann merged commit 5643b30 into moby:master Jan 19, 2017

This was referenced Jan 19, 2017

Revert "Gracefully fail if Network and IPAM plugins are used" #1881

Merged

Cherry-picking #1876 and reverts #1856 in 1.13.0 branch #1882

Closed

Cherry-picking #1876 and reverts #1856 in 1.13.1 branch #1883

Merged

aaronlehmann pushed a commit that referenced this pull request Jan 20, 2017

Merge pull request #1883 from mavenugo/1.13.1-plugin

0692326

Cherry-picking #1876 and reverts #1856 in 1.13.1 branch

aaronlehmann added process/cherry-picked and removed process/cherry-pick labels Jan 20, 2017

sgammill mentioned this pull request Jan 30, 2017

Create a V2 Docker plugin weaveworks/weave#2727

Merged

7 tasks done

fcrisciani mentioned this pull request Nov 16, 2017

[Backport 17.03] Adding ipam options to ipam driver requests #2449

Closed

Navigation Menu

Repository navigation

Networkallocator must make use of plugin-v2 apis - #1876

Networkallocator must make use of plugin-v2 apis#1876
aaronlehmann merged 3 commits into
moby:masterfrom
mavenugo:plugin-v2

mavenugo commented Jan 18, 2017

Uh oh!

mavenugo commented Jan 18, 2017

Uh oh!

aaronlehmann Jan 18, 2017

Uh oh!

aaronlehmann Jan 18, 2017

Uh oh!

mavenugo Jan 18, 2017

Uh oh!

anusha-ragunathan Jan 18, 2017

Uh oh!

anusha-ragunathan Jan 19, 2017

Uh oh!

aaronlehmann commented Jan 18, 2017

Uh oh!

codecov-io commented Jan 18, 2017

Uh oh!

mavenugo commented Jan 18, 2017

Uh oh!

aaronlehmann commented Jan 18, 2017

Uh oh!

anusha-ragunathan commented Jan 19, 2017

Uh oh!

dperny commented Jan 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

mavenugo commented Jan 18, 2017

Uh oh!

mavenugo commented Jan 18, 2017

Uh oh!

aaronlehmann Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

aaronlehmann Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

mavenugo Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

anusha-ragunathan Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

anusha-ragunathan Jan 19, 2017

Choose a reason for hiding this comment

Uh oh!

aaronlehmann commented Jan 18, 2017

Uh oh!

codecov-io commented Jan 18, 2017

Current coverage is 54.85% (diff: 36.66%)

Uh oh!

mavenugo commented Jan 18, 2017

Uh oh!

aaronlehmann commented Jan 18, 2017

Uh oh!

anusha-ragunathan commented Jan 19, 2017

Uh oh!

dperny commented Jan 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants