Skip to content

Commit

Permalink
api: Ignore User provided ParentID (#10424)
Browse files Browse the repository at this point in the history
ParentID is an internal field that Nomad sets for dispatched or parameterized jobs. Job submitters should not be able to set it directly, as that messes up children tracking.

Fixes #10422 . It specifically stops the scheduler from honoring the ParentID. The reason failure and why the scheduler didn't schedule that job once it was created is very interesting and requires follow up with a more technical issue.
  • Loading branch information
Mahmood Ali committed Apr 23, 2021
1 parent 6f17ad6 commit 982f0ac
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 3 deletions.
1 change: 0 additions & 1 deletion command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,6 @@ func ApiJobToStructJob(job *api.Job) *structs.Job {
Region: *job.Region,
Namespace: *job.Namespace,
ID: *job.ID,
ParentID: *job.ParentID,
Name: *job.Name,
Type: *job.Type,
Priority: *job.Priority,
Expand Down
64 changes: 62 additions & 2 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,68 @@ func TestHTTP_JobsRegister(t *testing.T) {
})
}

func TestHTTP_JobsRegister_IgnoresParentID(t *testing.T) {
t.Parallel()
httpTest(t, nil, func(s *TestAgent) {
// Create the job
job := MockJob()
parentID := "somebadparentid"
job.ParentID = &parentID
args := api.JobRegisterRequest{
Job: job,
WriteRequest: api.WriteRequest{Region: "global"},
}
buf := encodeReq(args)

// Make the HTTP request
req, err := http.NewRequest("PUT", "/v1/jobs", buf)
require.NoError(t, err)
respW := httptest.NewRecorder()

// Make the request
obj, err := s.Server.JobsRequest(respW, req)
require.NoError(t, err)

// Check the response
reg := obj.(structs.JobRegisterResponse)
require.NotEmpty(t, reg.EvalID)

// Check for the index
require.NotEmpty(t, respW.HeaderMap.Get("X-Nomad-Index"))

// Check the job is registered
getReq := structs.JobSpecificRequest{
JobID: *job.ID,
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: structs.DefaultNamespace,
},
}
var getResp structs.SingleJobResponse
err = s.Agent.RPC("Job.GetJob", &getReq, &getResp)
require.NoError(t, err)

require.NotNil(t, getResp.Job)
require.Equal(t, *job.ID, getResp.Job.ID)
require.Empty(t, getResp.Job.ParentID)

// check the eval exists
evalReq := structs.EvalSpecificRequest{
EvalID: reg.EvalID,
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: structs.DefaultNamespace,
},
}
var evalResp structs.SingleEvalResponse
err = s.Agent.RPC("Eval.GetEval", &evalReq, &evalResp)
require.NoError(t, err)

require.NotNil(t, evalResp.Eval)
require.Equal(t, reg.EvalID, evalResp.Eval.ID)
})
}

// Test that ACL token is properly threaded through to the RPC endpoint
func TestHTTP_JobsRegister_ACL(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -2172,7 +2234,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Namespace: "foo",
VaultNamespace: "ghi789",
ID: "foo",
ParentID: "lol",
Name: "name",
Type: "service",
Priority: 50,
Expand Down Expand Up @@ -2673,7 +2734,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Region: "global",
Namespace: "foo",
ID: "foo",
ParentID: "lol",
Name: "name",
Type: "system",
Priority: 50,
Expand Down

0 comments on commit 982f0ac

Please sign in to comment.