Skip to content
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

move plugins out of experimental #28226

Merged
merged 2 commits into from Nov 11, 2016
Merged

move plugins out of experimental #28226

merged 2 commits into from Nov 11, 2016

Conversation

@vieux
Copy link
Collaborator

vieux commented Nov 10, 2016

@thaJeztah

This comment has been minimized.

Copy link
Member

thaJeztah commented Nov 10, 2016

Can you check integration tests as well? Tests are probably skipped if experimental is not enabled

@vieux

This comment has been minimized.

Copy link
Collaborator Author

vieux commented Nov 10, 2016

@thaJeztah right, thanks

@vieux vieux force-pushed the vieux:exit_exp_plugin branch from b71079e to 3f3a993 Nov 10, 2016
Copy link
Member

thaJeztah left a comment

LGTM

@thaJeztah

This comment has been minimized.

Copy link
Member

thaJeztah commented Nov 10, 2016

/cc @albers @sdurrheimer because this will likely affect the completion scripts

@stevvooe

This comment has been minimized.

Copy link
Contributor

stevvooe commented Nov 10, 2016

This cannot be merged until #28148 is merged.

@anusha-ragunathan

This comment has been minimized.

Copy link
Contributor

anusha-ragunathan commented Nov 10, 2016

Please add 26760, which tracks this.

@vieux vieux force-pushed the vieux:exit_exp_plugin branch from 3f3a993 to b77508a Nov 10, 2016
@@ -6,7 +6,7 @@ keywords: "API, Docker, rcli, REST, documentation"

<!-- This file is maintained within the docker/docker Github
repository at https://github.com/docker/docker/. Make all
pull requests against that repo. If you see this file in
5B pull requests against that repo. If you see this file in

This comment has been minimized.

Copy link
@tiborvass

tiborvass Nov 10, 2016

Collaborator

?

@anusha-ragunathan

This comment has been minimized.

Copy link
Contributor

anusha-ragunathan commented Nov 10, 2016

tests that need to be updated:

  • docker_cli_daemon_experimental_test.go (all of them)
  • docker_cli_events_test.go (TestEventsPluginOps)
  • docker_cli_authz_plugin_v2_test.go (all of them)
  • docker_cli_network_unix_test.go (TestDockerPluginV2NetworkDriver)
@anusha-ragunathan

This comment has been minimized.

Copy link
Contributor

anusha-ragunathan commented Nov 10, 2016

daemon/daemon_experimental.go needs to remove the daemon.HasExperimental checks and merge with daemon/daemon.go. Else nothing will work.

* `GET /plugins/(plugin name)` inspect a plugin.
* `POST /plugins/(plugin name)/set` configure a plugin.
* `POST /plugins/(plugin name)/enable` enable a plugin.
* `POST /plugins/(plugin name)/disable` disable a plugin.

This comment has been minimized.

Copy link
@anusha-ragunathan

anusha-ragunathan Nov 10, 2016

Contributor

Add plugin create:
``POST /plugins/create?name=plugin name creates a plugin.

This comment has been minimized.

Copy link
@anusha-ragunathan

anusha-ragunathan Nov 10, 2016

Contributor

Also missing push

@@ -184,6 +184,14 @@ This section lists each version from latest to oldest. Each listing includes a
* `POST /services/create` and `POST /services/(id or name)/update` now accept the `TTY` parameter, which allocate a pseudo-TTY in container.
* `POST /services/create` and `POST /services/(id or name)/update` now accept the `DNSConfig` parameter, which specifies DNS related configurations in resolver configuration file (resolv.conf) through `Nameservers`, `Search`, and `Options`.
* `GET /networks/(id or name)` now includes IP and name of all peers nodes for swarm mode overlay networks.
* `GET /plugins` list plugins.
* `POST /plugins/pull?name=<plugin name>` install a plugin.

This comment has been minimized.

Copy link
@anusha-ragunathan

anusha-ragunathan Nov 10, 2016

Contributor

should be pulls a plugin. install implies an additional enable.

@@ -171,7 +170,6 @@ Config provides the base accessible fields for working with V0 plugin format

