fix(apiserver): send the requested signal, force kill and checkpoint options (bug-1940)

Cause: ControlJob (backend/clib.go) looked its content argument up as a
signal name, but the job control handlers pass a JSON JobControlEvent, so
the lookup always missed and every kill sent TERM (stop TSTP, resume
CONT); sig.force and ckpt.kill were hard-coded to 0 and the checkpoint
method was never set.

Fix: new pure jobCtrlParams (backend/convert_pure.go) decodes the event
(or a bare signal name/number), validates the signal against
JobSignalMapTable and returns an error instead of falling back, and maps
force_kill to xce_signal_t.force (kill) or xce_ckpt_options_t.kill
(checkpoint) plus the checkpoint method. ControlJob fills the C structs
from it. Tests: backend/jobctrl_params_test.go.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-10 08:31:16 +00:00
parent 8f164b8205
commit 4d2e80c4bb
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
3 changed files with 236 additions and 25 deletions

View file

@ -680,8 +680,9 @@ func SubmitJob(job *models.JobInfo, arraySize int) error {
} }
// ControlJob sends a job control action to the backend via xcecli_jobctrl. // ControlJob sends a job control action to the backend via xcecli_jobctrl.
// action must be one of the models.JOB_* constants; content is an optional // action must be one of the models.JOB_* constants; content is the
// signal name (e.g. "TERM") for kill operations. // JSON-encoded models.JobControlEvent (signal, force_kill, method) or a bare
// signal name/number -- see jobCtrlParams (convert_pure.go).
func ControlJob(jobid uint64, action int, content string) error { func ControlJob(jobid uint64, action int, content string) error {
var ctrl C.xce_jobctrl_t var ctrl C.xce_jobctrl_t
ctrl.jobid = C.uint64_t(jobid) ctrl.jobid = C.uint64_t(jobid)
@ -693,19 +694,28 @@ func ControlJob(jobid uint64, action int, content string) error {
// For signal-based actions set signo; for checkpoint set ckpt options. // For signal-based actions set signo; for checkpoint set ckpt options.
// xce_jobctrl_t.u is a C union — we use a static C helper to set the // xce_jobctrl_t.u is a C union — we use a static C helper to set the
// right branch safely. // right branch safely.
args, err := jobCtrlParams(action, content)
if err != nil {
return err
}
switch action { switch action {
case models.JOB_STOP, models.JOB_RESUME, models.JOB_KILL: case models.JOB_STOP, models.JOB_RESUME, models.JOB_KILL:
var sig C.xce_signal_t var sig C.xce_signal_t
sig.signo = C.int(signalNumber(content, action)) sig.signo = C.int(args.Signo)
sig.force = 0 sig.force = C.int(args.Force)
C.jobctrl_set_siginfo(&ctrl, &sig) C.jobctrl_set_siginfo(&ctrl, &sig)
case models.JOB_CHKPNT: case models.JOB_CHKPNT:
var ckpt C.xce_ckpt_options_t var ckpt C.xce_ckpt_options_t
ckpt.kill = 0 ckpt.kill = C.int(args.CkptKill)
if args.Method != "" {
// NULL makes xce_jobctrl_req() send "default"
ckpt.method = C.CString(args.Method)
defer C.free(unsafe.Pointer(ckpt.method))
}
C.jobctrl_set_ckptoption(&ctrl, &ckpt) C.jobctrl_set_ckptoption(&ctrl, &ckpt)
} }
_, err := callBackend("xcecli_jobctrl", func() int { _, err = callBackend("xcecli_jobctrl", func() int {
return int(C.xcecli_jobctrl(gCtx.ctx, &ctrl)) return int(C.xcecli_jobctrl(gCtx.ctx, &ctrl))
}) })
return err return err
@ -744,25 +754,6 @@ func ControlHost(hostname string, action int) error {
return err return err
} }
// signalNumber maps a job action and optional signal name to a POSIX signal
// number, falling back to sensible defaults.
func signalNumber(signame string, action int) int {
if signame != "" {
if n, ok := models.JobSignalMapTable[signame]; ok {
return n
}
}
switch action {
case models.JOB_KILL:
return models.JobSignalMapTable["TERM"] // 15
case models.JOB_STOP:
return models.JobSignalMapTable["TSTP"] // 20
case models.JOB_RESUME:
return models.JobSignalMapTable["CONT"] // 18
}
return 15 // SIGTERM as safe default
}
// FetchFile fetches up to maxBytes of a job output file from offset via the // FetchFile fetches up to maxBytes of a job output file from offset via the
// controller→worker chain. The filepath must be the fully resolved absolute // controller→worker chain. The filepath must be the fully resolved absolute
// path on the exec host. // path on the exec host.

View file

@ -7,6 +7,7 @@ import (
"fmt" "fmt"
"regexp" "regexp"
"strconv" "strconv"
"strings"
"time" "time"
) )
@ -215,6 +216,107 @@ func jobCtrlUnionDiscriminant(action int) int {
return 0 // JobControl_u_siginfo (stop/resume/kill all signal the job) return 0 // JobControl_u_siginfo (stop/resume/kill all signal the job)
} }
// jobCtrlArgs is what ControlJob (clib.go) puts into xce_jobctrl_t's union
// branch: Signo/Force fill xce_signal_t (stop/resume/kill), CkptKill/Method
// fill xce_ckpt_options_t (checkpoint).
type jobCtrlArgs struct {
Signo int
Force int
CkptKill int
Method string
}
// jobCtrlParams maps a job action and ControlJob's content argument to the
// values sent to the controller. content is either the JSON-encoded
// models.JobControlEvent the job control handlers pass (controllers/jobs.go)
// or, for older callers, a bare signal name or number; empty means the
// action's default signal.
//
// bug-1940: content used to be looked up only as a signal name, so the JSON
// event always missed and every kill sent TERM with force 0, and checkpoint
// kill/method were never set. An unknown signal is now an error instead of a
// silent fallback to the default.
func jobCtrlParams(action int, content string) (jobCtrlArgs, error) {
var (
args jobCtrlArgs
signo int
forceKill bool
)
content = strings.TrimSpace(content)
if strings.HasPrefix(content, "{") {
var evt models.JobControlEvent
if err := json.Unmarshal([]byte(content), &evt); err != nil {
return args, fmt.Errorf("invalid job control content: %w", err)
}
if evt.Signal != 0 && !isKnownSignal(evt.Signal) {
return args, fmt.Errorf("unsupported signal: %d", evt.Signal)
}
signo, forceKill, args.Method = evt.Signal, evt.ForceKill, evt.Method
} else if content != "" {
n, err := parseSignal(content)
if err != nil {
return args, err
}
signo = n
}
switch action {
case models.JOB_STOP, models.JOB_RESUME, models.JOB_KILL:
if signo == 0 {
signo = defaultActionSignal(action)
}
args.Signo = signo
if action == models.JOB_KILL && forceKill {
args.Force = 1
}
case models.JOB_CHKPNT:
if forceKill {
args.CkptKill = 1
}
}
return args, nil
}
// defaultActionSignal is the signal a stop/resume/kill sends when the caller
// gave none.
func defaultActionSignal(action int) int {
switch action {
case models.JOB_STOP:
return models.JobSignalMapTable["TSTP"] // 20
case models.JOB_RESUME:
return models.JobSignalMapTable["CONT"] // 18
}
return models.JobSignalMapTable["TERM"] // 15
}
// isKnownSignal reports whether n is one of models.JobSignalMapTable's values.
func isKnownSignal(n int) bool {
for _, v := range models.JobSignalMapTable {
if v == n {
return true
}
}
return false
}
// parseSignal accepts a signal number or name (case-insensitive, with or
// without the SIG prefix) and returns its number.
func parseSignal(s string) (int, error) {
if n, err := strconv.Atoi(s); err == nil {
if !isKnownSignal(n) {
return 0, fmt.Errorf("unsupported signal: %s", s)
}
return n, nil
}
name := strings.TrimPrefix(strings.ToUpper(s), "SIG")
if n, ok := models.JobSignalMapTable[name]; ok {
return n, nil
}
return 0, fmt.Errorf("unsupported signal: %s", s)
}
// resolveExecUser returns the effective exec user for a job submission: the // resolveExecUser returns the effective exec user for a job submission: the
// caller-specified ExecUser if present, otherwise the submitting user // caller-specified ExecUser if present, otherwise the submitting user
// (bug-1780: an unset execuser in the startjob request reaches xcecd as // (bug-1780: an unset execuser in the startjob request reaches xcecd as

