Guest24897
/
DockerCLI
mirror of https://github.com/docker/cli.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

WIP: refactor RunAttach to receive container.AttachOptions 

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2023-11-07 12:47:12 +01:00
parent 06b8da120e
commit 3498ec6550
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C
1 changed files with 32 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
cli/command/container/attach.go
View File

@ -51,8 +51,21 @@ func NewAttachCommand(dockerCLI command.Cli) *cobra.Command {
Short: "Attach local standard input, output, and error streams to a running container", Short: "Attach local standard input, output, and error streams to a running container",
Args: cli.ExactArgs(1), Args: cli.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
attachStdIn := true
if opts.NoStdin {
// TODO(thaJeztah): this is the tricky one: can we use container.AttachOptions for this one without it being ambiguous?
attachStdIn = false
}
containerID := args[0] containerID := args[0]
return RunAttach(cmd.Context(), dockerCLI, containerID, &opts) disableSignalProxy := !opts.Proxy
return RunAttach(cmd.Context(), dockerCLI, containerID, disableSignalProxy, container.AttachOptions{
Stream: true,
Stdin: attachStdIn,
Stdout: true,
Stderr: true,
DetachKeys: opts.DetachKeys,
})
}, },
Annotations: map[string]string{ Annotations: map[string]string{
"aliases": "docker container attach, docker attach", "aliases": "docker container attach, docker attach",
@ -64,61 +77,54 @@ func NewAttachCommand(dockerCLI command.Cli) *cobra.Command {
flags := cmd.Flags() flags := cmd.Flags()
flags.BoolVar(&opts.NoStdin, "no-stdin", false, "Do not attach STDIN") flags.BoolVar(&opts.NoStdin, "no-stdin", false, "Do not attach STDIN")
// Is this feature still used?
// It was added in https://github.com/moby/moby/commit/4918769b1ac2d38f23087b766140e6a7f8979310 to allow forwarding signals to containers
// Changed in https://github.com/moby/moby/commit/333bc23f21e8423d3085632db99a6d1df227c5f1
// And changed to be enabled by default in https://github.com/moby/moby/commit/e0b59ab52b87b8fc15dd5534c3231fdd74843f9f (unless a TTY is attached)
// related: https://github.com/moby/moby/commit/e0b59ab52b87b8fc15dd5534c3231fdd74843f9f#commitcomment-25897874
// related: https://github.com/moby/moby/issues/9098
// related: https://github.com/docker/cli/pull/1841
flags.BoolVar(&opts.Proxy, "sig-proxy", true, "Proxy all received signals to the process") flags.BoolVar(&opts.Proxy, "sig-proxy", true, "Proxy all received signals to the process")
flags.StringVar(&opts.DetachKeys, "detach-keys", "", "Override the key sequence for detaching a container") flags.StringVar(&opts.DetachKeys, "detach-keys", "", "Override the key sequence for detaching a container")
return cmd return cmd
} }
// RunAttach executes an `attach` command // RunAttach attaches to the given container.
func RunAttach(ctx context.Context, dockerCLI command.Cli, containerID string, opts *AttachOptions) error { func RunAttach(ctx context.Context, dockerCLI command.Cli, containerID string, disableSignalProxy bool, opts container.AttachOptions) error {
apiClient := dockerCLI.Client() apiClient := dockerCLI.Client()
attachStdIn := true
if opts.NoStdin {
// TODO(thaJeztah): this is the tricky one: can we use container.AttachOptions for this one without it being ambiguous?
attachStdIn = false
}
c, err := inspectContainerAndCheckState(ctx, apiClient, containerID) c, err := inspectContainerAndCheckState(ctx, apiClient, containerID)
if err != nil { if err != nil {
return err return err
} }
if attachStdIn { if opts.Stdin {
if err := dockerCLI.In().CheckTty(attachStdIn, c.Config.Tty); err != nil { if err := dockerCLI.In().CheckTty(opts.Stdin, c.Config.Tty); err != nil {
return err return err
} }
if !c.Config.OpenStdin { if !c.Config.OpenStdin {
// TODO(thaJeztah): should this produce an error? // TODO(thaJeztah): should this produce an error?
attachStdIn = false opts.Stdin = false
} }
} }
detachKeys := opts.DetachKeys
if opts.DetachKeys == "" { if opts.DetachKeys == "" {
detachKeys = dockerCLI.ConfigFile().DetachKeys opts.DetachKeys = dockerCLI.ConfigFile().DetachKeys
}
options := container.AttachOptions{
Stream: true,
Stdin: attachStdIn,
Stdout: true,
Stderr: true,
DetachKeys: detachKeys,
} }
var in io.ReadCloser var in io.ReadCloser
if options.Stdin { if opts.Stdin {
in = dockerCLI.In() in = dockerCLI.In()
} }
if opts.Proxy && !c.Config.Tty { // TODO(thaJeztah): should this still depend on Config.Tty? It's unconditionally enabled on `docker exec` since https://github.com/docker/cli/pull/1841/files
if !disableSignalProxy && !c.Config.Tty {
sigc := notifyAllSignals() sigc := notifyAllSignals()
go ForwardAllSignals(ctx, apiClient, containerID, sigc) go ForwardAllSignals(ctx, apiClient, containerID, sigc)
defer signal.StopCatch(sigc) defer signal.StopCatch(sigc)
} }
resp, errAttach := apiClient.ContainerAttach(ctx, containerID, options) resp, errAttach := apiClient.ContainerAttach(ctx, containerID, opts)
if errAttach != nil { if errAttach != nil {
return errAttach return errAttach
} }
@ -148,7 +154,7 @@ func RunAttach(ctx context.Context, dockerCLI command.Cli, containerID string, o
errorStream: dockerCLI.Err(), errorStream: dockerCLI.Err(),
resp: resp, resp: resp,
tty: c.Config.Tty, tty: c.Config.Tty,
detachKeys: options.DetachKeys, detachKeys: opts.DetachKeys,
} }
if err := streamer.stream(ctx); err != nil { if err := streamer.stream(ctx); err != nil {