Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

make runAttach public and allow passing context #4637

Merged
merged 2 commits into from
Nov 8, 2023
Merged

make runAttach public and allow passing context #4637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4abe42
make runAttach public and allow passing context
ndeloof Nov 6, 2023
a2ec50a
make `container` an explicit, required parameter
ndeloof Nov 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 25 additions & 25 deletions cli/command/container/attach.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ import (
"github.com/spf13/cobra"
)

type attachOptions struct {
noStdin bool
proxy bool
detachKeys string

container string
// AttachOptions group options for `attach` command
type AttachOptions struct {
NoStdin bool
Proxy bool
DetachKeys string
}

func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, args string) (*types.ContainerJSON, error) {
Expand All @@ -45,15 +44,16 @@ func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, ar

// NewAttachCommand creates a new cobra.Command for `docker attach`
func NewAttachCommand(dockerCli command.Cli) *cobra.Command {
var opts attachOptions
var opts AttachOptions
var container string

cmd := &cobra.Command{
Use: "attach [OPTIONS] CONTAINER",
Short: "Attach local standard input, output, and error streams to a running container",
Args: cli.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.container = args[0]
return runAttach(dockerCli, &opts)
container = args[0]
return RunAttach(context.Background(), dockerCli, container, &opts)
},
Annotations: map[string]string{
"aliases": "docker container attach, docker attach",
Expand All @@ -64,36 +64,36 @@ func NewAttachCommand(dockerCli command.Cli) *cobra.Command {
}

flags := cmd.Flags()
flags.BoolVar(&opts.noStdin, "no-stdin", false, "Do not attach STDIN")
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.BoolVar(&opts.NoStdin, "no-stdin", false, "Do not attach STDIN")
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")
return cmd
}

func runAttach(dockerCli command.Cli, opts *attachOptions) error {
ctx := context.Background()
// RunAttach executes an `attach` command
func RunAttach(ctx context.Context, dockerCli command.Cli, target string, opts *AttachOptions) error {
apiClient := dockerCli.Client()

// request channel to wait for client
resultC, errC := apiClient.ContainerWait(ctx, opts.container, "")
resultC, errC := apiClient.ContainerWait(ctx, target, "")

c, err := inspectContainerAndCheckState(ctx, apiClient, opts.container)
c, err := inspectContainerAndCheckState(ctx, apiClient, target)
if err != nil {
return err
}

if err := dockerCli.In().CheckTty(!opts.noStdin, c.Config.Tty); err != nil {
if err := dockerCli.In().CheckTty(!opts.NoStdin, c.Config.Tty); err != nil {
return err
}

detachKeys := dockerCli.ConfigFile().DetachKeys
if opts.detachKeys != "" {
detachKeys = opts.detachKeys
if opts.DetachKeys != "" {
detachKeys = opts.DetachKeys
}

options := container.AttachOptions{
Stream: true,
Stdin: !opts.noStdin && c.Config.OpenStdin,
Stdin: !opts.NoStdin && c.Config.OpenStdin,
Stdout: true,
Stderr: true,
DetachKeys: detachKeys,
Expand All @@ -104,13 +104,13 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
in = dockerCli.In()
}

if opts.proxy && !c.Config.Tty {
if opts.Proxy && !c.Config.Tty {
sigc := notifyAllSignals()
go ForwardAllSignals(ctx, dockerCli, opts.container, sigc)
go ForwardAllSignals(ctx, dockerCli, target, sigc)
defer signal.StopCatch(sigc)
}

resp, errAttach := apiClient.ContainerAttach(ctx, opts.container, options)
resp, errAttach := apiClient.ContainerAttach(ctx, target, options)
if errAttach != nil {
return errAttach
}
Expand All @@ -124,13 +124,13 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
// the container and not exit.
//
// Recheck the container's state to avoid attach block.
_, err = inspectContainerAndCheckState(ctx, apiClient, opts.container)
_, err = inspectContainerAndCheckState(ctx, apiClient, target)
if err != nil {
return err
}

if c.Config.Tty && dockerCli.Out().IsTerminal() {
resizeTTY(ctx, dockerCli, opts.container)
resizeTTY(ctx, dockerCli, target)
}

streamer := hijackedIOStreamer{
Expand Down
13 changes: 6 additions & 7 deletions cli/command/container/exec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ type ExecOptions struct {
Privileged bool
Env opts.ListOpts
Workdir string
Container string
Command []string
EnvFile opts.ListOpts
}
Expand All @@ -44,15 +43,16 @@ func NewExecOptions() ExecOptions {
// NewExecCommand creates a new cobra.Command for `docker exec`
func NewExecCommand(dockerCli command.Cli) *cobra.Command {
options := NewExecOptions()
var container string

cmd := &cobra.Command{
Use: "exec [OPTIONS] CONTAINER COMMAND [ARG...]",
Short: "Execute a command in a running container",
Args: cli.RequiresMinArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
options.Container = args[0]
container = args[0]
options.Command = args[1:]
return RunExec(dockerCli, options)
return RunExec(context.Background(), dockerCli, container, options)
},
ValidArgsFunction: completion.ContainerNames(dockerCli, false, func(container types.Container) bool {
return container.State != "paused"
Expand Down Expand Up @@ -96,20 +96,19 @@ func NewExecCommand(dockerCli command.Cli) *cobra.Command {
}

// RunExec executes an `exec` command
func RunExec(dockerCli command.Cli, options ExecOptions) error {
func RunExec(ctx context.Context, dockerCli command.Cli, container string, options ExecOptions) error {
execConfig, err := parseExec(options, dockerCli.ConfigFile())
if err != nil {
return err
}

ctx := context.Background()
client := dockerCli.Client()

// We need to check the tty _before_ we do the ContainerExecCreate, because
// otherwise if we error out we will leak execIDs on the server (and
// there's no easy way to clean those up). But also in order to make "not
// exist" errors take precedence we do a dummy inspect first.
if _, err := client.ContainerInspect(ctx, options.Container); err != nil {
if _, err := client.ContainerInspect(ctx, container); err != nil {
return err
}
if !execConfig.Detach {
Expand All @@ -120,7 +119,7 @@ func RunExec(dockerCli command.Cli, options ExecOptions) error {

fillConsoleSize(execConfig, dockerCli)

response, err := client.ContainerExecCreate(ctx, options.Container, *execConfig)
response, err := client.ContainerExecCreate(ctx, container, *execConfig)
if err != nil {
return err
}
Expand Down
5 changes: 2 additions & 3 deletions cli/command/container/exec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ func TestRunExec(t *testing.T) {
{
doc: "successful detach",
options: withDefaultOpts(ExecOptions{
Container: "thecontainer",
Detach: true,
Detach: true,
}),
client: fakeClient{execCreateFunc: execCreateWithID},
},
Expand All @@ -195,7 +194,7 @@ func TestRunExec(t *testing.T) {
t.Run(testcase.doc, func(t *testing.T) {
cli := test.NewFakeCli(&testcase.client)

err := RunExec(cli, testcase.options)
err := RunExec(context.Background(), cli, "thecontainer", testcase.options)
if testcase.expectedError != "" {
assert.ErrorContains(t, err, testcase.expectedError)
} else {
Expand Down
Loading