From d2a8c5da852d24e9ad436f65d53814d77fc87fd4 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 5746c3dd9204..714921ce537e 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 944c7f7727fb..ab504c959196 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 53c249f8db2e..637aad4ce9fa 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,