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.

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

Refactor command/container/run.go #5693

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Next Next commit
run/cleanup: use specific/clearer name for context
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
laurazard committed Dec 13, 2024
commit 2b68a7c950b14ba6a67875a23725ffe475d41ebe
16 changes: 8 additions & 8 deletions cli/command/container/run.go
Original file line number Diff line number Diff line change
@@ -154,8 +154,8 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
defer signal.StopCatch(sigc)
}

ctx, cancelFun := context.WithCancel(context.WithoutCancel(ctx))
defer cancelFun()
attachStartCtx, attachStartCancel := context.WithCancel(context.WithoutCancel(ctx))
defer attachStartCancel()

var (
waitDisplayID chan struct{}
@@ -179,7 +179,7 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
// ctx should not be cancellable here, as this would kill the stream to the container
// and we want to keep the stream open until the process in the container exits or until
// the user forcefully terminates the CLI.
closeFn, err := attachContainer(ctx, dockerCli, containerID, &errCh, config, container.AttachOptions{
closeFn, err := attachContainer(attachStartCtx, dockerCli, containerID, &errCh, config, container.AttachOptions{
Stream: true,
Stdin: config.AttachStdin,
Stdout: config.AttachStdout,
@@ -194,17 +194,17 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption

// New context here because we don't to cancel waiting on container exit/remove
// when we cancel attach, etc.
statusCtx, cancelStatusCtx := context.WithCancel(context.WithoutCancel(ctx))
statusCtx, cancelStatusCtx := context.WithCancel(context.WithoutCancel(attachStartCtx))
defer cancelStatusCtx()
statusChan := waitExitOrRemoved(statusCtx, apiClient, containerID, copts.autoRemove)

// start the container
if err := apiClient.ContainerStart(ctx, containerID, container.StartOptions{}); err != nil {
if err := apiClient.ContainerStart(attachStartCtx, containerID, container.StartOptions{}); err != nil {
// If we have hijackedIOStreamer, we should notify
// hijackedIOStreamer we are going to exit and wait
// to avoid the terminal are not restored.
if attach {
cancelFun()
attachStartCancel()
<-errCh
}

@@ -223,7 +223,7 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
}

if config.Tty && dockerCli.Out().IsTerminal() {
if err := MonitorTtySize(ctx, dockerCli, containerID, false); err != nil {
if err := MonitorTtySize(attachStartCtx, dockerCli, containerID, false); err != nil {
_, _ = fmt.Fprintln(stderr, "Error monitoring TTY size:", err)
}
}
@@ -246,7 +246,7 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
case status := <-statusChan:
// notify hijackedIOStreamer that we're exiting and wait
// so that the terminal can be restored.
cancelFun()
attachStartCancel()
<-errCh
if status != 0 {
return cli.StatusError{StatusCode: status}