diff --git a/cmpserver/plugin/plugin.go b/cmpserver/plugin/plugin.go index ca67ccecf214a..f03b73f24dcf6 100644 --- a/cmpserver/plugin/plugin.go +++ b/cmpserver/plugin/plugin.go @@ -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) }() diff --git a/cmpserver/plugin/plugin_test.go b/cmpserver/plugin/plugin_test.go index 936a38caba934..b253dc414cbdc 100644 --- a/cmpserver/plugin/plugin_test.go +++ b/cmpserver/plugin/plugin_test.go @@ -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 diff --git a/cmpserver/plugin/plugin_unix.go b/cmpserver/plugin/plugin_unix.go index a9dc157bc7ef8..ea6b7b5493910 100644 --- a/cmpserver/plugin/plugin_unix.go +++ b/cmpserver/plugin/plugin_unix.go @@ -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) +}