Plugins on swarm #33575

cpuguy83 · 2017-06-07T17:23:09Z

Support deploying plugins via swarm services

Modifies the services API to support deploying plugins (previously only
supported containers).
This makes use of some of the existing work around generic service
runtimes.

With this change you can curl -X POST myDocker:2375/services/create with a
PluginSpec defined in the TaskTemplate.
The plugin defined by in the PluginSpec will be deployed similar to a
"global mode" service.

Work still needed:

More tests (test constraints, others)
Deal with some race conditions in the plugin manager
Better way to filter swarm managed plugins from non-swarm managed plugins
Support enable/disable via services API?
Handle (old) task removes (ie, don't remove a plugin that has an active task)

The format of the plugin request that is sent to swarm is currently just a json encoded object specified in api/types/swarm... wondering if it would be better to change this to protobuf.

Ping @ehazlett @tiborvass

What this does not tackle:

UX, CLI is not handled here, only API's are added... even API's could maybe use some tweaks but worried about back-compat for older clients... specifically on service and task listings which currently only list services/tasks with the container runtime unless you set a runtime in the list filter.
Secrets, configs, mounts, networks, etc... this requires deeper changes to plugins and can be done separately.

cpuguy83 · 2017-06-08T18:46:22Z

Ok, I think this is ready for design review, and code is in a pretty good state.

ping @aaronlehmann

ehazlett · 2017-06-08T19:58:51Z

Sorry for the delay. +1 for pb encoding. In the "original" design, that was the intention we just left it out of the Generic Runtime PR as we didn't have anything to use yet.

aaronlehmann · 2017-06-08T21:55:50Z

wondering if it would be better to change this to protobuf.

Yes, I think protobuf would be better.

aaronlehmann · 2017-06-08T21:58:38Z

api/types/swarm/task.go

@@ -66,7 +66,8 @@ type TaskSpec struct {
 	// parameters have been changed.
 	ForceUpdate uint64

-	Runtime RuntimeType `json:",omitempty"`
+	Runtime    RuntimeType `json:",omitempty"`
+	PluginSpec *PluginSpec `json:",omitempty"`


I'd suggest putting this next to ContainerSpec in the struct definition and adding a comment that they are mutually exclusive. We should probably change ContainerSpec to be a pointer (and make sure no code assumes it exists).

aaronlehmann · 2017-06-08T22:00:13Z

daemon/cluster/controllers/plugin/controller.go

 	logrus.WithFields(logrus.Fields{
 		"controller": "plugin",
 	}).Debug("Prepare")
+
+	// TODO(@cpuguy83): support digest pinning


Looks like tag+digest is already supported. What else needs to be done?

Should we pin this before sending off to swarm?

Pinning happens on the CLI side starting with API 1.30.

(See docker/cli#30)

Ok makes sense to do that there.

aaronlehmann · 2017-06-08T22:02:21Z

daemon/cluster/controllers/plugin/controller.go

+	}
+
+	if canonical, ok := remote.(canonicalWithTag); ok {
+		remote, err = reference.WithDigest(reference.TrimNamed(remote), canonical.Digest())


It should be possible to pass a reference with both a tag and a digest to the pull code. This wasn't working properly earlier, but it was fixed recently.

Not sure if there are any plugin-specific issues with doing this, but ideally there shouldn't be.

So you are saying to drop the TrimNamed bit?

aaronlehmann · 2017-06-08T22:04:48Z

daemon/cluster/controllers/plugin/controller_test.go

+		if err != context.Canceled {
+			t.Fatal(err)
+		}
+	case <-time.After(500 * time.Millisecond):


Is 500 ms enough? I like to be very conservative with timeouts in test so that loaded machines don't cause spurious failures.

aaronlehmann · 2017-06-08T22:06:17Z

daemon/cluster/convert/service.go

+			switch g.Kind {
+			case string(types.RuntimePlugin):
+				var p types.GenericPluginPayload
+				json.Unmarshal(g.Payload.Value, &p)


I guess we'll change this to gRPC anyway, but the error needs to be checked.

aaronlehmann · 2017-06-08T22:07:52Z

daemon/cluster/services.go

@@ -50,14 +50,16 @@ func (c *Cluster) GetServices(options apitypes.ServiceListOptions) ([]types.Serv
 		return nil, err
 	}

+	if len(options.Filters.Get("runtime")) == 0 {
+		// Default to using the container runtime filter


Wondering if we should do this in the CLI instead of the API. No strong preference though.

I would prefer the CLI, but I didn't want to make a breaking change to the API here.

aaronlehmann · 2017-06-08T22:10:42Z

integration-cli/docker_api_swarm_service_test.go

+	d2 := s.AddDaemon(c, true, true)
+	d3 := s.AddDaemon(c, true, false)
+
+	time.Sleep(1 * time.Second) // make sure all daemons are ready to accept


I don't think this sleep is necessary. I don't recall any other tests that create services doing this.

Oh I copied this from the test above it. I can remove.

aaronlehmann · 2017-06-08T22:12:20Z

integration-cli/docker_api_swarm_service_test.go

+		s.Spec.TaskTemplate.Runtime = "plugin"
+		s.Spec.TaskTemplate.PluginSpec = &swarm.PluginSpec{
+			Name:   "test",
+			Remote: "cpuguy83/docker-logdriver-test",


I'm hoping we can avoid adding new dependencies on Docker Hub in tests; see #30626

Yeah, I keep putting this off... I'll work on something for this.

Ok, added a plugin builder + some registry integration. Check it out in integration-cli/fixtures/plugin.

aaronlehmann · 2017-06-08T22:14:04Z

plugin/events.go

+	"github.com/docker/docker/api/types"
+)
+
+// Event is emitted for actions performed on the


I remember this truncated comment from swarmkit :)

Such a coincidence :)

This commit extends SwarmKit secret management with pluggable secret backends support, using the current cluster configuration to understand if secrets are being stored locally, or need to be retrieved from a remote secret store. The solution will use the existing docker plugin framework. The approach is to add a new `driver` parameter to existing secrets, which defines whether the values are loaded from SwarmKit or from one of the secret plugins. The loading of secrets is done using the standard docker plugin infrastructure, which is already accessible in swarmkit and used in other flows (e.g., networking) Remarks: * I've added support to mock the plugin subsystem in controlapi server, I preferred this approach over loading the full plugin subsystem. Work still needed in this CR: - [ ] More unit tests (pending initial iteration) - [ ] Customized error handling Work still needed to complete this feature: - [ ] Inject secrets as part of plugin initialization - [ ] CLI support in docker References: moby/moby#33575 Signed-off-by: liron <liron@twistlock.com>

This commit extends SwarmKit secret management with pluggable secret backends support. The solution uses the existing docker plugin framework for loading plugins and the existing SwarmKit data backend for storing them. The approach is to add a new `driver` parameter to existing secrets, which defines whether the values are taken as is or fetched from one of the secret plugins. The loading of secrets is done using the standard docker plugin infrastructure, which is already accessible in SwarmKit and used in other flows (e.g., networking). The fetched values are are stored as regular SwarmKit secrets. Remarks: * I've added support for mocking the plugin subsystem when settings up the controlapi server. I preferred this approach over loading the full plugin subsystem in UT. Work still needed in this CR: - [ ] More unit tests (pending initial iteration) - [ ] Customized error handling (e.g., customize error string for Not Found) Work still needed to complete this feature: - [ ] Inject secrets as part of plugin initialization - [ ] CLI support in docker - [ ] Docs - [ ] Support scheduling plugins in swarm moby/moby#33575 Signed-off-by: liron <liron@twistlock.com>

cpuguy83 · 2017-07-06T18:27:01Z

Rebased, fixed imports, added client side validation for service spec to create/update.

yongtang · 2017-07-07T09:22:58Z

All tests passed after windowsRS1 rerun. Merging.

rogaha · 2017-07-07T12:57:34Z

💥 thanks @cpuguy83 for putting this together!

cpuguy83 · 2017-07-07T13:10:27Z

Just step 1, still need:

Support for plugins in private repos
Secrets
Networking
CLI

rogaha · 2017-07-07T13:14:06Z

@cpuguy83 do we have issues for those remaining pieces?

carlfischer1 · 2017-07-07T17:16:27Z

@rogaha @cpuguy83 for Secrets we have #30796

This commit extends SwarmKit secret management with pluggable secret backends support. The solution uses the existing docker plugin framework for loading plugins and the existing SwarmKit data backend for storing them. The approach is to add a new `driver` parameter to existing secrets, which defines whether the values are taken as is or fetched from one of the secret plugins. The loading of secrets is done using the standard docker plugin infrastructure, which is already accessible in SwarmKit and used in other flows (e.g., networking). The fetched values are evaluated before assigning them to worker nodes, so the payload is not stored in the raft store. Remarks: * I've added support for mocking the plugin subsystem when settings up the controlapi server. I preferred this approach over loading the full plugin subsystem in UT. Work still needed in this CR: - [ ] More unit tests (pending initial iteration) - [ ] Customized error handling (e.g., customize error string for Not Found) Work still needed to complete this feature: - [ ] Inject secrets as part of plugin initialization - [ ] CLI support in docker - [ ] Docs - [ ] Support scheduling plugins in swarm moby/moby#33575 Signed-off-by: liron <liron@twistlock.com>

dnephin · 2017-07-26T20:08:13Z

client/service_update.go

+	}
+	if len(imgPlatforms) > 0 {
+		service.TaskTemplate.Placement.Platforms = imgPlatforms
+	}


This (and the other changes to this file) seem like the wrong place to handle these concerns. Every other function in this package is only concerned with translating a golang struct into an HTTP request, performing the request, and translating the response into a goland struct.

With this change we've introduced a bunch of application logic and validation that does not belong here.

I think this needs to be extracted to some other package.

cc @thaJeztah re docker/cli#386

This is all pre-existing, just re-organized.

Ah, my mistake, looks like it was first introduced and modified in a few PRs starting around #33239

dnephin · 2017-07-26T20:09:12Z

client/service_create.go

+	}
+
+	if err := validateServiceSpec(service); err != nil {
+		return types.ServiceCreateResponse{}, err


same problem here

GordonTheTurtle added the status/0-triage label Jun 7, 2017

cpuguy83 added status/1-design-review and removed status/0-triage labels Jun 7, 2017

cpuguy83 force-pushed the plugins_on_swarm branch 4 times, most recently from 8322070 to 977fcc8 Compare June 8, 2017 02:19

cpuguy83 changed the title ~~[WIP] Plugins on swarm~~ Plugins on swarm Jun 8, 2017

cpuguy83 force-pushed the plugins_on_swarm branch 3 times, most recently from 9ab7953 to 97dec15 Compare June 8, 2017 18:37

cpuguy83 force-pushed the plugins_on_swarm branch 2 times, most recently from efcd344 to 9b02932 Compare June 8, 2017 18:54

aaronlehmann reviewed Jun 8, 2017

View reviewed changes

liron-l mentioned this pull request Jun 11, 2017

Pluggable secret backend moby/swarmkit#2239

Merged

6 tasks

cpuguy83 force-pushed the plugins_on_swarm branch from f4a5e00 to 72c3bcf Compare July 6, 2017 18:26

aaronlehmann approved these changes Jul 6, 2017

View reviewed changes

yongtang added the rebuild/windowsRS1 label Jul 7, 2017

GordonTheTurtle removed the rebuild/windowsRS1 label Jul 7, 2017

yongtang merged commit 05c7c31 into moby:master Jul 7, 2017

cpuguy83 mentioned this pull request Jul 7, 2017

Plugins on swarm with private repos #34010

Open

cpuguy83 deleted the plugins_on_swarm branch July 7, 2017 14:40

jamiegs mentioned this pull request Jul 13, 2017

Switch TaskSpec to use pointers for ContainerSpec so tests build successfully. fsouza/go-dockerclient#657

Merged

cyli mentioned this pull request Jul 20, 2017

ssh config files and/or private keys vieux/docker-volume-sshfs#24

Closed

thaJeztah added impact/api impact/changelog labels Jul 24, 2017

dnephin reviewed Jul 26, 2017

View reviewed changes

dnephin mentioned this pull request Jul 26, 2017

Remove application logic from client/ package #34268

Open

mdlinville mentioned this pull request Jul 27, 2017

Document plugins for swarm mode docker/docs#4003

Closed

vieux mentioned this pull request Aug 9, 2017

hide swarm plugins behind experimental flag #34455

Merged

yosifkit mentioned this pull request Aug 10, 2017

Add infinit. docker-library/docs#919

Closed

andrewhsu mentioned this pull request Aug 15, 2017

WIP [17.07] update vendor of components/engine in cli docker/docker-ce#195

Closed

Dimrok mentioned this pull request Aug 31, 2017

Docker Swarm Service infinit/infinit#45

Closed

thaJeztah mentioned this pull request Apr 30, 2018

Propagate managed plugins in swarm #36968

Open

thaJeztah mentioned this pull request Apr 16, 2020

ServiceCreate/ServiceUpdate: refactor and fix potential NPE #40826

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugins on swarm #33575

Plugins on swarm #33575

cpuguy83 commented Jun 7, 2017 •

edited

cpuguy83 commented Jun 8, 2017

ehazlett commented Jun 8, 2017

aaronlehmann commented Jun 8, 2017

aaronlehmann Jun 8, 2017

aaronlehmann Jun 8, 2017

cpuguy83 Jun 13, 2017

aaronlehmann Jun 13, 2017

aaronlehmann Jun 13, 2017

cpuguy83 Jun 13, 2017

aaronlehmann Jun 8, 2017

cpuguy83 Jun 11, 2017

aaronlehmann Jun 8, 2017

aaronlehmann Jun 8, 2017

aaronlehmann Jun 8, 2017

cpuguy83 Jun 9, 2017

aaronlehmann Jun 8, 2017

cpuguy83 Jun 9, 2017

aaronlehmann Jun 8, 2017

cpuguy83 Jun 11, 2017

cpuguy83 Jun 11, 2017

aaronlehmann Jun 8, 2017

cpuguy83 Jun 9, 2017

cpuguy83 commented Jul 6, 2017

yongtang commented Jul 7, 2017

rogaha commented Jul 7, 2017

cpuguy83 commented Jul 7, 2017

rogaha commented Jul 7, 2017

carlfischer1 commented Jul 7, 2017

dnephin Jul 26, 2017

dnephin Jul 26, 2017

cpuguy83 Jul 26, 2017

dnephin Jul 26, 2017

dnephin Jul 26, 2017

Plugins on swarm #33575

Plugins on swarm #33575

Conversation

cpuguy83 commented Jun 7, 2017 • edited

cpuguy83 commented Jun 8, 2017

ehazlett commented Jun 8, 2017

aaronlehmann commented Jun 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpuguy83 commented Jul 6, 2017

yongtang commented Jul 7, 2017

rogaha commented Jul 7, 2017

cpuguy83 commented Jul 7, 2017

rogaha commented Jul 7, 2017

carlfischer1 commented Jul 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpuguy83 commented Jun 7, 2017 •

edited