fix(apiserver): answer the controller's queue rejections as client errors (bug-1960)
Since the controller rejects submissions to closed or unknown queues (E_XCE_QUEUE_CLOSED, E_XCE_BAD_QUEUE), the apiserver answered them as a 500 "xcecli_jobsubmit failed: -10016". SubmitJob now maps them to backend.ErrQueueClosed / ErrQueueInvalid (numbers checked against errnox.h at compile time), and the submit and restart handlers answer 400 naming the queue, as the apiserver's own pre-check does. This covers a queue closed between that check and the controller's. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b239db5fa3
commit
4f57904134
5 changed files with 111 additions and 1 deletions
|
|
@ -606,6 +606,15 @@ const backendDegradedTTL = 10 * time.Second
|
||||||
// (reconnect.go) and currentClientConfigNMasters() (above).
|
// (reconnect.go) and currentClientConfigNMasters() (above).
|
||||||
var notReadyRC = -int(C.E_XCE_NOT_READY)
|
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
|
// callBackend is the single shared pre-call/post-call helper every
|
||||||
// gCtx.ctx-based backend-call wrapper below (SubmitJob, ControlJob,
|
// gCtx.ctx-based backend-call wrapper below (SubmitJob, ControlJob,
|
||||||
// ControlQueue, ControlHost, FetchFile) calls around its own single
|
// 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)
|
js.array_size = C.uint64_t(arraySize)
|
||||||
|
|
||||||
var outJobID C.uint64_t
|
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))
|
return int(C.xcecli_jobsubmit(gCtx.ctx, js, &outJobID))
|
||||||
})
|
})
|
||||||
|
if qerr := submitRCError(rc); qerr != nil {
|
||||||
|
return qerr
|
||||||
|
}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
|
||||||
31
src/frontend/apiserver/backend/submit_errors.go
Normal file
31
src/frontend/apiserver/backend/submit_errors.go
Normal file
|
|
@ -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
|
||||||
|
}
|
||||||
20
src/frontend/apiserver/backend/submit_errors_test.go
Normal file
20
src/frontend/apiserver/backend/submit_errors_test.go
Normal file
|
|
@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -1066,6 +1066,10 @@ func (j JobController) RestartJobByID(ctx *gin.Context) {
|
||||||
err = submitJob(&newJob, 1)
|
err = submitJob(&newJob, 1)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logger.Error("Failed to submit job for restart process.", zap.Error(err))
|
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)
|
ReturnError(ctx, common.ERROR_CODE, common.ERROR_WRITE_DATA_MSG+": "+err.Error(), nil)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
@ -1244,6 +1248,10 @@ func (j JobController) CreateJob(ctx *gin.Context) {
|
||||||
err = submitJob(&job, arraySize)
|
err = submitJob(&job, arraySize)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logger.Error("Failed to submit job.", zap.Error(err))
|
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)
|
ReturnError(ctx, common.ERROR_CODE, common.ERROR_WRITE_DATA_MSG+": "+err.Error(), nil)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
@ -3190,3 +3198,16 @@ func GetHostList(hosts []string) ([]string, error) {
|
||||||
}
|
}
|
||||||
return execHosts, nil
|
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
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue