fix(cmp): send sigterm to cmp commands before sigkill to allow for po…

…tential cleanup (#9180) (#14955)

* fix: send sigterm to cmp commands before sigkill to allow for potential cleanup

Signed-off-by: Ashin Sabu <[email protected]>

* fix: unit test for runCommand in cmpserver to test cleanup modified

Signed-off-by: Ashin Sabu <[email protected]>

* fix: change unit test for plugin/runCommand to avoid bad trap along with lint fix

Signed-off-by: Ashin Sabu <[email protected]>

---------

Signed-off-by: Ashin Sabu <[email protected]>

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)
 	}()
 

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

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)
+}