Skip to content
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
8 changes: 8 additions & 0 deletions cmpserver/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ func runCommand(ctx context.Context, command Command, path string, env []string)
<-ctx.Done()
// Kill by group ID to make sure child processes are killed. The - tells `kill` that it's a group ID.
// Since we didn't set Pgid in SysProcAttr, the group ID is the same as the process ID. https://pkg.go.dev/syscall#SysProcAttr

// Sending a TERM signal first to allow any potential cleanup if needed, and then sending a KILL signal
_ = sysCallTerm(-cmd.Process.Pid)

// modify cleanup timeout to allow process to cleanup
cleanupTimeout := 5 * time.Second
time.Sleep(cleanupTimeout)

_ = sysCallKill(-cmd.Process.Pid)
}()

Expand Down
22 changes: 22 additions & 0 deletions cmpserver/plugin/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,28 @@ func TestRunCommandEmptyCommand(t *testing.T) {
assert.ErrorContains(t, err, "Command is empty")
}

// TestRunCommandContextTimeoutWithGracefulTermination makes sure that the process is given enough time to cleanup before sending SIGKILL.
func TestRunCommandContextTimeoutWithCleanup(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 900*time.Millisecond)
defer cancel()

// Use a subshell so there's a child command.
// This command sleeps for 4 seconds which is currently less than the 5 second delay between SIGTERM and SIGKILL signal and then exits successfully.
command := Command{
Command: []string{"sh", "-c"},
Args: []string{`(trap 'echo "cleanup completed"; exit' TERM; sleep 4)`},
}

before := time.Now()
output, err := runCommand(ctx, command, "", []string{})
after := time.Now()

assert.Error(t, err) // The command should time out, causing an error.
assert.Less(t, after.Sub(before), 1*time.Second)
// The command should still have completed the cleanup after termination.
assert.Contains(t, output, "cleanup completed")
}

func Test_getParametersAnnouncement_empty_command(t *testing.T) {
staticYAML := `
- name: static-a
Expand Down
4 changes: 4 additions & 0 deletions cmpserver/plugin/plugin_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ func newSysProcAttr(setpgid bool) *syscall.SysProcAttr {
func sysCallKill(pid int) error {
return syscall.Kill(pid, syscall.SIGKILL)
}

func sysCallTerm(pid int) error {
return syscall.Kill(pid, syscall.SIGTERM)
}