From d9fd705bf8065939b7bf0162a7ef86d38aa731f5 Mon Sep 17 00:00:00 2001 From: Nishant Totla Date: Fri, 9 Sep 2016 12:14:35 +0100 Subject: [PATCH 1/4] Re-enable plugin filter Signed-off-by: Nishant Totla --- agent/exec/container/executor.go | 25 +++++++++++++++++++++++++ manager/scheduler/filter.go | 1 + manager/scheduler/pipeline.go | 7 +------ manager/scheduler/scheduler_test.go | 1 - 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/agent/exec/container/executor.go b/agent/exec/container/executor.go index a2f9e2968a..fae22a25d9 100644 --- a/agent/exec/container/executor.go +++ b/agent/exec/container/executor.go @@ -41,12 +41,37 @@ func (e *executor) Describe(ctx context.Context) (*api.NodeDescription, error) { } } + // add v1 plugins to 'plugins' addPlugins("Volume", info.Plugins.Volume) // Add builtin driver "overlay" (the only builtin multi-host driver) to // the plugin list by default. addPlugins("Network", append([]string{"overlay"}, info.Plugins.Network...)) addPlugins("Authorization", info.Plugins.Authorization) + // retrieve v2 plugins + v2plugins, err := e.client.PluginList(ctx) + if err != nil { + return nil, err + } + + // add v2 plugins to 'plugins' + for _, plgn := range v2plugins { + for _, typ := range plgn.Config.Interface.Types { + if typ.Prefix == "docker" && plgn.Enabled { + plgnTyp := typ.Capability + if typ.Capability == "volumedriver" { + plgnTyp = "Volume" + } else if typ.Capability == "networkdriver" { + plgnTyp = "Network" + } + plugins[api.PluginDescription{ + Type: plgnTyp, + Name: plgn.Name, + }] = struct{}{} + } + } + } + pluginFields := make([]api.PluginDescription, 0, len(plugins)) for k := range plugins { pluginFields = append(pluginFields, k) diff --git a/manager/scheduler/filter.go b/manager/scheduler/filter.go index 1ecbb7d08a..79415886f3 100644 --- a/manager/scheduler/filter.go +++ b/manager/scheduler/filter.go @@ -145,6 +145,7 @@ func (f *PluginFilter) Check(n *NodeInfo) bool { return true } +// pluginExistsOnNode returns true if the (pluginName, pluginType) pair is present in nodePlugins func (f *PluginFilter) pluginExistsOnNode(pluginType string, pluginName string, nodePlugins []api.PluginDescription) bool { for _, np := range nodePlugins { if pluginType == np.Type && pluginName == np.Name { diff --git a/manager/scheduler/pipeline.go b/manager/scheduler/pipeline.go index d9981aa125..00fd36c5c7 100644 --- a/manager/scheduler/pipeline.go +++ b/manager/scheduler/pipeline.go @@ -11,12 +11,7 @@ var ( // Always check for readiness first. &ReadyFilter{}, &ResourceFilter{}, - - // TODO(stevvooe): Do not filter based on plugins since they are lazy - // loaded in the engine. We can add this back when we can schedule - // plugins in the future. - // &PluginFilter{}, - + &PluginFilter{}, &ConstraintFilter{}, } ) diff --git a/manager/scheduler/scheduler_test.go b/manager/scheduler/scheduler_test.go index 04bc4f9b25..d67cb6651b 100644 --- a/manager/scheduler/scheduler_test.go +++ b/manager/scheduler/scheduler_test.go @@ -1387,7 +1387,6 @@ func watchAssignment(t *testing.T, watch chan events.Event) *api.Task { } func TestSchedulerPluginConstraint(t *testing.T) { - t.Skip("plugin filtering disabled since plugins on the engine are lazy loaded") ctx := context.Background() // Node1: vol plugin1 From 6cbd0db948a630e6845ba76c45a6c091b8377955 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 13 Dec 2016 14:17:25 -0800 Subject: [PATCH 2/4] scheduler: Fixes to plugin filter Be consistent between SetTask and Check on which mounts are checked against the plugin list. Be more defensive against nil pointer dereferences. Signed-off-by: Aaron Lehmann --- manager/scheduler/filter.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/manager/scheduler/filter.go b/manager/scheduler/filter.go index 79415886f3..79e0d079b4 100644 --- a/manager/scheduler/filter.go +++ b/manager/scheduler/filter.go @@ -93,6 +93,15 @@ type PluginFilter struct { t *api.Task } +func referencesVolumePlugin(mount api.Mount) bool { + return mount.Type == api.MountTypeVolume && + mount.VolumeOptions != nil && + mount.VolumeOptions.DriverConfig != nil && + mount.VolumeOptions.DriverConfig.Name != "" && + mount.VolumeOptions.DriverConfig.Name != "local" + +} + // SetTask returns true when the filter is enabled for a given task. func (f *PluginFilter) SetTask(t *api.Task) bool { c := t.Spec.GetContainer() @@ -100,12 +109,9 @@ func (f *PluginFilter) SetTask(t *api.Task) bool { var volumeTemplates bool if c != nil { for _, mount := range c.Mounts { - if mount.Type == api.MountTypeVolume && - mount.VolumeOptions != nil && - mount.VolumeOptions.DriverConfig != nil && - mount.VolumeOptions.DriverConfig.Name != "" && - mount.VolumeOptions.DriverConfig.Name != "local" { + if referencesVolumePlugin(mount) { volumeTemplates = true + break } } } @@ -128,7 +134,7 @@ func (f *PluginFilter) Check(n *NodeInfo) bool { container := f.t.Spec.GetContainer() if container != nil { for _, mount := range container.Mounts { - if mount.VolumeOptions != nil && mount.VolumeOptions.DriverConfig != nil { + if referencesVolumePlugin(mount) { if !f.pluginExistsOnNode("Volume", mount.VolumeOptions.DriverConfig.Name, nodePlugins) { return false } @@ -138,8 +144,10 @@ func (f *PluginFilter) Check(n *NodeInfo) bool { // Check if all network plugins required by task are installed on node for _, tn := range f.t.Networks { - if !f.pluginExistsOnNode("Network", tn.Network.DriverState.Name, nodePlugins) { - return false + if tn.Network != nil && tn.Network.DriverState != nil && tn.Network.DriverState.Name != "" { + if !f.pluginExistsOnNode("Network", tn.Network.DriverState.Name, nodePlugins) { + return false + } } } return true From 015208ac38e94b24254ac0a2eec0f1ce5176dead Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 13 Dec 2016 14:21:07 -0800 Subject: [PATCH 3/4] agent/exec/container: Tolerate errors from PluginList This endpoint may not always be implemented or supported. Signed-off-by: Aaron Lehmann --- agent/exec/container/executor.go | 33 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/agent/exec/container/executor.go b/agent/exec/container/executor.go index fae22a25d9..433e3001a0 100644 --- a/agent/exec/container/executor.go +++ b/agent/exec/container/executor.go @@ -8,6 +8,7 @@ import ( "github.com/docker/swarmkit/agent/exec" "github.com/docker/swarmkit/agent/secrets" "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/log" "golang.org/x/net/context" ) @@ -51,23 +52,23 @@ func (e *executor) Describe(ctx context.Context) (*api.NodeDescription, error) { // retrieve v2 plugins v2plugins, err := e.client.PluginList(ctx) if err != nil { - return nil, err - } - - // add v2 plugins to 'plugins' - for _, plgn := range v2plugins { - for _, typ := range plgn.Config.Interface.Types { - if typ.Prefix == "docker" && plgn.Enabled { - plgnTyp := typ.Capability - if typ.Capability == "volumedriver" { - plgnTyp = "Volume" - } else if typ.Capability == "networkdriver" { - plgnTyp = "Network" + log.L.WithError(err).Warning("PluginList operation failed") + } else { + // add v2 plugins to 'plugins' + for _, plgn := range v2plugins { + for _, typ := range plgn.Config.Interface.Types { + if typ.Prefix == "docker" && plgn.Enabled { + plgnTyp := typ.Capability + if typ.Capability == "volumedriver" { + plgnTyp = "Volume" + } else if typ.Capability == "networkdriver" { + plgnTyp = "Network" + } + plugins[api.PluginDescription{ + Type: plgnTyp, + Name: plgn.Name, + }] = struct{}{} } - plugins[api.PluginDescription{ - Type: plgnTyp, - Name: plgn.Name, - }] = struct{}{} } } } From 56bcb2fcb5a698201efb0491dbb6ad1671d5f6bf Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 13 Dec 2016 16:51:04 -0800 Subject: [PATCH 4/4] Handle tags in plugin names Signed-off-by: Aaron Lehmann --- agent/exec/container/executor.go | 6 +++++- manager/scheduler/filter.go | 14 +++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/agent/exec/container/executor.go b/agent/exec/container/executor.go index 433e3001a0..a043b1dd33 100644 --- a/agent/exec/container/executor.go +++ b/agent/exec/container/executor.go @@ -64,9 +64,13 @@ func (e *executor) Describe(ctx context.Context) (*api.NodeDescription, error) { } else if typ.Capability == "networkdriver" { plgnTyp = "Network" } + plgnName := plgn.Name + if plgn.Tag != "" { + plgnName += ":" + plgn.Tag + } plugins[api.PluginDescription{ Type: plgnTyp, - Name: plgn.Name, + Name: plgnName, }] = struct{}{} } } diff --git a/manager/scheduler/filter.go b/manager/scheduler/filter.go index 79e0d079b4..b2b64578b4 100644 --- a/manager/scheduler/filter.go +++ b/manager/scheduler/filter.go @@ -2,6 +2,7 @@ package scheduler import ( "fmt" + "strings" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/manager/constraint" @@ -156,7 +157,18 @@ func (f *PluginFilter) Check(n *NodeInfo) bool { // pluginExistsOnNode returns true if the (pluginName, pluginType) pair is present in nodePlugins func (f *PluginFilter) pluginExistsOnNode(pluginType string, pluginName string, nodePlugins []api.PluginDescription) bool { for _, np := range nodePlugins { - if pluginType == np.Type && pluginName == np.Name { + if pluginType != np.Type { + continue + } + if pluginName == np.Name { + return true + } + // This does not use the reference package to avoid the + // overhead of parsing references as part of the scheduling + // loop. This is okay only because plugin names are a very + // strict subset of the reference grammar that is always + // name:tag. + if strings.HasPrefix(np.Name, pluginName) && np.Name[len(pluginName):] == ":latest" { return true } }