From ff2ed6efa82ca459a5971e1a8b583df7b892bcdb Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Mon, 18 Mar 2019 09:33:25 +0000 Subject: [PATCH] cli-plugins: disable use of dial-stdio Since #1654 so far we've had problems with it not working on Windows (npipe lacked the `CloseRead` method) and problems with using tcp with tls (the tls connection also lacks `CloseRead`). Both of these were workedaround in #1718 which added a nop `CloseRead` method. However I am now seeing hangs (on Windows) where the `system dial-stdio` subprocess is not exiting (I'm unsure why so far). I think the 3rd problem found with this is an indication that `dial-stdio` is not quite ready for wider use outside of its initial usecase (support for `ssh://` URLs to connect to remote daemons). This change simply disables the `dial-stdio` path for all plugins. However rather than completely reverting 891b3d953e43 ("cli-plugins: use `docker system dial-stdio` to call the daemon") I've just disabled the functionality at the point of use and left in a trap door environment variable so that those who want to experiment with this mode (and perhaps fully debug it) have an easier path do doing so. The e2e test for this case is disabled unless the trap door envvar is set. I also renamed the test to clarify that it is about cli plugins. Signed-off-by: Ian Campbell --- cli-plugins/plugin/plugin.go | 6 +++++- e2e/cli-plugins/dial_test.go | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/cli-plugins/plugin/plugin.go b/cli-plugins/plugin/plugin.go index 934baed884..dfcd466ebd 100644 --- a/cli-plugins/plugin/plugin.go +++ b/cli-plugins/plugin/plugin.go @@ -31,7 +31,11 @@ func runPlugin(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager PersistentPreRunE = func(_ *cobra.Command, _ []string) error { var err error persistentPreRunOnce.Do(func() { - err = tcmd.Initialize(withPluginClientConn(plugin.Name())) + var opts []command.InitializeOpt + if os.Getenv("DOCKER_CLI_PLUGIN_USE_DIAL_STDIO") != "" { + opts = append(opts, withPluginClientConn(plugin.Name())) + } + err = tcmd.Initialize(opts...) }) return err } diff --git a/e2e/cli-plugins/dial_test.go b/e2e/cli-plugins/dial_test.go index a01feb0345..f483b4e578 100644 --- a/e2e/cli-plugins/dial_test.go +++ b/e2e/cli-plugins/dial_test.go @@ -11,7 +11,11 @@ import ( "gotest.tools/icmd" ) -func TestDialStdio(t *testing.T) { +func TestCLIPluginDialStdio(t *testing.T) { + if os.Getenv("DOCKER_CLI_PLUGIN_USE_DIAL_STDIO") == "" { + t.Skip("skipping plugin dial-stdio test since DOCKER_CLI_PLUGIN_USE_DIAL_STDIO is not set") + } + // Run the helloworld plugin forcing /bin/true as the `system // dial-stdio` target. It should be passed all arguments from // before the `helloworld` arg, but not the --who=foo which