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

Plugins: experimental support for new plugin management #23446

Merged
merged 3 commits into from Jun 15, 2016

Conversation

Projects
None yet
@tiborvass
Collaborator

tiborvass commented Jun 10, 2016

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 comment has been minimized.

Show comment
Hide comment
@subfuzion

subfuzion Jun 10, 2016

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.

subfuzion commented Jun 10, 2016

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.

@subfuzion subfuzion referenced this pull request Jun 10, 2016

Closed

Logging driver plugins #18604

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 11, 2016

Member

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

Member

thaJeztah commented Jun 11, 2016

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

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 11, 2016

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
Member

thaJeztah commented Jun 11, 2016

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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 11, 2016

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
Member

thaJeztah commented Jun 11, 2016

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
"Type": "host"
},
"Capabilities": null,
"Mounts": [

This comment has been minimized.

@cpuguy83

cpuguy83 Jun 12, 2016

Contributor

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

@cpuguy83

cpuguy83 Jun 12, 2016

Contributor

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

@anusha-ragunathan

This comment has been minimized.

Show comment
Hide comment
Contributor

anusha-ragunathan commented Jun 13, 2016

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure Jun 14, 2016

Contributor

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

Contributor

mlaventure commented Jun 14, 2016

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

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Jun 14, 2016

Contributor

LGTM

Contributor

cpuguy83 commented Jun 14, 2016

LGTM

@vieux

This comment has been minimized.

Show comment
Hide comment
@vieux

vieux Jun 14, 2016

Collaborator

LGTM

Collaborator

vieux commented Jun 14, 2016

LGTM

@icecrime

This comment has been minimized.

Show comment
Hide comment
@icecrime

icecrime Jun 14, 2016

Contributor

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

LGTM, nice work 👍

Contributor

icecrime commented Jun 14, 2016

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

LGTM, nice work 👍

@icecrime

This comment has been minimized.

Show comment
Hide comment
@icecrime

icecrime Jun 14, 2016

Contributor

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?

Contributor

icecrime commented Jun 14, 2016

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

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>
plugins: vendor engine-api to import new plugin types
Signed-off-by: Tibor Vass <tibor@docker.com>
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 14, 2016

Member

@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

Member

thaJeztah commented Jun 14, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@icecrime

icecrime Jun 14, 2016

Contributor

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

Contributor

icecrime commented Jun 14, 2016

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

@anusha-ragunathan

This comment has been minimized.

Show comment
Hide comment
@anusha-ragunathan

anusha-ragunathan Jun 14, 2016

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?

Contributor

anusha-ragunathan commented Jun 14, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 14, 2016

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

Member

thaJeztah commented Jun 14, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 14, 2016

Member

docs LGTM (for the RC)

Member

thaJeztah commented Jun 14, 2016

docs LGTM (for the RC)

docker plugin commandline reference
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
type Errors []error
func (errs Errors) Error() string {
if len(errs) < 1 {

This comment has been minimized.

@crosbymichael

crosbymichael Jun 14, 2016

Contributor

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

@crosbymichael

crosbymichael Jun 14, 2016

Contributor

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

This comment has been minimized.

@tiborvass

tiborvass Jun 14, 2016

Collaborator

will fix in followup

@tiborvass

tiborvass Jun 14, 2016

Collaborator

will fix in followup

@cpuguy83 cpuguy83 merged commit 6ed921d into moby: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

This comment has been minimized.

Show comment
Hide comment
@HackToday

HackToday Jun 20, 2016

Contributor

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

Contributor

HackToday commented Jun 20, 2016

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

@albers

This comment has been minimized.

Show comment
Hide comment
@albers

albers Jul 3, 2016

Member

@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?

Member

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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 3, 2016

Member

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

Member

thaJeztah commented Jul 3, 2016

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

@albers

This comment has been minimized.

Show comment
Hide comment
@albers

albers Jul 4, 2016

Member

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

Member

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