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:
parent
8f164b8205
commit
4d2e80c4bb
3 changed files with 236 additions and 25 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
118
src/frontend/apiserver/backend/jobctrl_params_test.go
Normal file
118
src/frontend/apiserver/backend/jobctrl_params_test.go
Normal 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)
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue