From cf24a9eaaf7bcf0aace0aafef8b44e1401c01330 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 23 Apr 2021 16:36:54 -0400 Subject: [PATCH] api: /v1/jobs always include namespaces (#10434) Add Namespace as a top-level field in `/v1/jobs` stub. The `/v1/jobs` endpoint already includes the namespace under `JobSummary`, though the API is odd, as typically the job ID and Namespace are in the same level, and the oddity complicates the UI frontend development. The downside of adding it is redundant field, that makes the response body a bit bigger, specially for clusters with large jobs. Though, it should compress nicely and I expect the overhead to be small to overall response size. The benefit of a cleaner and more consistent API seem worth it. Fixes #10431 --- nomad/job_endpoint.go | 1 - nomad/job_endpoint_test.go | 42 ++++++++++++-------------------------- nomad/structs/structs.go | 1 + 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 5746c3dd9..714921ce5 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -1427,7 +1427,6 @@ func (j *Job) listAllNamespaces(args *structs.JobListRequest, reply *structs.Job } stub := job.Stub(summary) - stub.Namespace = job.Namespace jobs = append(jobs, stub) } reply.Jobs = jobs diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 944c7f772..ab504c959 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -4662,9 +4662,7 @@ func TestJobEndpoint_ListJobs(t *testing.T) { job := mock.Job() state := s1.fsm.State() err := state.UpsertJob(structs.MsgTypeTestSetup, 1000, job) - if err != nil { - t.Fatalf("err: %v", err) - } + require.NoError(t, err) // Lookup the jobs get := &structs.JobListRequest{ @@ -4674,19 +4672,12 @@ func TestJobEndpoint_ListJobs(t *testing.T) { }, } var resp2 structs.JobListResponse - if err := msgpackrpc.CallWithCodec(codec, "Job.List", get, &resp2); err != nil { - t.Fatalf("err: %v", err) - } - if resp2.Index != 1000 { - t.Fatalf("Bad index: %d %d", resp2.Index, 1000) - } - - if len(resp2.Jobs) != 1 { - t.Fatalf("bad: %#v", resp2.Jobs) - } - if resp2.Jobs[0].ID != job.ID { - t.Fatalf("bad: %#v", resp2.Jobs[0]) - } + err = msgpackrpc.CallWithCodec(codec, "Job.List", get, &resp2) + require.NoError(t, err) + require.Equal(t, uint64(1000), resp2.Index) + require.Len(t, resp2.Jobs, 1) + require.Equal(t, job.ID, resp2.Jobs[0].ID) + require.Equal(t, job.Namespace, resp2.Jobs[0].Namespace) // Lookup the jobs by prefix get = &structs.JobListRequest{ @@ -4697,19 +4688,12 @@ func TestJobEndpoint_ListJobs(t *testing.T) { }, } var resp3 structs.JobListResponse - if err := msgpackrpc.CallWithCodec(codec, "Job.List", get, &resp3); err != nil { - t.Fatalf("err: %v", err) - } - if resp3.Index != 1000 { - t.Fatalf("Bad index: %d %d", resp3.Index, 1000) - } - - if len(resp3.Jobs) != 1 { - t.Fatalf("bad: %#v", resp3.Jobs) - } - if resp3.Jobs[0].ID != job.ID { - t.Fatalf("bad: %#v", resp3.Jobs[0]) - } + err = msgpackrpc.CallWithCodec(codec, "Job.List", get, &resp3) + require.NoError(t, err) + require.Equal(t, uint64(1000), resp3.Index) + require.Len(t, resp3.Jobs, 1) + require.Equal(t, job.ID, resp3.Jobs[0].ID) + require.Equal(t, job.Namespace, resp3.Jobs[0].Namespace) } // TestJobEndpoint_ListJobs_AllNamespaces_OSS asserts that server diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 53c249f8d..637aad4ce 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4334,6 +4334,7 @@ func (j *Job) HasUpdateStrategy() bool { func (j *Job) Stub(summary *JobSummary) *JobListStub { return &JobListStub{ ID: j.ID, + Namespace: j.Namespace, ParentID: j.ParentID, Name: j.Name, Datacenters: j.Datacenters,