Plugins: experimental support for new plugin management #23446

Merged
merged 3 commits into from Jun 15, 2016

Projects

None yet
@tiborvass
Contributor
tiborvass commented Jun 10, 2016 edited

Fixes #20363

NOTE: this PR modifies vendor and the vendor check is expected to fail. Once we agree on design we'll do the proper vendoring.

This patch introduces a new experimental engine-level plugin management
with a new API and command line. Plugins can be distributed via a Docker registry,
and their lifecycle is managed by the engine.
This makes plugins a first-class construct.

For more background, have a look at issue #20363.

Documentation is in a separate commit. If you want to understand how the new plugin
system works, you can start by reading the documentation.

Note: backwards compatibility with existing plugins is maintained, albeit they won't
benefit from the advantages of the new system.

@subfuzion

This sounds great. This seems like the general solution to the kind of thing I was hoping for when I wrote my thoughts on a generic logging driver that would let you specify the image to use for custom logging. My particular use case was to add Kafka logging support without having to wait for an official driver. Is that the right kind of use case this PR can help with? That would be awesome.

@thaJeztah
Member

@subfuzion I don't think this adds support for new type of plugins (yet), so no logging-driver plugins yet

@thaJeztah
Member

When denying the initial install, subsequent installs fail;

$ docker plugin install aragunathan/no-remove
Plugin "aragunathan/no-remove:latest" requested the following privileges:
 - Networking: host
 - Mounting host path: /data
