diff --git a/src/frontend/apiserver/backend/clib.go b/src/frontend/apiserver/backend/clib.go index 9f5040dc..ec264c85 100644 --- a/src/frontend/apiserver/backend/clib.go +++ b/src/frontend/apiserver/backend/clib.go @@ -680,8 +680,9 @@ func SubmitJob(job *models.JobInfo, arraySize int) error { } // 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 -// signal name (e.g. "TERM") for kill operations. +// action must be one of the models.JOB_* constants; content is the +// 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 { var ctrl C.xce_jobctrl_t 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. // xce_jobctrl_t.u is a C union — we use a static C helper to set the // right branch safely. + args, err := jobCtrlParams(action, content) + if err != nil { + return err + } switch action { case models.JOB_STOP, models.JOB_RESUME, models.JOB_KILL: var sig C.xce_signal_t - sig.signo = C.int(signalNumber(content, action)) - sig.force = 0 + sig.signo = C.int(args.Signo) + sig.force = C.int(args.Force) C.jobctrl_set_siginfo(&ctrl, &sig) case models.JOB_CHKPNT: 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) } - _, err := callBackend("xcecli_jobctrl", func() int { + _, err = callBackend("xcecli_jobctrl", func() int { return int(C.xcecli_jobctrl(gCtx.ctx, &ctrl)) }) return err @@ -744,25 +754,6 @@ func ControlHost(hostname string, action int) error { 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 // controller→worker chain. The filepath must be the fully resolved absolute // path on the exec host. diff --git a/src/frontend/apiserver/backend/convert_pure.go b/src/frontend/apiserver/backend/convert_pure.go index 1a3feb20..15b036cd 100644 --- a/src/frontend/apiserver/backend/convert_pure.go +++ b/src/frontend/apiserver/backend/convert_pure.go @@ -7,6 +7,7 @@ import ( "fmt" "regexp" "strconv" + "strings" "time" ) @@ -215,6 +216,107 @@ func jobCtrlUnionDiscriminant(action int) int { 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 // caller-specified ExecUser if present, otherwise the submitting user // (bug-1780: an unset execuser in the startjob request reaches xcecd as diff --git a/src/frontend/apiserver/backend/jobctrl_params_test.go b/src/frontend/apiserver/backend/jobctrl_params_test.go new file mode 100644 index 00000000..01f34fe3 --- /dev/null +++ b/src/frontend/apiserver/backend/jobctrl_params_test.go @@ -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) + } +}