From cc718b34443428ea15933948abf074b20aacf599 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 18 Jul 2023 17:13:42 +0200 Subject: [PATCH 1/2] vendor: github.com/docker/cli v24.0.4 full diff: https://github.com/docker/cli/compare/v24.0.2...v24.0.4 notable changes: - ssh: fix error on commandconn close, add ping and default - commandconn: return original error while closing Signed-off-by: Sebastiaan van Stijn --- go.mod | 2 +- go.sum | 4 +- .../github.com/docker/cli/cli/command/cli.go | 12 +- .../cli/connhelper/commandconn/commandconn.go | 206 +++++++++--------- .../docker/cli/cli/connhelper/connhelper.go | 9 + vendor/modules.txt | 2 +- 6 files changed, 120 insertions(+), 115 deletions(-) diff --git a/go.mod b/go.mod index 16cd51466ad..d6c626ad0e4 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/containerd/continuity v0.4.1 github.com/containerd/typeurl/v2 v2.1.1 github.com/creack/pty v1.1.18 - github.com/docker/cli v24.0.2+incompatible + github.com/docker/cli v24.0.4+incompatible github.com/docker/cli-docs-tool v0.6.0 github.com/docker/distribution v2.8.2+incompatible github.com/docker/docker v24.0.5-0.20230714235725-36e9e796c6fc+incompatible // 24.0 diff --git a/go.sum b/go.sum index 5003265cd6f..b285da6c299 100644 --- a/go.sum +++ b/go.sum @@ -150,8 +150,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/distribution/distribution/v3 v3.0.0-20230214150026-36d8c594d7aa h1:L9Ay/slwQ4ERSPaurC+TVkZrM0K98GNrEEo1En3e8as= github.com/distribution/distribution/v3 v3.0.0-20230214150026-36d8c594d7aa/go.mod h1:WHNsWjnIn2V1LYOrME7e8KxSeKunYHsxEm4am0BUtcI= -github.com/docker/cli v24.0.2+incompatible h1:QdqR7znue1mtkXIJ+ruQMGQhpw2JzMJLRXp6zpzF6tM= -github.com/docker/cli v24.0.2+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/cli v24.0.4+incompatible h1:Y3bYF9ekNTm2VFz5U/0BlMdJy73D+Y1iAAZ8l63Ydzw= +github.com/docker/cli v24.0.4+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/cli-docs-tool v0.6.0 h1:Z9x10SaZgFaB6jHgz3OWooynhSa40CsWkpe5hEnG/qA= github.com/docker/cli-docs-tool v0.6.0/go.mod h1:zMjqTFCU361PRh8apiXzeAZ1Q/xupbIwTusYpzCXS/o= github.com/docker/distribution v2.8.2+incompatible h1:T3de5rq0dB1j30rp0sA2rER+m322EBzniBPB6ZIzuh8= diff --git a/vendor/github.com/docker/cli/cli/command/cli.go b/vendor/github.com/docker/cli/cli/command/cli.go index 4d8b9dc406b..1551c14da85 100644 --- a/vendor/github.com/docker/cli/cli/command/cli.go +++ b/vendor/github.com/docker/cli/cli/command/cli.go @@ -8,7 +8,6 @@ import ( "path/filepath" "runtime" "strconv" - "strings" "sync" "time" @@ -327,13 +326,8 @@ func (cli *DockerCli) getInitTimeout() time.Duration { func (cli *DockerCli) initializeFromClient() { ctx := context.Background() - if !strings.HasPrefix(cli.dockerEndpoint.Host, "ssh://") { - // @FIXME context.WithTimeout doesn't work with connhelper / ssh connections - // time="2020-04-10T10:16:26Z" level=warning msg="commandConn.CloseWrite: commandconn: failed to wait: signal: killed" - var cancel func() - ctx, cancel = context.WithTimeout(ctx, cli.getInitTimeout()) - defer cancel() - } + ctx, cancel := context.WithTimeout(ctx, cli.getInitTimeout()) + defer cancel() ping, err := cli.client.Ping(ctx) if err != nil { @@ -381,7 +375,7 @@ func (cli *DockerCli) ContextStore() store.Store { // the "default" context is used if: // // - The "--host" option is set -// - The "DOCKER_HOST" ([DefaultContextName]) environment variable is set +// - The "DOCKER_HOST" ([client.EnvOverrideHost]) environment variable is set // to a non-empty value. // // In these cases, the default context is used, which uses the host as diff --git a/vendor/github.com/docker/cli/cli/connhelper/commandconn/commandconn.go b/vendor/github.com/docker/cli/cli/connhelper/commandconn/commandconn.go index 202ddb84cc0..835c4c48df8 100644 --- a/vendor/github.com/docker/cli/cli/connhelper/commandconn/commandconn.go +++ b/vendor/github.com/docker/cli/cli/connhelper/commandconn/commandconn.go @@ -23,6 +23,7 @@ import ( "runtime" "strings" "sync" + "sync/atomic" "syscall" "time" @@ -64,81 +65,68 @@ func New(_ context.Context, cmd string, args ...string) (net.Conn, error) { // commandConn implements net.Conn type commandConn struct { - cmd *exec.Cmd - cmdExited bool - cmdWaitErr error - cmdMutex sync.Mutex - stdin io.WriteCloser - stdout io.ReadCloser - stderrMu sync.Mutex - stderr bytes.Buffer - stdioClosedMu sync.Mutex // for stdinClosed and stdoutClosed - stdinClosed bool - stdoutClosed bool - localAddr net.Addr - remoteAddr net.Addr + cmdMutex sync.Mutex // for cmd, cmdWaitErr + cmd *exec.Cmd + cmdWaitErr error + cmdExited atomic.Bool + stdin io.WriteCloser + stdout io.ReadCloser + stderrMu sync.Mutex // for stderr + stderr bytes.Buffer + stdinClosed atomic.Bool + stdoutClosed atomic.Bool + closing atomic.Bool + localAddr net.Addr + remoteAddr net.Addr } -// killIfStdioClosed kills the cmd if both stdin and stdout are closed. -func (c *commandConn) killIfStdioClosed() error { - c.stdioClosedMu.Lock() - stdioClosed := c.stdoutClosed && c.stdinClosed - c.stdioClosedMu.Unlock() - if !stdioClosed { - return nil +// kill terminates the process. On Windows it kills the process directly, +// whereas on other platforms, a SIGTERM is sent, before forcefully terminating +// the process after 3 seconds. +func (c *commandConn) kill() { + if c.cmdExited.Load() { + return } - return c.kill() -} - -// killAndWait tries sending SIGTERM to the process before sending SIGKILL. -func killAndWait(cmd *exec.Cmd) error { + c.cmdMutex.Lock() var werr error if runtime.GOOS != "windows" { werrCh := make(chan error) - go func() { werrCh <- cmd.Wait() }() - cmd.Process.Signal(syscall.SIGTERM) + go func() { werrCh <- c.cmd.Wait() }() + _ = c.cmd.Process.Signal(syscall.SIGTERM) select { case werr = <-werrCh: case <-time.After(3 * time.Second): - cmd.Process.Kill() + _ = c.cmd.Process.Kill() werr = <-werrCh } } else { - cmd.Process.Kill() - werr = cmd.Wait() + _ = c.cmd.Process.Kill() + werr = c.cmd.Wait() } - return werr + c.cmdWaitErr = werr + c.cmdMutex.Unlock() + c.cmdExited.Store(true) } -// kill returns nil if the command terminated, regardless to the exit status. -func (c *commandConn) kill() error { - var werr error - c.cmdMutex.Lock() - if c.cmdExited { - werr = c.cmdWaitErr - } else { - werr = killAndWait(c.cmd) - c.cmdWaitErr = werr - c.cmdExited = true - } - c.cmdMutex.Unlock() - if werr == nil { - return nil - } - wExitErr, ok := werr.(*exec.ExitError) - if ok { - if wExitErr.ProcessState.Exited() { - return nil - } +// handleEOF handles io.EOF errors while reading or writing from the underlying +// command pipes. +// +// When we've received an EOF we expect that the command will +// be terminated soon. As such, we call Wait() on the command +// and return EOF or the error depending on whether the command +// exited with an error. +// +// If Wait() does not return within 10s, an error is returned +func (c *commandConn) handleEOF(err error) error { + if err != io.EOF { + return err } - return errors.Wrapf(werr, "commandconn: failed to wait") -} -func (c *commandConn) onEOF(eof error) error { - // when we got EOF, the command is going to be terminated - var werr error c.cmdMutex.Lock() - if c.cmdExited { + defer c.cmdMutex.Unlock() + + var werr error + if c.cmdExited.Load() { werr = c.cmdWaitErr } else { werrCh := make(chan error) @@ -146,18 +134,17 @@ func (c *commandConn) onEOF(eof error) error { select { case werr = <-werrCh: c.cmdWaitErr = werr - c.cmdExited = true + c.cmdExited.Store(true) case <-time.After(10 * time.Second): - c.cmdMutex.Unlock() c.stderrMu.Lock() stderr := c.stderr.String() c.stderrMu.Unlock() - return errors.Errorf("command %v did not exit after %v: stderr=%q", c.cmd.Args, eof, stderr) + return errors.Errorf("command %v did not exit after %v: stderr=%q", c.cmd.Args, err, stderr) } } - c.cmdMutex.Unlock() + if werr == nil { - return eof + return err } c.stderrMu.Lock() stderr := c.stderr.String() @@ -166,71 +153,86 @@ func (c *commandConn) onEOF(eof error) error { } func ignorableCloseError(err error) bool { - errS := err.Error() - ss := []string{ - os.ErrClosed.Error(), + return strings.Contains(err.Error(), os.ErrClosed.Error()) +} + +func (c *commandConn) Read(p []byte) (int, error) { + n, err := c.stdout.Read(p) + // check after the call to Read, since + // it is blocking, and while waiting on it + // Close might get called + if c.closing.Load() { + // If we're currently closing the connection + // we don't want to call onEOF + return n, err } - for _, s := range ss { - if strings.Contains(errS, s) { - return true - } + + return n, c.handleEOF(err) +} + +func (c *commandConn) Write(p []byte) (int, error) { + n, err := c.stdin.Write(p) + // check after the call to Write, since + // it is blocking, and while waiting on it + // Close might get called + if c.closing.Load() { + // If we're currently closing the connection + // we don't want to call onEOF + return n, err } - return false + + return n, c.handleEOF(err) } +// CloseRead allows commandConn to implement halfCloser func (c *commandConn) CloseRead() error { // NOTE: maybe already closed here if err := c.stdout.Close(); err != nil && !ignorableCloseError(err) { - logrus.Warnf("commandConn.CloseRead: %v", err) + return err } - c.stdioClosedMu.Lock() - c.stdoutClosed = true - c.stdioClosedMu.Unlock() - if err := c.killIfStdioClosed(); err != nil { - logrus.Warnf("commandConn.CloseRead: %v", err) - } - return nil -} + c.stdoutClosed.Store(true) -func (c *commandConn) Read(p []byte) (int, error) { - n, err := c.stdout.Read(p) - if err == io.EOF { - err = c.onEOF(err) + if c.stdinClosed.Load() { + c.kill() } - return n, err + + return nil } +// CloseWrite allows commandConn to implement halfCloser func (c *commandConn) CloseWrite() error { // NOTE: maybe already closed here if err := c.stdin.Close(); err != nil && !ignorableCloseError(err) { - logrus.Warnf("commandConn.CloseWrite: %v", err) - } - c.stdioClosedMu.Lock() - c.stdinClosed = true - c.stdioClosedMu.Unlock() - if err := c.killIfStdioClosed(); err != nil { - logrus.Warnf("commandConn.CloseWrite: %v", err) + return err } - return nil -} + c.stdinClosed.Store(true) -func (c *commandConn) Write(p []byte) (int, error) { - n, err := c.stdin.Write(p) - if err == io.EOF { - err = c.onEOF(err) + if c.stdoutClosed.Load() { + c.kill() } - return n, err + return nil } +// Close is the net.Conn func that gets called +// by the transport when a dial is cancelled +// due to it's context timing out. Any blocked +// Read or Write calls will be unblocked and +// return errors. It will block until the underlying +// command has terminated. func (c *commandConn) Close() error { - var err error - if err = c.CloseRead(); err != nil { + c.closing.Store(true) + defer c.closing.Store(false) + + if err := c.CloseRead(); err != nil { logrus.Warnf("commandConn.Close: CloseRead: %v", err) + return err } - if err = c.CloseWrite(); err != nil { + if err := c.CloseWrite(); err != nil { logrus.Warnf("commandConn.Close: CloseWrite: %v", err) + return err } - return err + + return nil } func (c *commandConn) LocalAddr() net.Addr { diff --git a/vendor/github.com/docker/cli/cli/connhelper/connhelper.go b/vendor/github.com/docker/cli/cli/connhelper/connhelper.go index 397149c3e28..b98d97c25d4 100644 --- a/vendor/github.com/docker/cli/cli/connhelper/connhelper.go +++ b/vendor/github.com/docker/cli/cli/connhelper/connhelper.go @@ -5,6 +5,7 @@ import ( "context" "net" "net/url" + "strings" "github.com/docker/cli/cli/connhelper/commandconn" "github.com/docker/cli/cli/connhelper/ssh" @@ -51,6 +52,7 @@ func getConnectionHelper(daemonURL string, sshFlags []string) (*ConnectionHelper if sp.Path != "" { args = append(args, "--host", "unix://"+sp.Path) } + sshFlags = addSSHTimeout(sshFlags) args = append(args, "system", "dial-stdio") return commandconn.New(ctx, "ssh", append(sshFlags, sp.Args(args...)...)...) }, @@ -71,3 +73,10 @@ func GetCommandConnectionHelper(cmd string, flags ...string) (*ConnectionHelper, Host: "http://docker.example.com", }, nil } + +func addSSHTimeout(sshFlags []string) []string { + if !strings.Contains(strings.Join(sshFlags, ""), "ConnectTimeout") { + sshFlags = append(sshFlags, "-o ConnectTimeout=30") + } + return sshFlags +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 4b6853a93d5..14136807d32 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -209,7 +209,7 @@ github.com/davecgh/go-spew/spew # github.com/distribution/distribution/v3 v3.0.0-20230214150026-36d8c594d7aa ## explicit; go 1.18 github.com/distribution/distribution/v3/reference -# github.com/docker/cli v24.0.2+incompatible +# github.com/docker/cli v24.0.4+incompatible ## explicit github.com/docker/cli/cli github.com/docker/cli/cli-plugins/manager From 094d1aded87d583d5d0ceac7b82955f1b445b8d1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 19 Jul 2023 11:04:45 +0200 Subject: [PATCH 2/2] commands: NewRootCmd: remove obsolete logrus filter hook This hook was added in 278f94a8b62052ed790bb1c6832268eb79a6d03d and 72758fef22441c37f809cce098f73a21e618b9d1 to suppress spurious warnings printed by the CLI's cli/connhelper/commandconn package; https://github.com/docker/cli/blob/3fb4fb83dfb5db0c0753a8316f21aea54dab32c5/cli/connhelper/commandconn/commandconn.go#L203-L214 Those logs were removed in https://github.com/docker/cli/commit/a5ebe2282aabaf983c116525af4a7c5eeedf2c6e so we can remove the hook. Signed-off-by: Sebastiaan van Stijn --- commands/root.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/commands/root.go b/commands/root.go index 4dd5fc8a1f5..26ed68ce6f7 100644 --- a/commands/root.go +++ b/commands/root.go @@ -52,17 +52,6 @@ func NewRootCmd(name string, isPlugin bool, dockerCli command.Cli) *cobra.Comman "using default config store", )) - // filter out useless commandConn.CloseWrite warning message that can occur - // when listing builder instances with "buildx ls" for those that are - // unreachable: "commandConn.CloseWrite: commandconn: failed to wait: signal: killed" - // https://github.com/docker/cli/blob/3fb4fb83dfb5db0c0753a8316f21aea54dab32c5/cli/connhelper/commandconn/commandconn.go#L203-L214 - logrus.AddHook(logutil.NewFilter([]logrus.Level{ - logrus.WarnLevel, - }, - "commandConn.CloseWrite:", - "commandConn.CloseRead:", - )) - addCommands(cmd, dockerCli) return cmd }