fix(apiserver): check the default queue exists and is open on submit (bug-1960)
Cause: preCheckJobRequest (controllers/jobs.go) ran the reserved-name, existence and closed checks only when a queue name was given; an empty name was set to "normal" and skipped them, so jobs were accepted into a closed default queue. Fix: default the name first, then run the same checks for every submission. Two existing tests now seed the default queue. (The controller side of bug-1960 is fixed separately.) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
387a421c57
commit
1ff068f6e7
3 changed files with 63 additions and 19 deletions
|
|
@ -1311,26 +1311,27 @@ func preCheckJobRequest(req *models.JobSubmitRequest) (n int, isArray bool, err
|
|||
// goes through the same checks and conversions below
|
||||
n, isArray = 1, false
|
||||
if req.QueueName == "" {
|
||||
// bug-1960: the default queue gets the same existence and closed
|
||||
// checks as a named one below
|
||||
req.QueueName = "normal"
|
||||
} else {
|
||||
// Reject submissions to the reserved internal recovery queue.
|
||||
if req.QueueName == "lost_and_found" {
|
||||
logger.Error("Job submission to reserved queue rejected",
|
||||
zap.String("queue_name", req.QueueName))
|
||||
return 0, false, fmt.Errorf(
|
||||
"cannot submit jobs to 'lost_and_found' — this is an internal recovery queue")
|
||||
}
|
||||
//validate queue_name whether exists
|
||||
queueInfo, ok := store.Queues.Get(req.QueueName)
|
||||
if !ok {
|
||||
logger.Error("Invalid queue name specified", zap.String("queue_name", req.QueueName))
|
||||
return 0, false, fmt.Errorf("invalid queue name: %s", req.QueueName)
|
||||
}
|
||||
queue_statusStr := formatQueueStatus(int64(queueInfo.QueueStatus))
|
||||
if strings.HasPrefix(strings.ToLower(queue_statusStr), "closed") {
|
||||
logger.Error("Queue is closed", zap.String("queue_name", req.QueueName))
|
||||
return 0, false, fmt.Errorf("cannot submit job to queue %s: queue is closed", req.QueueName)
|
||||
}
|
||||
}
|
||||
// Reject submissions to the reserved internal recovery queue.
|
||||
if req.QueueName == "lost_and_found" {
|
||||
logger.Error("Job submission to reserved queue rejected",
|
||||
zap.String("queue_name", req.QueueName))
|
||||
return 0, false, fmt.Errorf(
|
||||
"cannot submit jobs to 'lost_and_found' — this is an internal recovery queue")
|
||||
}
|
||||
//validate queue_name whether exists
|
||||
queueInfo, ok := store.Queues.Get(req.QueueName)
|
||||
if !ok {
|
||||
logger.Error("Invalid queue name specified", zap.String("queue_name", req.QueueName))
|
||||
return 0, false, fmt.Errorf("invalid queue name: %s", req.QueueName)
|
||||
}
|
||||
queue_statusStr := formatQueueStatus(int64(queueInfo.QueueStatus))
|
||||
if strings.HasPrefix(strings.ToLower(queue_statusStr), "closed") {
|
||||
logger.Error("Queue is closed", zap.String("queue_name", req.QueueName))
|
||||
return 0, false, fmt.Errorf("cannot submit job to queue %s: queue is closed", req.QueueName)
|
||||
}
|
||||
if req.JobName != "" {
|
||||
// A name containing '[' or ']' is an array index expression; it is
|
||||
|
|
|
|||
|
|
@ -413,6 +413,7 @@ func TestJobHandler_SmokeSubmitOrdinaryJob(t *testing.T) {
|
|||
// as a 500 and nothing is recorded.
|
||||
func TestJobHandler_SmokeSubmitErrorIs500(t *testing.T) {
|
||||
f := newHandlerFixture(t)
|
||||
f.seedQueue("normal") // the default queue must exist (bug-1960)
|
||||
f.submitErr = errors.New("backend down")
|
||||
|
||||
w := f.submit(map[string]interface{}{"job_name": "smoke", "command": "sleep 1"})
|
||||
|
|
|
|||
|
|
@ -122,6 +122,47 @@ func TestPreCheckJobRequest_EmptyQueue(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// bug-1960: a submission without a queue goes to the default queue "normal",
|
||||
// which must pass the same closed check as an explicitly named queue.
|
||||
func TestPreCheckJobRequest_DefaultQueueClosed(t *testing.T) {
|
||||
resetJobSubmitState()
|
||||
store.Queues.Put("normal", models.QueueInfo{
|
||||
QueueName: "normal",
|
||||
QueueStatus: int(models.QUEUESTAT["CLOSED"]),
|
||||
})
|
||||
|
||||
req := &models.JobSubmitRequest{QueueName: "", Command: "sleep 1"}
|
||||
_, _, err := preCheckJobRequest(req)
|
||||
if err == nil || !strings.Contains(err.Error(), "queue is closed") {
|
||||
t.Fatalf("submission to the closed default queue: err = %v, want \"queue is closed\"", err)
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1960: the default queue must also exist, as a named queue must.
|
||||
func TestPreCheckJobRequest_DefaultQueueMissing(t *testing.T) {
|
||||
resetJobSubmitState()
|
||||
|
||||
req := &models.JobSubmitRequest{QueueName: "", Command: "sleep 1"}
|
||||
_, _, err := preCheckJobRequest(req)
|
||||
if err == nil || !strings.Contains(err.Error(), "invalid queue name: normal") {
|
||||
t.Fatalf("submission with no default queue: err = %v, want \"invalid queue name: normal\"", err)
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1960: an open default queue is still accepted and filled in.
|
||||
func TestPreCheckJobRequest_DefaultQueueOpen(t *testing.T) {
|
||||
resetJobSubmitState()
|
||||
store.Queues.Put("normal", models.QueueInfo{QueueName: "normal"})
|
||||
|
||||
req := &models.JobSubmitRequest{QueueName: "", Command: "sleep 1"}
|
||||
if _, _, err := preCheckJobRequest(req); err != nil {
|
||||
t.Fatalf("submission to the open default queue rejected: %v", err)
|
||||
}
|
||||
if req.QueueName != "normal" {
|
||||
t.Errorf("QueueName = %q, want normal", req.QueueName)
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateJobControl_HandlesRealJobInfoTypes is a regression guard for
|
||||
// bug-1742 (gitea #45): models.JobInfo.JobStatus and .ArrayIdx are plain Go
|
||||
// `int` (see the JobInfo struct in models/jobs.go), but validateJobControl
|
||||
|
|
@ -278,6 +319,7 @@ func TestPreCheckJobRequest_NamedJobsGetFullValidation(t *testing.T) {
|
|||
origCfg := utils.JobYamlConfig
|
||||
utils.JobYamlConfig = &utils.JobConfig{MaxArraySize: 1000}
|
||||
t.Cleanup(func() { utils.JobYamlConfig = origCfg })
|
||||
store.Queues.Put("normal", models.QueueInfo{QueueName: "normal"}) // default queue (bug-1960)
|
||||
|
||||
for _, name := range []string{"", "myjob", "arr[1-3]"} {
|
||||
req := &models.JobSubmitRequest{JobName: name, Command: "sleep 1"}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue