From 06135c50af58b6079c58eeaba5479ce7864f054e Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 27 Mar 2017 16:55:17 -0700 Subject: [PATCH] Fix dispatch of periodic job This PR fixes an issue in which when a periodic and parameterized job was dispatched, an allocation would be immediately created. Fixes https://github.com/hashicorp/nomad/issues/2470 --- command/job_dispatch.go | 11 ++++++-- nomad/fsm.go | 2 +- nomad/job_endpoint.go | 56 +++++++++++++++++++++----------------- nomad/job_endpoint_test.go | 29 +++++++++++++++++++- 4 files changed, 69 insertions(+), 29 deletions(-) diff --git a/command/job_dispatch.go b/command/job_dispatch.go index cc9ff597115c..13b180ea9437 100644 --- a/command/job_dispatch.go +++ b/command/job_dispatch.go @@ -125,13 +125,20 @@ func (c *JobDispatchCommand) Run(args []string) int { return 1 } + // See if an evaluation was created. If the job is periodic there will be no + // eval. + evalCreated := resp.EvalID != "" + basic := []string{ fmt.Sprintf("Dispatched Job ID|%s", resp.DispatchedJobID), - fmt.Sprintf("Evaluation ID|%s", limit(resp.EvalID, length)), + } + if evalCreated { + basic = append(basic, fmt.Sprintf("Evaluation ID|%s", limit(resp.EvalID, length))) } c.Ui.Output(formatKV(basic)) - if detach { + // Nothing to do + if detach || !evalCreated { return 0 } diff --git a/nomad/fsm.go b/nomad/fsm.go index 25221ca4f4c4..cb8e1f701d71 100644 --- a/nomad/fsm.go +++ b/nomad/fsm.go @@ -305,7 +305,7 @@ func (n *nomadFSM) applyUpsertJob(buf []byte, index uint64) interface{} { return nil } - if parent.IsPeriodic() { + if parent.IsPeriodic() && !parent.IsParameterized() { t, err := n.periodicDispatcher.LaunchTime(req.Job.ID) if err != nil { n.logger.Printf("[ERR] nomad.fsm: LaunchTime(%v) failed: %v", req.Job.ID, err) diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 780fd0776821..866c7fd00712 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -842,34 +842,40 @@ func (j *Job) Dispatch(args *structs.JobDispatchRequest, reply *structs.JobDispa return err } - // Create a new evaluation - eval := &structs.Evaluation{ - ID: structs.GenerateUUID(), - Priority: dispatchJob.Priority, - Type: dispatchJob.Type, - TriggeredBy: structs.EvalTriggerJobRegister, - JobID: dispatchJob.ID, - JobModifyIndex: jobCreateIndex, - Status: structs.EvalStatusPending, - } - update := &structs.EvalUpdateRequest{ - Evals: []*structs.Evaluation{eval}, - WriteRequest: structs.WriteRequest{Region: args.Region}, - } + reply.JobCreateIndex = jobCreateIndex + reply.DispatchedJobID = dispatchJob.ID + reply.Index = jobCreateIndex + + // If the job is periodic, we don't create an eval. + if !dispatchJob.IsPeriodic() { + // Create a new evaluation + eval := &structs.Evaluation{ + ID: structs.GenerateUUID(), + Priority: dispatchJob.Priority, + Type: dispatchJob.Type, + TriggeredBy: structs.EvalTriggerJobRegister, + JobID: dispatchJob.ID, + JobModifyIndex: jobCreateIndex, + Status: structs.EvalStatusPending, + } + update := &structs.EvalUpdateRequest{ + Evals: []*structs.Evaluation{eval}, + WriteRequest: structs.WriteRequest{Region: args.Region}, + } - // Commit this evaluation via Raft - _, evalIndex, err := j.srv.raftApply(structs.EvalUpdateRequestType, update) - if err != nil { - j.srv.logger.Printf("[ERR] nomad.job: Eval create failed: %v", err) - return err + // Commit this evaluation via Raft + _, evalIndex, err := j.srv.raftApply(structs.EvalUpdateRequestType, update) + if err != nil { + j.srv.logger.Printf("[ERR] nomad.job: Eval create failed: %v", err) + return err + } + + // Setup the reply + reply.EvalID = eval.ID + reply.EvalCreateIndex = evalIndex + reply.Index = evalIndex } - // Setup the reply - reply.EvalID = eval.ID - reply.EvalCreateIndex = evalIndex - reply.JobCreateIndex = jobCreateIndex - reply.DispatchedJobID = dispatchJob.ID - reply.Index = evalIndex return nil } diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 4a9af00e2511..fb92b0efa0f8 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -1938,6 +1938,10 @@ func TestJobEndpoint_Dispatch(t *testing.T) { MetaOptional: []string{"foo", "bar"}, } + // Periodic dispatch job + d6 := mock.PeriodicJob() + d6.ParameterizedJob = &structs.ParameterizedJobConfig{} + reqNoInputNoMeta := &structs.JobDispatchRequest{} reqInputDataNoMeta := &structs.JobDispatchRequest{ Payload: []byte("hello world"), @@ -1971,6 +1975,7 @@ func TestJobEndpoint_Dispatch(t *testing.T) { name string parameterizedJob *structs.Job dispatchReq *structs.JobDispatchRequest + noEval bool err bool errStr string } @@ -2052,6 +2057,12 @@ func TestJobEndpoint_Dispatch(t *testing.T) { err: true, errStr: "Payload exceeds maximum size", }, + { + name: "periodic job dispatched, ensure no eval", + parameterizedJob: d6, + dispatchReq: reqNoInputNoMeta, + noEval: true, + }, } for _, tc := range cases { @@ -2088,7 +2099,18 @@ func TestJobEndpoint_Dispatch(t *testing.T) { } // Check that we got an eval and job id back - if dispatchResp.EvalID == "" || dispatchResp.DispatchedJobID == "" { + switch dispatchResp.EvalID { + case "": + if !tc.noEval { + t.Fatalf("Bad response") + } + default: + if tc.noEval { + t.Fatalf("Got eval %q", dispatchResp.EvalID) + } + } + + if dispatchResp.DispatchedJobID == "" { t.Fatalf("Bad response") } @@ -2108,11 +2130,16 @@ func TestJobEndpoint_Dispatch(t *testing.T) { t.Fatalf("bad parent ID") } + if tc.noEval { + return + } + // Lookup the evaluation eval, err := state.EvalByID(ws, dispatchResp.EvalID) if err != nil { t.Fatalf("err: %v", err) } + if eval == nil { t.Fatalf("expected eval") }