diff --git a/src/frontend/apiserver/controllers/jobs.go b/src/frontend/apiserver/controllers/jobs.go index 51f410f4..0d2da44f 100644 --- a/src/frontend/apiserver/controllers/jobs.go +++ b/src/frontend/apiserver/controllers/jobs.go @@ -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 diff --git a/src/frontend/apiserver/controllers/jobs_handler_test.go b/src/frontend/apiserver/controllers/jobs_handler_test.go index a279aab5..a378f963 100644 --- a/src/frontend/apiserver/controllers/jobs_handler_test.go +++ b/src/frontend/apiserver/controllers/jobs_handler_test.go @@ -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"}) diff --git a/src/frontend/apiserver/controllers/jobs_test.go b/src/frontend/apiserver/controllers/jobs_test.go index 93b598f2..2e0587ef 100644 --- a/src/frontend/apiserver/controllers/jobs_test.go +++ b/src/frontend/apiserver/controllers/jobs_test.go @@ -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"}