Skip to content

Commit

Permalink
cli/command/container: ForwardAllSignals: rewrite to use ContainerAPI…
Browse files Browse the repository at this point in the history
…Client

This function only needed the ContainerAPIClient, and not the whole CLI. This
patch refactors it to use the shallower interface.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
  • Loading branch information
thaJeztah committed Nov 6, 2023
1 parent 9cb175f commit 7497075
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 17 deletions.
2 changes: 1 addition & 1 deletion cli/command/container/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {

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

Expand Down
8 changes: 4 additions & 4 deletions cli/command/container/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func runRun(dockerCli command.Cli, flags *pflag.FlagSet, ropts *runOptions, copt
func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptions, containerCfg *containerConfig) error {
config := containerCfg.Config
stdout, stderr := dockerCli.Out(), dockerCli.Err()
client := dockerCli.Client()
apiClient := dockerCli.Client()

config.ArgsEscaped = false

Expand Down Expand Up @@ -150,7 +150,7 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio
}
if opts.sigProxy {
sigc := notifyAllSignals()
go ForwardAllSignals(ctx, dockerCli, containerID, sigc)
go ForwardAllSignals(ctx, apiClient, containerID, sigc)
defer signal.StopCatch(sigc)
}

Expand Down Expand Up @@ -186,10 +186,10 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio
defer closeFn()
}

statusChan := waitExitOrRemoved(ctx, dockerCli.Client(), containerID, copts.autoRemove)
statusChan := waitExitOrRemoved(ctx, apiClient, containerID, copts.autoRemove)

// start the container
if err := client.ContainerStart(ctx, containerID, container.StartOptions{}); err != nil {
if err := apiClient.ContainerStart(ctx, 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.
Expand Down
6 changes: 3 additions & 3 deletions cli/command/container/signals.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ import (
"os"
gosignal "os/signal"

"github.com/docker/cli/cli/command"
"github.com/docker/docker/client"
"github.com/moby/sys/signal"
"github.com/sirupsen/logrus"
)

// ForwardAllSignals forwards signals to the container
//
// The channel you pass in must already be setup to receive any signals you want to forward.
func ForwardAllSignals(ctx context.Context, cli command.Cli, cid string, sigc <-chan os.Signal) {
func ForwardAllSignals(ctx context.Context, apiClient client.ContainerAPIClient, cid string, sigc <-chan os.Signal) {
var (
s os.Signal
ok bool
Expand Down Expand Up @@ -48,7 +48,7 @@ func ForwardAllSignals(ctx context.Context, cli command.Cli, cid string, sigc <-
continue
}

if err := cli.Client().ContainerKill(ctx, cid, sig); err != nil {
if err := apiClient.ContainerKill(ctx, cid, sig); err != nil {
logrus.Debugf("Error sending signal: %s", err)
}
}
Expand Down
6 changes: 2 additions & 4 deletions cli/command/container/signals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"testing"
"time"

"github.com/docker/cli/internal/test"
"github.com/moby/sys/signal"
)

Expand All @@ -15,16 +14,15 @@ func TestForwardSignals(t *testing.T) {
defer cancel()

called := make(chan struct{})
client := &fakeClient{containerKillFunc: func(ctx context.Context, container, signal string) error {
apiClient := &fakeClient{containerKillFunc: func(ctx context.Context, container, signal string) error {
close(called)
return nil
}}

cli := test.NewFakeCli(client)
sigc := make(chan os.Signal)
defer close(sigc)

go ForwardAllSignals(ctx, cli, t.Name(), sigc)
go ForwardAllSignals(ctx, apiClient, t.Name(), sigc)

timer := time.NewTimer(30 * time.Second)
defer timer.Stop()
Expand Down
6 changes: 2 additions & 4 deletions cli/command/container/signals_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"testing"
"time"

"github.com/docker/cli/internal/test"
"golang.org/x/sys/unix"
"gotest.tools/v3/assert"
)
Expand All @@ -23,18 +22,17 @@ func TestIgnoredSignals(t *testing.T) {
defer cancel()

var called bool
client := &fakeClient{containerKillFunc: func(ctx context.Context, container, signal string) error {
apiClient := &fakeClient{containerKillFunc: func(ctx context.Context, container, signal string) error {
called = true
return nil
}}

cli := test.NewFakeCli(client)
sigc := make(chan os.Signal)
defer close(sigc)

done := make(chan struct{})
go func() {
ForwardAllSignals(ctx, cli, t.Name(), sigc)
ForwardAllSignals(ctx, apiClient, t.Name(), sigc)
close(done)
}()

Expand Down
2 changes: 1 addition & 1 deletion cli/command/container/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func RunStart(dockerCli command.Cli, opts *StartOptions) error {
// We always use c.ID instead of container to maintain consistency during `docker start`
if !c.Config.Tty {
sigc := notifyAllSignals()
go ForwardAllSignals(ctx, dockerCli, c.ID, sigc)
go ForwardAllSignals(ctx, dockerCli.Client(), c.ID, sigc)
defer signal.StopCatch(sigc)
}

Expand Down

0 comments on commit 7497075

Please sign in to comment.