diff --git a/src/frontend/apiserver/backend/clib.go b/src/frontend/apiserver/backend/clib.go index 40bcd52b..3a70a218 100644 --- a/src/frontend/apiserver/backend/clib.go +++ b/src/frontend/apiserver/backend/clib.go @@ -606,6 +606,15 @@ const backendDegradedTTL = 10 * time.Second // (reconnect.go) and currentClientConfigNMasters() (above). var notReadyRC = -int(C.E_XCE_NOT_READY) +// submit_errors.go's error numbers must match errnox.h (compile-time check: +// a mismatch makes an array index out of range). +var ( + _ = [1]int{}[C.E_XCE_QUEUE_CLOSED-xceErrQueueClosed] + _ = [1]int{}[xceErrQueueClosed-C.E_XCE_QUEUE_CLOSED] + _ = [1]int{}[C.E_XCE_BAD_QUEUE-xceErrBadQueue] + _ = [1]int{}[xceErrBadQueue-C.E_XCE_BAD_QUEUE] +) + // callBackend is the single shared pre-call/post-call helper every // gCtx.ctx-based backend-call wrapper below (SubmitJob, ControlJob, // ControlQueue, ControlHost, FetchFile) calls around its own single @@ -665,9 +674,12 @@ func SubmitJob(job *models.JobInfo, arraySize int) error { js.array_size = C.uint64_t(arraySize) var outJobID C.uint64_t - _, err := callBackend("xcecli_jobsubmit", func() int { + rc, err := callBackend("xcecli_jobsubmit", func() int { return int(C.xcecli_jobsubmit(gCtx.ctx, js, &outJobID)) }) + if qerr := submitRCError(rc); qerr != nil { + return qerr + } if err != nil { return err } diff --git a/src/frontend/apiserver/backend/submit_errors.go b/src/frontend/apiserver/backend/submit_errors.go new file mode 100644 index 00000000..d20c04c1 --- /dev/null +++ b/src/frontend/apiserver/backend/submit_errors.go @@ -0,0 +1,31 @@ +package backend + +import "errors" + +// Queue rejections of a job submission by the controller (bug-1960), so the +// handlers can answer a client error naming the queue instead of a generic +// backend failure. The queue may have been closed after the apiserver's own +// check, or the submission came with a queue the controller does not know. +var ( + ErrQueueClosed = errors.New("queue is closed") + ErrQueueInvalid = errors.New("invalid queue") +) + +// E_XCE_QUEUE_CLOSED and E_XCE_BAD_QUEUE (include/xce/errnox.h); clib.go +// checks at compile time that they match the C definitions. +const ( + xceErrQueueClosed = 10016 + xceErrBadQueue = 10008 +) + +// submitRCError maps a negative xcecli_jobsubmit result to its typed error, +// or nil when it is not a queue rejection. +func submitRCError(rc int) error { + switch rc { + case -xceErrQueueClosed: + return ErrQueueClosed + case -xceErrBadQueue: + return ErrQueueInvalid + } + return nil +} diff --git a/src/frontend/apiserver/backend/submit_errors_test.go b/src/frontend/apiserver/backend/submit_errors_test.go new file mode 100644 index 00000000..94baed6e --- /dev/null +++ b/src/frontend/apiserver/backend/submit_errors_test.go @@ -0,0 +1,20 @@ +package backend + +import ( + "errors" + "testing" +) + +// bug-1960: the controller's queue rejections become typed errors the +// handlers can turn into client errors. +func TestSubmitRCError(t *testing.T) { + if err := submitRCError(-10016); !errors.Is(err, ErrQueueClosed) { + t.Errorf("-10016: got %v, want ErrQueueClosed", err) + } + if err := submitRCError(-10008); !errors.Is(err, ErrQueueInvalid) { + t.Errorf("-10008: got %v, want ErrQueueInvalid", err) + } + if err := submitRCError(-5); err != nil { + t.Errorf("-5: got %v, want nil (generic failure)", err) + } +} diff --git a/src/frontend/apiserver/controllers/jobs.go b/src/frontend/apiserver/controllers/jobs.go index ac4205cc..36150786 100644 --- a/src/frontend/apiserver/controllers/jobs.go +++ b/src/frontend/apiserver/controllers/jobs.go @@ -1066,6 +1066,10 @@ func (j JobController) RestartJobByID(ctx *gin.Context) { err = submitJob(&newJob, 1) if err != nil { logger.Error("Failed to submit job for restart process.", zap.Error(err)) + if qerr := queueRejection(err, newJob.QueueName); qerr != nil { + ReturnError(ctx, common.INVALID_PARAMS_CODE, common.INVALID_PARAMS_MSG+": "+qerr.Error(), nil) + return + } ReturnError(ctx, common.ERROR_CODE, common.ERROR_WRITE_DATA_MSG+": "+err.Error(), nil) return } @@ -1244,6 +1248,10 @@ func (j JobController) CreateJob(ctx *gin.Context) { err = submitJob(&job, arraySize) if err != nil { logger.Error("Failed to submit job.", zap.Error(err)) + if qerr := queueRejection(err, job.QueueName); qerr != nil { + ReturnError(ctx, common.INVALID_PARAMS_CODE, common.INVALID_PARAMS_MSG+": "+qerr.Error(), nil) + return + } ReturnError(ctx, common.ERROR_CODE, common.ERROR_WRITE_DATA_MSG+": "+err.Error(), nil) return } @@ -3190,3 +3198,16 @@ func GetHostList(hosts []string) ([]string, error) { } return execHosts, nil } + +// queueRejection turns the controller's queue rejection of a submission +// (bug-1960) into the client error the apiserver's own check would give, +// or returns nil for any other failure. +func queueRejection(err error, queue string) error { + switch { + case errors.Is(err, backend.ErrQueueClosed): + return fmt.Errorf("cannot submit job to queue %s: queue is closed", queue) + case errors.Is(err, backend.ErrQueueInvalid): + return fmt.Errorf("invalid queue name: %s", queue) + } + return nil +} diff --git a/src/frontend/apiserver/controllers/jobs_handler_test.go b/src/frontend/apiserver/controllers/jobs_handler_test.go index a378f963..44339cb8 100644 --- a/src/frontend/apiserver/controllers/jobs_handler_test.go +++ b/src/frontend/apiserver/controllers/jobs_handler_test.go @@ -1577,3 +1577,29 @@ func TestJobHandler_SubmitRejectsNonObjectExtendData(t *testing.T) { } } } + +// bug-1960: a submission the controller rejects for its queue (closed after +// the apiserver's own check, or unknown) is a client error naming the queue, +// not a 500 with a raw error number. +func TestJobHandler_SubmitQueueRejectedByController(t *testing.T) { + for _, tc := range []struct { + err error + want string + }{ + {backend.ErrQueueClosed, "queue is closed"}, + {backend.ErrQueueInvalid, "invalid queue"}, + } { + f := newHandlerFixture(t) + f.seedQueue("normal") + f.submitErr = tc.err + w := f.submit(map[string]interface{}{"command": "sleep 1", "queue_name": "normal"}) + if w.Code != http.StatusBadRequest { + t.Errorf("%v: status = %d, want 400; body: %s", tc.err, w.Code, w.Body.String()) + continue + } + msg, _ := decodeHandlerResponse(t, w).Msg.(string) + if !strings.Contains(msg, tc.want) || !strings.Contains(msg, "normal") { + t.Errorf("%v: msg = %q, want it to name the queue and say %q", tc.err, msg, tc.want) + } + } +}