Skip to content

Commit

Permalink
Merge pull request docker#4638 from thaJeztah/ForwardAllSignals_no_cli
Browse files Browse the repository at this point in the history
cli/command/container: ForwardAllSignals: rewrite to use ContainerAPIClient
  • Loading branch information
thaJeztah authored Nov 13, 2023
2 parents 6424018 + 3cd77c9 commit 1862725
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(ctx context.Context, dockerCli command.Cli, target string, opts *

if opts.Proxy && !c.Config.Tty {
sigc := notifyAllSignals()
go ForwardAllSignals(ctx, dockerCli, target, sigc)
go ForwardAllSignals(ctx, apiClient, target, 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 1862725

Please sign in to comment.