View file

@ -0,0 +1,118 @@
package backend
import (
"apiserver/models"
"encoding/json"
"testing"
)
// jobCtrlEventContent is what the job control handlers (controllers/jobs.go)
// pass to ControlJob as content: a JSON-encoded models.JobControlEvent whose
// Signal is already numeric (preCheckJobControlRequest converts names).
func jobCtrlEventContent(t *testing.T, evt models.JobControlEvent) string {
t.Helper()
b, err := json.Marshal(evt)
if err != nil {
t.Fatalf("marshal JobControlEvent: %v", err)
}
return string(b)
}
// bug-1940: the signal the user asked for and force_kill must reach the
// xce_signal_t sent to the controller; before the fix the JSON content was
// looked up as a signal name, missed, and fell back to TERM with force 0.
func TestJobCtrlParams_KillCarriesRequestedSignalAndForce(t *testing.T) {
content := jobCtrlEventContent(t, models.JobControlEvent{
UniqueJobID: 42, Signal: 10, ForceKill: true,
})
got, err := jobCtrlParams(models.JOB_KILL, content)
if err != nil {
t.Fatalf("jobCtrlParams: unexpected error %v", err)
}
if got.Signo != 10 || got.Force != 1 {
t.Errorf("kill USR1 force_kill: got signo %d force %d, want signo 10 force 1",
got.Signo, got.Force)
}
}
func TestJobCtrlParams_KillWithoutForce(t *testing.T) {
content := jobCtrlEventContent(t, models.JobControlEvent{UniqueJobID: 42, Signal: 9})
got, err := jobCtrlParams(models.JOB_KILL, content)
if err != nil {
t.Fatalf("jobCtrlParams: unexpected error %v", err)
}
if got.Signo != 9 || got.Force != 0 {
t.Errorf("kill KILL: got signo %d force %d, want signo 9 force 0", got.Signo, got.Force)
}
}
// An event without a signal keeps the per-action defaults.
func TestJobCtrlParams_DefaultSignalPerAction(t *testing.T) {
cases := []struct {
action int
want int
}{
{models.JOB_KILL, 15},
{models.JOB_STOP, 20},
{models.JOB_RESUME, 18},
}
for _, tc := range cases {
content := jobCtrlEventContent(t, models.JobControlEvent{UniqueJobID: 42})
got, err := jobCtrlParams(tc.action, content)
if err != nil {
t.Fatalf("action %d: unexpected error %v", tc.action, err)
}
if got.Signo != tc.want {
t.Errorf("action %d: got signo %d, want %d", tc.action, got.Signo, tc.want)
}
}
}
// ControlJob's legacy content form, a bare signal name or number, is still
// accepted (case-insensitive, optional SIG prefix).
func TestJobCtrlParams_BareSignalNameOrNumber(t *testing.T) {
for content, want := range map[string]int{
"USR1": 10, "usr2": 12, "SIGHUP": 1, "9": 9, "": 15,
} {
got, err := jobCtrlParams(models.JOB_KILL, content)
if err != nil {
t.Fatalf("content %q: unexpected error %v", content, err)
}
if got.Signo != want {
t.Errorf("content %q: got signo %d, want %d", content, got.Signo, want)
}
}
}
// An unknown signal is an error, never a silent fallback to TERM.
func TestJobCtrlParams_InvalidSignalIsRejected(t *testing.T) {
for _, content := range []string{
"BOGUS", "0", "99", "-3",
`{"unique_job_id":42,"signal":99}`,
`{"unique_job_id":42,"signal":-1}`,
`{"unique_job_id":42,`,
} {
if got, err := jobCtrlParams(models.JOB_KILL, content); err == nil {
t.Errorf("content %q: want an error, got signo %d", content, got.Signo)
}
}
}
// Checkpoint: force_kill (kill the job after the checkpoint) and the method
// reach xce_ckpt_options_t.
func TestJobCtrlParams_CheckpointCarriesKillAndMethod(t *testing.T) {
content := jobCtrlEventContent(t, models.JobControlEvent{
UniqueJobID: 42, ForceKill: true, Method: "CRIU",
})
got, err := jobCtrlParams(models.JOB_CHKPNT, content)
if err != nil {
t.Fatalf("jobCtrlParams: unexpected error %v", err)
}
if got.CkptKill != 1 || got.Method != "CRIU" {
t.Errorf("checkpoint force_kill CRIU: got kill %d method %q, want kill 1 method \"CRIU\"",
got.CkptKill, got.Method)
}
}