```
{
"configVersion": "v0",

This comment has been minimized.

Copy link
@anusha-ragunathan

anusha-ragunathan Nov 10, 2016

Contributor

Why is this being removed?

This comment has been minimized.

Copy link
@vieux

vieux Nov 10, 2016

Author Collaborator

It's a doc left-over from a previous PR, it's already removed from the code, after talking with @tiborvass and @stevvooe.

  • You needed to json-decode in order to get the version, if the format change a lot, it won't even work.
  • The version is already present in the media-type, that's how other media handle versioning.
@vieux vieux force-pushed the vieux:exit_exp_plugin branch from b77508a to d949d07 Nov 10, 2016
@vieux

This comment has been minimized.

Copy link
Collaborator Author

vieux commented Nov 10, 2016

Thanks @anusha-ragunathan, I missed a lot indeed

@anusha-ragunathan

This comment has been minimized.

Copy link
Contributor

anusha-ragunathan commented Nov 10, 2016

Thank you for doing this @vieux . I know you've been burning the midnight oil with a lotta plugin PRs!

@vieux vieux force-pushed the vieux:exit_exp_plugin branch from 9f276f0 to 9375710 Nov 10, 2016
Copy link
Member

vdemeester left a comment

LGTM 🐸

* `POST /plugins/(plugin name)/set` configure a plugin.
* `POST /plugins/(plugin name)/enable` enable a plugin.
* `POST /plugins/(plugin name)/disable` disable a plugin.
* `POST /plugins/(plugin name)/push` push a pluging.

This comment has been minimized.

Copy link
@anusha-ragunathan

anusha-ragunathan Nov 10, 2016

Contributor

typo pluging

@@ -153,7 +153,8 @@ func Pull(ref reference.Named, rs registry.Service, metaheader http.Header, auth
logrus.Debugf("pull.go: error in json.Unmarshal(): %v", err)
return nil, err
}
if m.Config.MediaType != schema2.MediaTypePluginConfig {
if m.Config.MediaType != schema2.MediaTypePluginConfig &&
m.Config.MediaType != "application/vnd.docker.plugin.image.v0+json" { //TODO: remove this v0 before 1.13 GA

This comment has been minimized.

Copy link
@anusha-ragunathan

anusha-ragunathan Nov 10, 2016

Contributor

A similar check for image.v0 needs to be added in https://github.com/vieux/docker/blob/exit_exp_plugin/distribution/pull_v2.go#L361, to avoid using docker pull to pull plugin images.

This comment has been minimized.

Copy link
@vieux

vieux Nov 10, 2016

Author Collaborator

thanks

@vieux vieux force-pushed the vieux:exit_exp_plugin branch 4 times, most recently from 0bc70b9 to 683167c Nov 10, 2016
@anusha-ragunathan

This comment has been minimized.

Copy link
Contributor

anusha-ragunathan commented Nov 10, 2016

LGTM (once tests are all green)

@tiborvass

This comment has been minimized.

Copy link
Collaborator

tiborvass commented Nov 10, 2016

@vieux needs rebase :(

@tiborvass

This comment has been minimized.

Copy link
Collaborator

tiborvass commented Nov 10, 2016

apart from small conflict, LGTM

@lowenna

This comment has been minimized.

Copy link
Contributor

lowenna commented Nov 10, 2016

As plugins don't work on Windows (at least I've never verified and don't believe they do anyway), can we get this updated to report a nice "not supported on this platform" message for the API endpoints? And document accordingly.

vieux added 2 commits Nov 10, 2016
Signed-off-by: Victor Vieux <vieux@docker.com>
Signed-off-by: Victor Vieux <vieux@docker.com>
@vieux

This comment has been minimized.

Copy link
Collaborator Author

vieux commented Nov 10, 2016

@jhowardmsft yes it's on my list of things to do during the freeze

@vieux vieux force-pushed the vieux:exit_exp_plugin branch from 683167c to 970b23d Nov 10, 2016
@lowenna

This comment has been minimized.

Copy link
Contributor

lowenna commented Nov 11, 2016

Thanks, @vieux

@vieux vieux merged commit acd6e0d into moby:master Nov 11, 2016
5 checks passed
5 checks passed
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 26632 has succeeded
Details
janky Jenkins build Docker-PRs 35203 has succeeded
Details
vendor Jenkins build Docker-PRs-vendor 2456 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 6119 has succeeded
Details
@vieux vieux deleted the vieux:exit_exp_plugin branch Nov 11, 2016
@tomdee

This comment has been minimized.

Copy link
Contributor

tomdee commented Nov 16, 2016

@vieux Am I right in thinking that network plugins still aren't supported?

@mavenugo

This comment has been minimized.

Copy link
Contributor

mavenugo commented Nov 16, 2016

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.