Skip to content

Commit

Permalink
cli-plugins: disable use of dial-stdio
Browse files Browse the repository at this point in the history
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 891b3d9 ("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 <ijc@docker.com>
  • Loading branch information
Ian Campbell committed Mar 18, 2019
1 parent 651ccc0 commit ff2ed6e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
6 changes: 5 additions & 1 deletion cli-plugins/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion e2e/cli-plugins/dial_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ff2ed6e

Please sign in to comment.