From a5dca035984078e8d1c1fc136a5752b77c32a7d6 Mon Sep 17 00:00:00 2001 From: Adam Babik Date: Thu, 25 Jul 2024 00:31:44 +0200 Subject: [PATCH] F --- internal/command/command_unix.go | 15 ++++++++------- internal/command/command_unix_test.go | 2 +- internal/command/command_virtual.go | 12 +++++------- internal/command/command_windows.go | 2 +- internal/runnerv2service/service_execute_test.go | 12 ++++-------- 5 files changed, 19 insertions(+), 24 deletions(-) diff --git a/internal/command/command_unix.go b/internal/command/command_unix.go index d27377fb7..13abcfdc6 100644 --- a/internal/command/command_unix.go +++ b/internal/command/command_unix.go @@ -12,13 +12,14 @@ import ( "golang.org/x/sys/unix" ) -// func setSysProcAttrCtty(cmd *exec.Cmd) { -// if cmd.SysProcAttr == nil { -// cmd.SysProcAttr = &syscall.SysProcAttr{} -// } -// cmd.SysProcAttr.Setctty = true -// cmd.SysProcAttr.Setsid = true -// } +func setSysProcAttrCtty(cmd *exec.Cmd, tty int) { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Ctty = tty + cmd.SysProcAttr.Setctty = true + cmd.SysProcAttr.Setsid = true +} func setSysProcAttrPgid(cmd *exec.Cmd) { if cmd.SysProcAttr == nil { diff --git a/internal/command/command_unix_test.go b/internal/command/command_unix_test.go index 62c16be47..38a2eb5ed 100644 --- a/internal/command/command_unix_test.go +++ b/internal/command/command_unix_test.go @@ -482,7 +482,7 @@ func TestCommand_SimulateCtrlC(t *testing.T) { require.NoError(t, err) } - require.EqualError(t, cmd.Wait(), "signal: killed") + require.EqualError(t, cmd.Wait(), "exit status 130") } func TestCommand_StopWithSignal(t *testing.T) { diff --git a/internal/command/command_virtual.go b/internal/command/command_virtual.go index ea27d4d73..180c522c1 100644 --- a/internal/command/command_virtual.go +++ b/internal/command/command_virtual.go @@ -81,13 +81,11 @@ func (c *virtualCommand) Start(ctx context.Context) (err error) { c.cmd.Stdout = c.tty c.cmd.Stderr = c.tty - // TODO(adamb): creating a new session and setting a controling terminal - // might be required; find the use case and then implement it properly. - // The current implementation misses setting Ctty. - // setSysProcAttrCtty(c.cmd) - // Creating a new process group is required to properly replicate a behaviour - // similar to CTRL-C in the terminal, which sends a SIGINT to the whole group. - setSysProcAttrPgid(c.cmd) + // Create a new session and set the controlling terminal to tty. + // The new process group is created automatically so that sending + // a signal to the command will affect the whole group. + setSysProcAttrCtty(c.cmd, 3) // 3 is the index of the i-th element in ExtraFiles + c.cmd.ExtraFiles = []*os.File{c.tty} c.logger.Info("starting a virtual command", zap.Any("config", redactConfig(c.ProgramConfig()))) if err := c.cmd.Start(); err != nil { diff --git a/internal/command/command_windows.go b/internal/command/command_windows.go index a452600a7..59b748999 100644 --- a/internal/command/command_windows.go +++ b/internal/command/command_windows.go @@ -9,7 +9,7 @@ import ( "github.com/pkg/errors" ) -// func setSysProcAttrCtty(cmd *exec.Cmd) {} +func setSysProcAttrCtty(cmd *exec.Cmd, tty int) {} func setSysProcAttrPgid(cmd *exec.Cmd) {} diff --git a/internal/runnerv2service/service_execute_test.go b/internal/runnerv2service/service_execute_test.go index 733a2a228..da0d3375e 100644 --- a/internal/runnerv2service/service_execute_test.go +++ b/internal/runnerv2service/service_execute_test.go @@ -710,22 +710,18 @@ func TestRunnerServiceServerExecute_WithInput(t *testing.T) { // cancel sleep time.Sleep(time.Millisecond * 500) - err = stream.Send(&runnerv2alpha1.ExecuteRequest{InputData: []byte{0x04}}) + err = stream.Send(&runnerv2alpha1.ExecuteRequest{InputData: []byte{0x03}}) assert.NoError(t, err) - // terminate outer shell and its children time.Sleep(time.Millisecond * 500) - err = stream.Send(&runnerv2alpha1.ExecuteRequest{ - // TODO(adamb): figure out why SIGINT does not work - Stop: runnerv2alpha1.ExecuteStop_EXECUTE_STOP_KILL, - }) + err = stream.Send(&runnerv2alpha1.ExecuteRequest{InputData: []byte{0x04}}) assert.NoError(t, err) result := <-execResult // TODO(adamb): This should be a specific gRPC error rather than Unknown. - assert.Contains(t, result.Err.Error(), "signal: killed") - assert.Equal(t, 137, result.ExitCode) + assert.Contains(t, result.Err.Error(), "exit status 130") + assert.Equal(t, 130, result.ExitCode) }) t.Run("CloseSendDirection", func(t *testing.T) {