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

feat: support for --sig-proxy in run #3043

Merged
merged 1 commit into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 9 additions & 2 deletions cmd/nerdctl/container_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func setCreateFlags(cmd *cobra.Command) {
cmd.Flags().Bool("help", false, "show help")

cmd.Flags().BoolP("tty", "t", false, "Allocate a pseudo-TTY")
cmd.Flags().Bool("sig-proxy", true, "Proxy received signals to the process (default true)")
cmd.Flags().BoolP("interactive", "i", false, "Keep STDIN open even if not attached")
cmd.Flags().String("restart", "no", `Restart policy to apply when a container exits (implemented values: "no"|"always|on-failure:n|unless-stopped")`)
cmd.RegisterFlagCompletionFunc("restart", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
Expand Down Expand Up @@ -287,6 +288,10 @@ func processCreateCommandFlagsInRun(cmd *cobra.Command) (opt types.ContainerCrea

opt.InRun = true

opt.SigProxy, err = cmd.Flags().GetBool("sig-proxy")
if err != nil {
return
}
opt.Interactive, err = cmd.Flags().GetBool("interactive")
if err != nil {
return
Expand Down Expand Up @@ -394,8 +399,10 @@ func runAction(cmd *cobra.Command, args []string) error {
log.L.WithError(err).Error("console resize")
}
} else {
sigC := signalutil.ForwardAllSignals(ctx, task)
defer signalutil.StopCatch(sigC)
if createOpt.SigProxy {
sigC := signalutil.ForwardAllSignals(ctx, task)
defer signalutil.StopCatch(sigC)
}
}

statusC, err := task.Wait(ctx)
Expand Down
75 changes: 75 additions & 0 deletions cmd/nerdctl/container_run_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"runtime"
"strconv"
"strings"
"syscall"
"testing"
"time"

Expand Down Expand Up @@ -311,6 +312,80 @@ func TestRunTTY(t *testing.T) {
assert.Equal(t, 0, res.ExitCode, res.Combined())
}

func runSigProxy(t *testing.T, args ...string) (string, bool, bool) {
t.Parallel()
base := testutil.NewBase(t)
testContainerName := testutil.Identifier(t)
defer base.Cmd("rm", "-f", testContainerName).Run()

fullArgs := []string{"run"}
fullArgs = append(fullArgs, args...)
fullArgs = append(fullArgs,
"--name",
testContainerName,
testutil.CommonImage,
"sh",
"-c",
testutil.SigProxyTestScript,
)

result := base.Cmd(fullArgs...).Start()
process := result.Cmd.Process

// Waits until we reach the trap command in the shell script, then sends SIGINT.
time.Sleep(3 * time.Second)
syscall.Kill(process.Pid, syscall.SIGINT)

// Waits until SIGINT is sent and responded to, then kills process to avoid timeout
time.Sleep(3 * time.Second)
process.Kill()

sigIntRecieved := strings.Contains(result.Stdout(), testutil.SigProxyTrueOut)
timedOut := strings.Contains(result.Stdout(), testutil.SigProxyTimeoutMsg)

return result.Stdout(), sigIntRecieved, timedOut
}

func TestRunSigProxy(t *testing.T) {

type testCase struct {
name string
args []string
want bool
expectedOut string
}
testCases := []testCase{
{
name: "SigProxyDefault",
args: []string{},
want: true,
expectedOut: testutil.SigProxyTrueOut,
},
{
name: "SigProxyTrue",
args: []string{"--sig-proxy=true"},
want: true,
expectedOut: testutil.SigProxyTrueOut,
},
{
name: "SigProxyFalse",
args: []string{"--sig-proxy=false"},
want: false,
expectedOut: "",
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
stdout, sigIntRecieved, timedOut := runSigProxy(t, tc.args...)
errorMsg := fmt.Sprintf("%s failed;\nExpected: '%s'\nActual: '%s'", tc.name, tc.expectedOut, stdout)
assert.Equal(t, false, timedOut, errorMsg)
assert.Equal(t, tc.want, sigIntRecieved, errorMsg)
})
}
}

func TestRunWithFluentdLogDriver(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("fluentd log driver is not yet implemented on Windows")
Expand Down
3 changes: 2 additions & 1 deletion docs/command-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ Basic flags:
- :whale: :blue_square: `-i, --interactive`: Keep STDIN open even if not attached"
- :whale: :blue_square: `-t, --tty`: Allocate a pseudo-TTY
- :warning: WIP: currently `-t` conflicts with `-d`
- :whale: `-sig-proxy`: Proxy received signals to the process (default true)
- :whale: :blue_square: `-d, --detach`: Run container in background and print container ID
- :whale: `--restart=(no|always|on-failure|unless-stopped)`: Restart policy to apply when a container exits
- Default: "no"
Expand Down Expand Up @@ -388,7 +389,7 @@ IPFS flags:
Unimplemented `docker run` flags:
`--attach`, `--blkio-weight-device`, `--cpu-rt-*`, `--device-*`,
`--disable-content-trust`, `--domainname`, `--expose`, `--health-*`, `--isolation`, `--no-healthcheck`,
`--link*`, `--publish-all`, `--sig-proxy`, `--storage-opt`,
`--link*`, `--publish-all`, `--storage-opt`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the usage of sig-proxy?

Also, please squash the commits

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏾
Added it

`--userns`, `--volume-driver`

### :whale: :blue_square: nerdctl exec
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/types/container_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ type ContainerCreateOptions struct {
Interactive bool
// TTY specifies whether to allocate a pseudo-TTY for the container
TTY bool
// SigProxy specifies whether to proxy all received signals to the process
SigProxy bool
// Detach runs container in background and print container ID
Detach bool
// The key sequence for detaching a container.
Expand Down
1 change: 1 addition & 0 deletions pkg/composer/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type RunOptions struct {
Detach bool
NoDeps bool
Tty bool
SigProxy bool
Interactive bool
Rm bool
User string
Expand Down
5 changes: 5 additions & 0 deletions pkg/testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,11 @@ func (c *Cmd) runIfNecessary() *icmd.Result {
return c.runResult
}

func (c *Cmd) Start() *icmd.Result {
c.Base.T.Helper()
return icmd.StartCmd(c.Cmd)
}

func (c *Cmd) CmdOption(cmdOptions ...func(*Cmd)) *Cmd {
for _, opt := range cmdOptions {
opt(c)
Expand Down
19 changes: 19 additions & 0 deletions pkg/testutil/testutil_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,25 @@ var (
// It should be "connection refused" as per the TCP RFC.
// https://www.rfc-editor.org/rfc/rfc793
ExpectedConnectionRefusedError = "connection refused"

SigProxyTrueOut = "received SIGINT"
SigProxyTimeoutMsg = "Timed Out; No signal received"
SigProxyTestScript = `#!/bin/sh
set -eu

sig_msg () {
printf "` + SigProxyTrueOut + `"
end
}

trap sig_msg INT
timeout=0
while [ $timeout -ne 10 ]; do
timeout=$((timeout+1))
sleep 1
done
printf "` + SigProxyTimeoutMsg + `"
end`
)

const (
Expand Down
Loading