diff --git a/.kiro/specs/job-array-one-based-index/tasks.md b/.kiro/specs/job-array-one-based-index/tasks.md index fd02bcdb..bebe2675 100644 --- a/.kiro/specs/job-array-one-based-index/tasks.md +++ b/.kiro/specs/job-array-one-based-index/tasks.md @@ -114,7 +114,7 @@ - _Requirements: 5.2, 5.11_ - _Boundary: store query pipeline_ -- [ ] 5.4 Add handler test seams and the non-cgo handler test fixture +- [x] 5.4 Add handler test seams and the non-cgo handler test fixture - Route ID generation, array ID generation, job submission and control dispatch through replaceable package variables - Create the non-cgo handler test file with a fixture that saves the stores, sets up auth/RBAC, then seeds jobs (controllable status) and their queues and the array-size config, and restores everything on cleanup; the dispatch replacement records each dispatched job - Done when a smoke handler test in the new file submits an ordinary job and controls one seeded job through the seams with the dispatch recorded, and the existing apiserver tests still pass @@ -244,3 +244,4 @@ - 4.1: after `jobsub -a N`, `$jobid` is the array identity, so `expect-job $jobid` currently reports not found; 4.3 must make it behave per 4.7/4.9 (not-found hint naming `[k]`, or exact-ID match). Nit for 4.3: the `refused` early return in `cmd_jobsub` sits between the bug-786 ack comment and the code it describes — move it above the comment. - 4.3: the controller acks `setpend` (`do_set_preason`, job.c ~3869-3880) and `jobrun` (`handle_jobdispatch`, job.c ~2246-2258) with status 0 before doing the work, so those two harness commands can never report not-found; 4.7/5.13 for them depend on a controller change outside this spec's boundary. `jobinfo`'s not-found is `rc == 0` with no row (`do_reply_joblist`), not -ENOENT. - 5.1: Go helpers: `ParseHumanJobIDStrict(s) (uint64, error)` returns sentinel `ErrJobIDSyntax` / `ErrJobIDWidth` (callers trim and split `[...]` first; no whitespace accepted); `MakeElementID(key uint64, k int) uint64`; `IsArrayCapableKey(key)`; `GenerateArrayJobID(c, a, n)` (non-cgo stub returns errNoCGO). +- 5.4: handler fixture in `controllers/jobs_handler_test.go` (`newHandlerFixture`, `seedJob`/`seedOrdinaryJob`/`seedArrayElements`/`seedQueue`, `submit`, `controlByID`, `controlByFilter`, `dispatchedIDs`, `submissions`); submit bodies must be client-style JSON maps (marshalled `models.JobSubmitRequest` breaks on zero CustomTime). Handlers are registered without `RequirePermission`; in-handler RBAC/queue/status checks still run. diff --git a/src/frontend/apiserver/controllers/jobs.go b/src/frontend/apiserver/controllers/jobs.go index 10cde0b7..7ae25885 100644 --- a/src/frontend/apiserver/controllers/jobs.go +++ b/src/frontend/apiserver/controllers/jobs.go @@ -1423,7 +1423,7 @@ func (j JobController) CreateJob(ctx *gin.Context) { } // Generate job ID via C idgen (single call for both normal and array jobs) - jobID, err := backend.GenerateJobID(clusterID, apiserverID, arraySize) + jobID, err := generateJobID(clusterID, apiserverID, arraySize) if err != nil { logger.Error("Failed to generate job ID", zap.Error(err)) ReturnError(ctx, common.ERROR_CODE, "failed to generate job ID: "+err.Error(), nil) @@ -1505,7 +1505,7 @@ func (j JobController) CreateJob(ctx *gin.Context) { logger.Debug("jobinfo before submit.", zap.Any("job", job)) // Submit job to backend (single call handles both normal and array) - err = services.SubmitJob(&job, arraySize) + err = submitJob(&job, arraySize) if err != nil { logger.Error("Failed to submit job.", zap.Error(err)) ReturnError(ctx, common.ERROR_CODE, common.ERROR_WRITE_DATA_MSG+": "+err.Error(), nil) @@ -1534,6 +1534,17 @@ func preCheckJobRestartRequest(req *models.JobRestartRequest) error { // preCheckJobRequest; a variable so tests can substitute it (bug-1744). var validateResreqSyntax = backend.ValidateResreqSyntax +// Backend seams used by CreateJob, ControlJobByID and the filter-based +// ControlJob; variables (like validateResreqSyntax) so the non-cgo handler +// tests (jobs_handler_test.go) can replace them and observe ID allocation, +// submission and control dispatch. Production code never reassigns them. +var ( + generateJobID = backend.GenerateJobID + generateArrayJobID = backend.GenerateArrayJobID + submitJob = services.SubmitJob + controlJob = backend.ControlJob +) + func preCheckJobRequest(req *models.JobSubmitRequest) ([]int, error) { logger.Debug("Enter preCheckJobRequest") if req.QueueName == "" { @@ -2608,7 +2619,7 @@ func (j JobController) ControlJobByID(ctx *gin.Context) { return } - err = backend.ControlJob(uint64(successJob["unique_job_id"].(int64)), acttype, string(jobCtrlEvtBytes)) + err = controlJob(uint64(successJob["unique_job_id"].(int64)), acttype, string(jobCtrlEvtBytes)) if err != nil { logger.Error("send job control to backend error", zap.Error(err)) ReturnError(ctx, common.ERROR_CODE, common.ERROR_WRITE_DATA_MSG+": "+err.Error(), nil) @@ -2844,7 +2855,7 @@ func (j JobController) ControlJob(ctx *gin.Context) { ReturnError(ctx, common.ERROR_CODE, "encode job control event error:"+": "+err.Error(), nil) return } - err = backend.ControlJob(uint64(successJob["unique_job_id"].(int64)), acttype, string(jobCtrlEvtBytes)) + err = controlJob(uint64(successJob["unique_job_id"].(int64)), acttype, string(jobCtrlEvtBytes)) if err != nil { logger.Error("send job control to backend error", zap.Error(err)) ReturnError(ctx, common.ERROR_CODE, common.ERROR_WRITE_DATA_MSG+": "+err.Error(), nil) diff --git a/src/frontend/apiserver/controllers/jobs_handler_test.go b/src/frontend/apiserver/controllers/jobs_handler_test.go new file mode 100644 index 00000000..c9060bc0 --- /dev/null +++ b/src/frontend/apiserver/controllers/jobs_handler_test.go @@ -0,0 +1,473 @@ +//go:build !cgo + +// Handler-level tests for the job endpoints (CreateJob, GetJobsInfo, +// GetJobByIDs, ControlJobByID and the filter-based ControlJob), driven +// through the test seams in jobs.go (generateJobID, generateArrayJobID, +// submitJob, controlJob) so no C backend is needed. +// +// This file is !cgo because it reuses the JWT/RBAC fixtures (signTestJWT, +// setupRBACTest) from integration_test.go, which is !cgo-only. +package controllers + +import ( + "apiserver/backend" + "apiserver/models" + "apiserver/store" + "apiserver/utils" + "bytes" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "sync" + "testing" + "time" + + "github.com/gin-gonic/gin" +) + +// handlerTestUser is the user every handler fixture request is signed as. +const handlerTestUser = "handler_user" + +// handlerTestRBAC grants handlerTestUser every permission the job handlers +// check, over the "all" domain. The role is deliberately not cluster_admin: +// the handlers are registered without RequirePermission, and each handler +// does its own job:control / queue:control / queue:submit checks. +const handlerTestRBAC = ` +permissions: + - job:view + - job:control + - queue:submit + - queue:control + - queue:view +roles: + handler_tester: + permissions: + - action: job:view + domain: + - type: all + - action: job:control + domain: + - type: all + - action: queue:submit + domain: + - type: all + - action: queue:control + domain: + - type: all + - action: queue:view + domain: + - type: all +users: + handler_user: + roles: + - name: handler_tester + domain: + - type: all +` + +// handlerTestOrdinaryBase is the first ID the generateJobID replacement +// hands out: tag 0, a non-zero timestamp field, index bits clear. +const handlerTestOrdinaryBase = uint64(1)<<40 | uint64(1)<<20 + +// handlerTestArrayBase is the first array key the generateArrayJobID +// replacement hands out (array-capable: tag 0, low 15 bits clear). +const handlerTestArrayBase = uint64(2)<<40 | uint64(1)<<20 + +// submittedJob is one call the submitJob replacement saw. +type submittedJob struct { + Job models.JobInfo + ArraySize int +} + +// dispatchedControl is one call the controlJob replacement saw. +type dispatchedControl struct { + UniqueJobID uint64 + Action int + Content string +} + +// handlerFixture is the shared fixture of the job handler tests: a gin +// router with the job handlers, a signed token, seeded stores and recording +// replacements of the backend seams. +type handlerFixture struct { + t *testing.T + router *gin.Engine + token string + + mu sync.Mutex + // generateCalls records the arraySize argument of every generateJobID call. + generateCalls []int + // arrayCalls records the n argument of every generateArrayJobID call. + arrayCalls []int + submitted []submittedJob + dispatched []dispatchedControl + nextID uint64 + nextArray uint64 + + // Errors the replacements return (nil = success). + generateErr error + submitErr error + controlErr error +} + +// newHandlerFixture builds the fixture. Order matters: setupRBACTest calls +// resetIntegrationState -> store.InitStores, which replaces every store, so +// the original store pointers are saved first, RBAC is set up next, and +// seeding (seedJob/seedQueue) happens afterwards. Everything replaced here is +// restored in t.Cleanup. +func newHandlerFixture(t *testing.T) *handlerFixture { + t.Helper() + + // Save every global this fixture (or setupRBACTest) replaces. + origJobs, origArrayJobs, origQueues, origHosts := store.Jobs, store.ArrayJobs, store.Queues, store.Hosts + origResources, origResLocs, origClusters := store.Resources, store.ResourceLocations, store.Clusters + origExecHosts, origRoles, origUserRoles := store.JobExecHosts, store.Roles, store.UserRoles + origGenerate, origGenerateArray := generateJobID, generateArrayJobID + origSubmit, origControl := submitJob, controlJob + origJobCfg, origLog, origAuthCfg := utils.JobYamlConfig, utils.Log, utils.AuthenticationYamlConfig + origRBAC := rbacConfig + + t.Cleanup(func() { + store.Jobs, store.ArrayJobs, store.Queues, store.Hosts = origJobs, origArrayJobs, origQueues, origHosts + store.Resources, store.ResourceLocations, store.Clusters = origResources, origResLocs, origClusters + store.JobExecHosts, store.Roles, store.UserRoles = origExecHosts, origRoles, origUserRoles + generateJobID, generateArrayJobID = origGenerate, origGenerateArray + submitJob, controlJob = origSubmit, origControl + utils.JobYamlConfig, utils.Log, utils.AuthenticationYamlConfig = origJobCfg, origLog, origAuthCfg + rbacConfig = origRBAC + }) + + // Auth/RBAC (replaces the stores with empty ones), then config. + setupRBACTest(t, handlerTestRBAC) + utils.JobYamlConfig = &utils.JobConfig{MaxArraySize: 1000} + + f := &handlerFixture{ + t: t, + token: signTestJWT(handlerTestUser, "handler_tester", time.Now().Add(time.Hour)), + nextID: handlerTestOrdinaryBase, + nextArray: handlerTestArrayBase, + } + + generateJobID = func(_, _ int, arraySize int) (uint64, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.generateCalls = append(f.generateCalls, arraySize) + if f.generateErr != nil { + return 0, f.generateErr + } + id := f.nextID + f.nextID++ + return id, nil + } + generateArrayJobID = func(_, _ int, n int) (uint64, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.arrayCalls = append(f.arrayCalls, n) + if f.generateErr != nil { + return 0, f.generateErr + } + key := f.nextArray + f.nextArray += uint64(1) << 15 // next array-capable key + return backend.MakeElementID(key, 1), nil + } + submitJob = func(job *models.JobInfo, arraySize int) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.submitErr != nil { + return f.submitErr + } + f.submitted = append(f.submitted, submittedJob{Job: *job, ArraySize: arraySize}) + return nil + } + controlJob = func(jobid uint64, action int, content string) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.controlErr != nil { + return f.controlErr + } + f.dispatched = append(f.dispatched, dispatchedControl{UniqueJobID: jobid, Action: action, Content: content}) + return nil + } + + jc := JobController{} + r := newTestRouter() + g := r.Group("/xce/v1/jobs") + g.GET("", jc.GetJobsInfo) + g.GET("/:jobids", jc.GetJobByIDs) + g.POST("/create", jc.CreateJob) + g.POST("/:jobids/control", jc.ControlJobByID) + g.POST("/control", jc.ControlJob) + f.router = r + return f +} + +// seedQueue adds an open queue to store.Queues (idempotent). +func (f *handlerFixture) seedQueue(name string) { + f.t.Helper() + if !store.Queues.Has(name) { + store.Queues.Put(name, models.QueueInfo{QueueName: name}) + } +} + +// seedJob stores job under its Unique_JobID and seeds its queue, so the +// handlers' queue:control check (store.GetAllQueueNames) admits it. An empty +// QueueName defaults to "normal" and an empty SubmitUser to handlerTestUser. +func (f *handlerFixture) seedJob(job models.JobInfo) models.JobInfo { + f.t.Helper() + if job.QueueName == "" { + job.QueueName = "normal" + } + if job.SubmitUser == "" { + job.SubmitUser = handlerTestUser + } + f.seedQueue(job.QueueName) + store.Jobs.Put(uint64(job.Unique_JobID), job) + return job +} + +// seedOrdinaryJob seeds an ordinary (non-array) job with the given status +// name (a models.JOBSTAT key, e.g. "RUN"). +func (f *handlerFixture) seedOrdinaryJob(id uint64, name, status string) models.JobInfo { + f.t.Helper() + st, ok := models.JOBSTAT[status] + if !ok { + f.t.Fatalf("seedOrdinaryJob: unknown status %q", status) + } + return f.seedJob(models.JobInfo{ + Unique_JobID: int64(id), + JobID: int64(id), + ArrayIdx: 0, + JobName: name, + JobStatus: st, + }) +} + +// seedArrayElements seeds elements idx... of the array identified by key +// (an array-capable key), all with the given name and status. +func (f *handlerFixture) seedArrayElements(key uint64, name, status string, idx ...int) []models.JobInfo { + f.t.Helper() + if !backend.IsArrayCapableKey(key) { + f.t.Fatalf("seedArrayElements: %#x is not an array-capable key", key) + } + st, ok := models.JOBSTAT[status] + if !ok { + f.t.Fatalf("seedArrayElements: unknown status %q", status) + } + out := make([]models.JobInfo, 0, len(idx)) + for _, k := range idx { + elem := backend.MakeElementID(key, k) + out = append(out, f.seedJob(models.JobInfo{ + Unique_JobID: int64(elem), + JobID: int64(backend.ExtractArrayBaseID(elem)), + ArrayIdx: backend.ExtractArrayIdx(elem), + JobName: name, + JobStatus: st, + })) + } + return out +} + +// do sends a request with an optional JSON body, signed with the fixture +// token, and returns the recorded response. +func (f *handlerFixture) do(method, path string, body interface{}) *httptest.ResponseRecorder { + f.t.Helper() + var buf bytes.Buffer + if body != nil { + if err := json.NewEncoder(&buf).Encode(body); err != nil { + f.t.Fatalf("encode request body: %v", err) + } + } + req := httptest.NewRequest(method, path, &buf) + req.RemoteAddr = "127.0.0.1:0" + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+f.token) + w := httptest.NewRecorder() + f.router.ServeHTTP(w, req) + return w +} + +// submit posts a submit body to CreateJob. The body is a JSON object as a +// client sends it (models.JobSubmitRequest is not used directly: its zero +// CustomTime fields do not marshal into a form the strict binder accepts). +func (f *handlerFixture) submit(req map[string]interface{}) *httptest.ResponseRecorder { + f.t.Helper() + return f.do(http.MethodPost, "/xce/v1/jobs/create", req) +} + +// controlByID posts a control action to ControlJobByID for jobids (the +// path segment as written, e.g. "123" or "123[1-3]"). +func (f *handlerFixture) controlByID(jobids, action string) *httptest.ResponseRecorder { + f.t.Helper() + return f.do(http.MethodPost, "/xce/v1/jobs/"+jobids+"/control", + models.JobControlJobIDRequest{Action: action}) +} + +// controlByFilter posts a control request to the filter-based ControlJob. +func (f *handlerFixture) controlByFilter(req models.JobControlRequest) *httptest.ResponseRecorder { + f.t.Helper() + return f.do(http.MethodPost, "/xce/v1/jobs/control", req) +} + +// dispatchedIDs returns the unique_job_id of every controlJob call, in order. +func (f *handlerFixture) dispatchedIDs() []uint64 { + f.mu.Lock() + defer f.mu.Unlock() + ids := make([]uint64, 0, len(f.dispatched)) + for _, d := range f.dispatched { + ids = append(ids, d.UniqueJobID) + } + return ids +} + +// submissions returns a copy of every submitJob call. +func (f *handlerFixture) submissions() []submittedJob { + f.mu.Lock() + defer f.mu.Unlock() + return append([]submittedJob(nil), f.submitted...) +} + +// jobIDString renders a job ID as the decimal path segment the REST API takes. +func jobIDString(id uint64) string { return strconv.FormatUint(id, 10) } + +// handlerResponse is the envelope ReturnSuccess/ReturnError write. +type handlerResponse struct { + Code int `json:"code"` + Msg interface{} `json:"msg"` + Data json.RawMessage `json:"data"` + Details json.RawMessage `json:"details"` +} + +// decodeHandlerResponse decodes the response envelope. +func decodeHandlerResponse(t *testing.T, w *httptest.ResponseRecorder) handlerResponse { + t.Helper() + var r handlerResponse + if err := json.Unmarshal(w.Body.Bytes(), &r); err != nil { + t.Fatalf("decode response: %v; body: %s", err, w.Body.String()) + } + return r +} + +// ========== Smoke tests (task 5.4) ========== + +// TestJobHandler_SmokeSubmitOrdinaryJob submits an ordinary job through the +// generateJobID/submitJob seams. +func TestJobHandler_SmokeSubmitOrdinaryJob(t *testing.T) { + f := newHandlerFixture(t) + f.seedQueue("normal") + + w := f.submit(map[string]interface{}{"job_name": "smoke", "command": "sleep 1", "queue_name": "normal"}) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + resp := decodeHandlerResponse(t, w) + if resp.Code != 201 { + t.Fatalf("code = %d, want 201; body: %s", resp.Code, w.Body.String()) + } + var data models.JobSubmitResponse + if err := json.Unmarshal(resp.Data, &data); err != nil { + t.Fatalf("decode data: %v", err) + } + if uint64(data.JobID) != handlerTestOrdinaryBase { + t.Errorf("jobid = %d, want %d", data.JobID, handlerTestOrdinaryBase) + } + + if len(f.generateCalls) != 1 || f.generateCalls[0] != 1 { + t.Errorf("generateJobID calls = %v, want [1]", f.generateCalls) + } + if len(f.arrayCalls) != 0 { + t.Errorf("generateArrayJobID calls = %v, want none", f.arrayCalls) + } + subs := f.submissions() + if len(subs) != 1 { + t.Fatalf("submitJob calls = %d, want 1", len(subs)) + } + if got := uint64(subs[0].Job.Unique_JobID); got != handlerTestOrdinaryBase { + t.Errorf("submitted unique_job_id = %d, want %d", got, handlerTestOrdinaryBase) + } + if subs[0].ArraySize != 1 || subs[0].Job.JobName != "smoke" || subs[0].Job.ArrayIdx != 0 { + t.Errorf("submitted = {size %d, name %q, idx %d}, want {1, smoke, 0}", + subs[0].ArraySize, subs[0].Job.JobName, subs[0].Job.ArrayIdx) + } +} + +// TestJobHandler_SmokeSubmitErrorIs500 checks a submitJob failure surfaces +// as a 500 and nothing is recorded. +func TestJobHandler_SmokeSubmitErrorIs500(t *testing.T) { + f := newHandlerFixture(t) + f.submitErr = errors.New("backend down") + + w := f.submit(map[string]interface{}{"job_name": "smoke", "command": "sleep 1"}) + if w.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500; body: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "backend down") { + t.Errorf("body does not carry the submit error: %s", w.Body.String()) + } + if len(f.submissions()) != 0 { + t.Errorf("submissions recorded despite error: %v", f.submissions()) + } +} + +// TestJobHandler_SmokeControlByID controls one seeded ordinary job by ID +// and checks the dispatch is recorded once with its unique_job_id. +func TestJobHandler_SmokeControlByID(t *testing.T) { + f := newHandlerFixture(t) + const id = uint64(1)<<40 | 7 + f.seedOrdinaryJob(id, "victim", "RUN") + f.seedOrdinaryJob(id+1, "bystander", "RUN") + + w := f.controlByID(jobIDString(id), "kill") + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + if resp := decodeHandlerResponse(t, w); resp.Code != 202 { + t.Fatalf("code = %d, want 202; body: %s", resp.Code, w.Body.String()) + } + got := f.dispatchedIDs() + if len(got) != 1 || got[0] != id { + t.Fatalf("dispatched = %v, want [%d]", got, id) + } + if f.dispatched[0].Action != models.JOB_KILL { + t.Errorf("dispatched action = %d, want JOB_KILL (%d)", f.dispatched[0].Action, models.JOB_KILL) + } +} + +// TestJobHandler_SmokeControlByFilter controls one seeded job through the +// filter-based ControlJob and checks the dispatch is recorded. +func TestJobHandler_SmokeControlByFilter(t *testing.T) { + f := newHandlerFixture(t) + const id = uint64(1)<<40 | 9 + f.seedOrdinaryJob(id, "filtered", "RUN") + f.seedOrdinaryJob(id+1, "other", "RUN") + + w := f.controlByFilter(models.JobControlRequest{ + Filter: models.JobControlFilter{JobName: "filtered"}, + Action: "kill", + }) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + got := f.dispatchedIDs() + if len(got) != 1 || got[0] != id { + t.Fatalf("dispatched = %v, want [%d]", got, id) + } +} + +// TestJobHandler_SmokeControlErrorIs500 checks a controlJob failure is a 500. +func TestJobHandler_SmokeControlErrorIs500(t *testing.T) { + f := newHandlerFixture(t) + const id = uint64(1)<<40 | 11 + f.seedOrdinaryJob(id, "victim", "RUN") + f.controlErr = errors.New("backend down") + + w := f.controlByID(jobIDString(id), "kill") + if w.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500; body: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "backend down") { + t.Errorf("body does not carry the control error: %s", w.Body.String()) + } +}