From 46bb23ea9134aef1ff1ba93edd0d71c713ac7cc8 Mon Sep 17 00:00:00 2001 From: "lei.chen" Date: Mon, 19 Oct 2026 07:06:48 +0000 Subject: [PATCH] fix(schedulerclient): ListJobs sort + all-users filters (advisor P0 findings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two live-verification defects in the jobs-list path the advisor depends on: 1. The client never passed "sort", so the apiserver's default unique_job_id:asc + limit returned the OLDEST jobs first — every recent PEND job was invisible to any limited list. JobFilter.Sort now forwards to the sort query param. 2. The apiserver's job list handler appends submit_user:eq: when no submit_user filter is given (controllers/jobs.go), so even a scope-all service key saw only its own jobs (0 rows on the live cluster). JobFilter.AllUsers emits filter={submit_user:eq:all} to get the cluster-wide view; an explicit Owner still wins. Co-Authored-By: Claude Code --- src/frontend/schedulerclient/httpclient.go | 9 ++- .../schedulerclient/httpclient_test.go | 56 +++++++++++++++++++ src/frontend/schedulerclient/types.go | 9 +++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/frontend/schedulerclient/httpclient.go b/src/frontend/schedulerclient/httpclient.go index 7d7c3836..c7460092 100644 --- a/src/frontend/schedulerclient/httpclient.go +++ b/src/frontend/schedulerclient/httpclient.go @@ -48,13 +48,17 @@ func (hc *HTTPClient) ListJobs(ctx context.Context, filter JobFilter) ([]*URMJob // The apiserver's job list accepts filters via the "filter" query param in // "field:op:value" form (e.g. queue_name:eq:normal); the individual // queue_name/submit_user params it ignores (400). Multiple conditions are - // brace-wrapped: {a},{b}. + // brace-wrapped: {a},{b}. Without an explicit submit_user condition the + // handler appends submit_user:eq: (jobs.go), so a cluster-wide + // view needs AllUsers → submit_user:eq:all. var conds []string if filter.Queue != "" { conds = append(conds, "queue_name:eq:"+filter.Queue) } if filter.Owner != "" { conds = append(conds, "submit_user:eq:"+filter.Owner) + } else if filter.AllUsers { + conds = append(conds, "submit_user:eq:all") } if len(conds) > 0 { req.SetQueryParam("filter", "{"+strings.Join(conds, "},{")+"}") @@ -65,6 +69,9 @@ func (hc *HTTPClient) ListJobs(ctx context.Context, filter JobFilter) ([]*URMJob if filter.PageToken != "" { req.SetQueryParam("page_token", filter.PageToken) } + if filter.Sort != "" { + req.SetQueryParam("sort", filter.Sort) + } // Note: status filter on URM statuses is applied in post-processing // because the apiserver works with raw status strings, not URM statuses. diff --git a/src/frontend/schedulerclient/httpclient_test.go b/src/frontend/schedulerclient/httpclient_test.go index 6fe863ed..0c25456a 100644 --- a/src/frontend/schedulerclient/httpclient_test.go +++ b/src/frontend/schedulerclient/httpclient_test.go @@ -49,3 +49,59 @@ func TestListNodeMemoryBytesKeepsByteUnit(t *testing.T) { t.Fatalf("MemoryBytesTotal = %d, want %d (apiserver max_mem is already bytes; must not be ×1e6)", n.MemoryBytesTotal, realMemBytes) } } + +// TestListJobsAllUsersSortLimit pins the three query params the advisor's +// cluster-wide view depends on (P0 verification findings): +// - AllUsers must emit filter={submit_user:eq:all} — without an explicit +// submit_user condition the apiserver handler appends +// submit_user:eq:, so even a scope-all key sees only its own jobs. +// - Sort must reach the "sort" query param — the default +// unique_job_id:asc + limit returns the OLDEST jobs first. +// - Limit must reach the "limit" query param. +func TestListJobsAllUsersSortLimit(t *testing.T) { + var gotFilter, gotSort, gotLimit string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/xce/v1/jobs" { + t.Errorf("unexpected path %q", r.URL.Path) + } + gotFilter = r.URL.Query().Get("filter") + gotSort = r.URL.Query().Get("sort") + gotLimit = r.URL.Query().Get("limit") + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"code":200,"msg":"ok","data":{"count":0,"rows":[],"page_token":""}}`)) + })) + defer srv.Close() + + hc := NewClient(srv.URL, "xce_ak_test") + jobs, _, err := hc.ListJobs(context.Background(), JobFilter{ + Statuses: []JobStatus{JobStatusPending}, + AllUsers: true, + Sort: "submit_time:desc", + Limit: 200, + }) + if err != nil { + t.Fatalf("ListJobs: %v", err) + } + if len(jobs) != 0 { + t.Fatalf("expected 0 jobs, got %d", len(jobs)) + } + if gotFilter != "{submit_user:eq:all}" { + t.Errorf("filter = %q, want {submit_user:eq:all}", gotFilter) + } + if gotSort != "submit_time:desc" { + t.Errorf("sort = %q, want submit_time:desc (default unique_job_id:asc hides recent jobs)", gotSort) + } + if gotLimit != "200" { + t.Errorf("limit = %q, want 200", gotLimit) + } + + // Owner still wins over AllUsers: an explicit owner filter must not be + // doubled by the all-users one. + _, _, err = hc.ListJobs(context.Background(), JobFilter{Owner: "alice", AllUsers: true}) + if err != nil { + t.Fatalf("ListJobs owner: %v", err) + } + if gotFilter != "{submit_user:eq:alice}" { + t.Errorf("filter with Owner+AllUsers = %q, want {submit_user:eq:alice}", gotFilter) + } +} diff --git a/src/frontend/schedulerclient/types.go b/src/frontend/schedulerclient/types.go index 7923fc9d..b91a93c5 100644 --- a/src/frontend/schedulerclient/types.go +++ b/src/frontend/schedulerclient/types.go @@ -78,6 +78,15 @@ type JobFilter struct { Owner string // filter by owner username Limit int // max results (0 = default) PageToken string // pagination cursor from a prior response + // Sort is the apiserver sort spec, e.g. "submit_time:desc". Empty → the + // apiserver's default, unique_job_id:asc — which with a limit returns + // the OLDEST jobs first (a list that hides every recent job). + Sort string + // AllUsers widens the owner filter: the apiserver appends + // submit_user:eq: to every list unless the caller passes an + // explicit submit_user filter, so even a scope-all credential sees only + // its own jobs without this (bug found by the advisor P0 verification). + AllUsers bool } // LogOpts specifies how to retrieve job logs.