[WIP] cli-plugins: look for plugins in experimental folder when experimental is enabled #1902
Conversation
tiborvass
force-pushed
the
plugin_experimental2
branch
from
849209a to
ca44e4b
Compare
Codecov Report
@@ Coverage Diff @@
## master #1902 +/- ##
==========================================
- Coverage 56.75% 56.73% -0.03%
==========================================
Files 309 309
Lines 21680 21701 +21
==========================================
+ Hits 12305 12311 +6
- Misses 8476 8489 +13
- Partials 899 901 +2 |
cli-plugins/manager/manager.go
Outdated
| dirs := make([]string, len(pluginDirs), len(pluginDirs)*2) | ||
| copy(dirs, pluginDirs) | ||
| for _, dir := range dirs { | ||
| pluginDirs = append(pluginDirs, dir+"-experimental") |
There was a problem hiding this comment.
This puts all experimental paths after all non-experimental paths, rather than interleving them, is that as intended?
IOW this is:
~/.docker/cli-plugins
/usr/local/lib/docker/cli-plugins
/usr/lib/docker/cli-plugins
~/.docker/cli-plugins-experimental
/usr/local/lib/docker/cli-plugins-experimental
/usr/lib/docker/cli-plugins-experimental
and not
~/.docker/cli-plugins
~/.docker/cli-plugins-experimental
/usr/local/lib/docker/cli-plugins
/usr/local/lib/docker/cli-plugins-experimental
/usr/lib/docker/cli-plugins
/usr/lib/docker/cli-plugins-experimental
I think it's deliberate but just wanted to check that the implications had been considered i.e. As a user I cannot now install an experimental update to something shipped in /usr/lib/docker/cli-plugins in my home dir.
There was a problem hiding this comment.
Was thinking about this; if someone installs both an "experimental" and "stable" version of a plugin; which one should take precedence?
There was a problem hiding this comment.
it should be precisely the first enabled plugin found in the search path, which is why the above distinction on the ordering might matter.
With the current layout I think there is no way to install an experimental version "over" an non-experimental one, other than by pretending it is non-experimental
There was a problem hiding this comment.
We could do either, I thought this was not only easier to implement, but also made more sense. But happy to change it.
| } | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| fmt.Println("toto", tc.c.Experimental()) |
There was a problem hiding this comment.
Code looks ok, I left some minor comments/nits.
I think the stuff about e2e testing from #1902 (comment) applies to this variant applies here too but you were going to do that in a follow up so that's fine.
thaJeztah
changed the title
|
I renamed this "WIP" for now; as there might be things to discuss w.r.t. Docker Desktop integration and some other concerns; lets do so on #1897 |
…l is enabled This patch expands the cli plugin search paths by adding `-experimental` to each, only if the CLI has experimental mode enabled. To test that experimental plugins do not work by default: ``` $ ln -s plugins-linux-amd64 build/plugins-linux-amd64-experimental $ vim ~/.config/docker.json # add $(pwd)/build/plugins-linux-amd64 to "cliPluginsExtraDirs" $ make plugins $ make binary $ docker helloworld ``` To show it enabled: ``` DOCKER_CLI_EXPERIMENTAL=enabled docker helloworld ``` Signed-off-by: Tibor Vass <tibor@docker.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass
force-pushed
the
plugin_experimental2
branch
from
ca44e4b to
e39aa2d
Compare
|
@ijc Updated with fixes. |
|
I can't retrigger validate, but validate passed locally for me. |
|
restarted CI (you can restart after connecting GitHub in the user-settings in CircleCI) |
|
closing as #1898 was merged |
|
I'm reopening because I thought this was still desired. |
Yeah, I was about to say I don't think the two approaches need to be mutually exclusive... |
This is an alternative to #1898
Closes #1897 #1898
Related to docker/docker-ce-packaging#332
This patch expands the cli plugin search paths by adding
-experimentalto each,only if the CLI has experimental mode enabled.
To test that experimental plugins do not work by default:
To show it enabled:
Signed-off-by: Tibor Vass tibor@docker.com