Do you grant the above permissions? [y/N]  (pressed ENTER here, as it's the default)
Permission denied while installing plugin aragunathan/no-remove:latest

$ docker plugin install aragunathan/no-remove
Error response from daemon: aragunathan/no-remove:latest exists

Logs are below; note that my "deny permissions" does not show up in the logs,
perhaps we should add a log-entry for that, so that it can be found back:

DEBU[0031] Calling POST /v1.24/plugins/aragunathan/no-remove:latest/pull
DEBU[0032] Trying to pull aragunathan/no-remove from https://registry-1.docker.io v2
DEBU[0033] Increasing token expiration to: 60 seconds
DEBU[0033] manifest: {
   "schemaVersion": 2,
   "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
   "config": {
      "mediaType": "application/vnd.docker.plugin.v0+json",
      "size": 872,
      "digest": "sha256:073da4dede0a523c610c892bdfe1bc7dc4d7d0cf38b946be7155db197b2f2274"
   },
   "layers": [
      {
         "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
         "size": 5112374,
         "digest": "sha256:1c482de4cb9f561bc67ae257fc6462c77f66aa733d9d2d7e64432403af316f24"
      }
   ]
}
DEBU[0035] types.Plugin{Manifest:types.PluginManifest{ManifestVersion:"", Description:"", Documentation:"", Entrypoint:[]string(nil), Interface:types.PluginInterface{Types:[]types.InterfaceType(nil), Socket:""}, Network:types.PluginNetwork{Type:""}, Capabilities:[]string(nil), Mounts:[]struct { types.PluginSetting; types.PluginMount }(nil), Devices:[]struct { types.PluginSetting; types.PluginDevice }(nil), Env:[]struct { types.PluginSetting; types.PluginEnv }(nil), Args:[]struct { types.PluginSetting; types.PluginArg }(nil)}, Config:types.PluginConfig{Mounts:[]types.PluginMount(nil), Env:[]string(nil), Args:[]string(nil), Devices:[]types.PluginDevice(nil)}, Active:false, Name:"no-remove:latest", Tag:"", ID:""}
DEBU[0045] Calling GET /v1.24/info
DEBU[0045] Calling POST /v1.24/plugins/aragunathan/no-remove:latest/pull
DEBU[0045] plugin already exists
ERRO[0045] Handler for POST /v1.24/plugins/aragunathan/no-remove:latest/pull returned error: aragunathan/no-remove:latest exists

Doing remove, followed by install does not resolve this;

$ docker plugin rm aragunathan/no-remove:latest
aragunathan/no-remove:latest

$ docker plugin install aragunathan/no-remove
Error response from daemon: aragunathan/no-remove:latest exists

It looks like removing the plugin doesn't work in that case:

$ docker plugin rm aragunathan/no-remove:latest
aragunathan/no-remove:latest

$ docker plugin ls
NAME                    TAG                 ACTIVE
aragunathan/no-remove   latest              false
@thaJeztah
Member

Looks like CI is failing;

https://jenkins.dockerproject.org/job/Docker-PRs-experimental/19809/console

22:59:48 + go test -check.v -check.timeout=5m -test.timeout=360m github.com/docker/docker/integration-cli
23:00:17 # github.com/docker/docker/integration-cli
23:00:17 ./docker_cli_external_graphdriver_unix_test.go:80: unknown plugins.Plugin field 'Name' in struct literal

https://jenkins.dockerproject.org/job/Docker-PRs/28607/console

23:57:19 # github.com/docker/docker/plugin
23:57:19 plugin/manager.go:285: cannot convert spec (type specs.Spec) to type libcontainerd.Spec
23:57:19 plugin/manager.go:354: cannot use specs.Root literal (type specs.Root) as type windowsoci.Root in assignment
23:57:19 plugin/manager.go:379: cannot use s (type windowsoci.WindowsSpec) as type specs.Spec in return argument
23:57:19 plugin/manager.go:383: cannot use s (type windowsoci.WindowsSpec) as type specs.Spec in return argument
23:57:19 plugin/manager.go:387: cannot use m (type specs.Mount) as type windowsoci.Mount in append
23:57:19 plugin/manager.go:400: cannot use specs.Process literal (type specs.Process) as type windowsoci.Process in assignment
23:57:19 plugin/manager.go:402: cannot use s (type windowsoci.WindowsSpec) as type specs.Spec in return argument
23:58:05 Build step 'Execute shell' marked build as failure
@cpuguy83 cpuguy83 commented on the diff Jun 12, 2016
docs/reference/commandline/plugin_inspect.md
+ "Documentation": "https://docs.docker.com/engine/extend/plugins/",
+ "Entrypoint": [
+ "plugin-no-remove",
+ "/data"
+ ],
+ "Interface": {
+ "Types": [
+ "docker.volumedriver/1.0"
+ ],
+ "Socket": "plugins.sock"
+ },
+ "Network": {
+ "Type": "host"
+ },
+ "Capabilities": null,
+ "Mounts": [
@cpuguy83
cpuguy83 Jun 12, 2016 Contributor

We should reconcile this with docker/swarmkit#918 and docker/docker#22373

@icecrime icecrime added this to the 1.12.0 milestone Jun 13, 2016
@mlaventure
Contributor

Super long read! But couldn't find anything that poped out, since this is going into experimental, LGTM (IANAM).

@cpuguy83
Contributor

LGTM

@vieux
Member
vieux commented Jun 14, 2016

LGTM

@icecrime
Member

The UX and overall design are great. It needs polishing, but this is experimental, so let's move 😉

LGTM, nice work 👍

@icecrime
Member

We need to remove those docs, as the feature is experimental.

@tiborvass Can we please gather them in a single document under the experimental/ docs tree?

tiborvass added some commits May 16, 2016
@tiborvass tiborvass plugins: experimental support for new plugin management
This patch introduces a new experimental engine-level plugin management
with a new API and command line. Plugins can be distributed via a Docker
registry, and their lifecycle is managed by the engine.
This makes plugins a first-class construct.

For more background, have a look at issue #20363.

Documentation is in a separate commit. If you want to understand how the
new plugin system works, you can start by reading the documentation.

Note: backwards compatibility with existing plugins is maintained,
albeit they won't benefit from the advantages of the new system.

Signed-off-by: Tibor Vass <tibor@docker.com>
Signed-off-by: Anusha Ragunathan <anusha@docker.com>
f371170
@tiborvass tiborvass plugins: vendor engine-api to import new plugin types
Signed-off-by: Tibor Vass <tibor@docker.com>
e5b7d36
@thaJeztah
Member
thaJeztah commented Jun 14, 2016 edited

@tiborvass @icecrime you can also add an

advisory="experimental"

meta header; we added an option to the template to show a "this is an experimental feature" warning

@icecrime
Member

I think it's better to do as usual and have it in a single page under experimental/, but as you wish.

@anusha-ragunathan
Contributor

@thaJeztah : experimental advisory can be added to plugin docs. but the "support for advisory" PR is in docker/docs-base-1.12-integration. Do you know when it'll be merged to docker/docker?

@thaJeztah
Member

@anusha-ragunathan is see there was a PR to do that but closed, docker/docs-base#263, perhaps the 1.12-docs-base is going to be used, let me check

@thaJeztah
Member

docs LGTM (for the RC)

@thaJeztah @tiborvass thaJeztah docker plugin commandline reference
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
e79873c
@crosbymichael crosbymichael commented on the diff Jun 14, 2016
cli/error.go
@@ -0,0 +1,21 @@
+package cli
+
+import "bytes"
+
+// Errors is a list of errors.
+// Useful in a loop if you don't want to return the error right away and you want to display after the loop,
+// all the errors that happened during the loop.
+type Errors []error
+
+func (errs Errors) Error() string {
+ if len(errs) < 1 {
@crosbymichael
crosbymichael Jun 14, 2016 Member

nit: strings.Join() for this so you are not going from string to byte to string,etc

@tiborvass
tiborvass Jun 14, 2016 Contributor

will fix in followup

@cpuguy83 cpuguy83 merged commit 6ed921d into docker:master Jun 15, 2016

8 of 9 checks passed

experimental Jenkins build Docker-PRs-experimental 20005 is running
Details
docker/dco-signed All commits signed
Details
documentation success
Details
gccgo Jenkins build Docker-PRs-gccgo 6870 has succeeded
Details
janky Jenkins build Docker-PRs 28800 has succeeded
Details
userns Jenkins build Docker-PRs-userns 10960 has succeeded
Details
vendor Jenkins build Docker-PRs-vendor 1179 has succeeded
Details
win2lin Jenkins build Docker-PRs-Win2Lin 27388 has succeeded
Details
windowsTP5 Jenkins build Docker-PRs-WoW-TP5 3287 has succeeded
Details
@HackToday
Contributor

hi @thaJeztah and @tiborvass where can I found the documentation about this ?

@albers
Contributor
albers commented Jul 3, 2016

@thaJeztah I'm not sure how I should handle experimental features in bash completion.
There is no notion of an "experimental completion", so I fear that pulling support for experimental features into the completion might cause problems in release management.

If I do a default build from the sources, I will get no experimental features. Having them supported in the completion would mean an inconsistency.

My current policy is to ignore experimental features in bash completion. WDYT?

@thaJeztah
Member

@albers yes, I think we should indeed ignore the completion for now

@albers
Contributor
albers commented Jul 4, 2016

@thaJeztah yeah, I think maintaining two variants would not be adequate. Thanks for confirming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment