[Carry #1517] Support v2 plugins; re-enable scheduler plugin filter #1808
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
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 <aaron.lehmann@docker.com>
This endpoint may not always be implemented or supported. Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Current coverage is 55.36% (diff: 44.44%)@@ master #1808 diff @@
==========================================
Files 102 102
Lines 16941 16957 +16
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 9338 9388 +50
+ Misses 6444 6410 -34
Partials 1159 1159
|
aluzzardi
approved these changes
Dec 14, 2016
|
SGTM, workflow looks good |
nishanttotla
approved these changes
Dec 14, 2016
aaronlehmann
added
process/cherry-pick
process/cherry-picked
and removed
process/cherry-pick
labels
stevvooe
reviewed
Jan 10, 2017
| for _, plgn := range v2plugins { | ||
| for _, typ := range plgn.Config.Interface.Types { | ||
| if typ.Prefix == "docker" && plgn.Enabled { | ||
| plgnTyp := typ.Capability |
There was a problem hiding this comment.
Is it a good idea to be coercing capability to type?
What if there are multi-capability plugins?
stevvooe
reviewed
Jan 10, 2017
| } | ||
| plgnName := plgn.Name | ||
| if plgn.Tag != "" { | ||
| plgnName += ":" + plgn.Tag |
There was a problem hiding this comment.
For all of our effort, how did Tag end up being separated in plugins? Can we please fix this?
There was a problem hiding this comment.
There was a problem hiding this comment.
Odd, when I went through search, field was still there.
stevvooe
reviewed
Jan 10, 2017
| 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 { |
There was a problem hiding this comment.
Is this comparison case sensitive?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This carries #1517:
On top of this, it adds:
DescribeifPluginListis unimplementedcc @nishanttotla @aluzzardi @vieux @anusha-ragunathan @tonistiigi