Skip to content

Commit

Permalink
fix tests that fail after catching FSM.Apply errors
Browse files Browse the repository at this point in the history
`Job.Deregister` should not emit evals once the job has been purged and should
return an error if the job doesn't exist.
  • Loading branch information
tgross committed Mar 1, 2023
1 parent 709c491 commit 93ef921
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 49 deletions.
2 changes: 1 addition & 1 deletion api/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1556,7 +1556,7 @@ func TestJobs_Deregister(t *testing.T) {

// Attempting delete on non-existing job returns an error
_, _, err = jobs.Deregister("nope", false, nil)
must.NoError(t, err)
must.Error(t, err)

// Do a soft deregister of an existing job
evalID, wm3, err := jobs.Deregister("job1", false, nil)
Expand Down
1 change: 1 addition & 0 deletions api/nodes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
)

func queryNodeList(t *testing.T, nodes *Nodes) ([]*NodeListStub, *QueryMeta) {
t.Helper()
var (
nodeListStub []*NodeListStub
queryMeta *QueryMeta
Expand Down
7 changes: 5 additions & 2 deletions nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,9 @@ func (j *Job) Deregister(args *structs.JobDeregisterRequest, reply *structs.JobD
if err != nil {
return err
}
if job == nil {
return nil
}

var eval *structs.Evaluation

Expand All @@ -854,7 +857,7 @@ func (j *Job) Deregister(args *structs.JobDeregisterRequest, reply *structs.JobD
now := time.Now().UnixNano()

// If the job is periodic or parameterized, we don't create an eval.
if job == nil || !(job.IsPeriodic() || job.IsParameterized()) {
if !(job.IsPeriodic() || job.IsParameterized()) {

// The evaluation priority is determined by several factors. It
// defaults to the job default priority and is overridden by the
Expand All @@ -863,7 +866,7 @@ func (j *Job) Deregister(args *structs.JobDeregisterRequest, reply *structs.JobD
// If the user supplied an eval priority override, we subsequently
// use this.
priority := structs.JobDefaultPriority
if job != nil {
if job.Priority > 0 {
priority = job.Priority
}
if args.EvalPriority > 0 {
Expand Down
54 changes: 8 additions & 46 deletions nomad/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3587,13 +3587,11 @@ func TestJobEndpoint_Deregister_ACL(t *testing.T) {
require.NotZero(eval.CreateTime)
require.NotZero(eval.ModifyTime)

// Deregistration is not idempotent, produces a new eval after the job is
// deregistered. TODO(langmartin) make it idempotent.
// Deregistration is idempotent
var validResp2 structs.JobDeregisterResponse
err = msgpackrpc.CallWithCodec(codec, "Job.Deregister", req, &validResp2)
require.NoError(err)
require.NotEqual("", validResp2.EvalID)
require.NotEqual(validResp.EvalID, validResp2.EvalID)
must.NoError(t, err)
must.Eq(t, "", validResp2.EvalID)
}

func TestJobEndpoint_Deregister_Nonexistent(t *testing.T) {
Expand All @@ -3616,51 +3614,15 @@ func TestJobEndpoint_Deregister_Nonexistent(t *testing.T) {
},
}
var resp2 structs.JobDeregisterResponse
if err := msgpackrpc.CallWithCodec(codec, "Job.Deregister", dereg, &resp2); err != nil {
t.Fatalf("err: %v", err)
}
if resp2.JobModifyIndex == 0 {
t.Fatalf("bad index: %d", resp2.Index)
}
must.NoError(t, msgpackrpc.CallWithCodec(codec, "Job.Deregister", dereg, &resp2))
must.Eq(t, 0, resp2.JobModifyIndex, must.Sprint("expected no modify index"))

// Lookup the evaluation
state := s1.fsm.State()
ws := memdb.NewWatchSet()
eval, err := state.EvalByID(ws, resp2.EvalID)
if err != nil {
t.Fatalf("err: %v", err)
}
if eval == nil {
t.Fatalf("expected eval")
}
if eval.CreateIndex != resp2.EvalCreateIndex {
t.Fatalf("index mis-match")
}

if eval.Priority != structs.JobDefaultPriority {
t.Fatalf("bad: %#v", eval)
}
if eval.Type != structs.JobTypeService {
t.Fatalf("bad: %#v", eval)
}
if eval.TriggeredBy != structs.EvalTriggerJobDeregister {
t.Fatalf("bad: %#v", eval)
}
if eval.JobID != jobID {
t.Fatalf("bad: %#v", eval)
}
if eval.JobModifyIndex != resp2.JobModifyIndex {
t.Fatalf("bad: %#v", eval)
}
if eval.Status != structs.EvalStatusPending {
t.Fatalf("bad: %#v", eval)
}
if eval.CreateTime == 0 {
t.Fatalf("eval CreateTime is unset: %#v", eval)
}
if eval.ModifyTime == 0 {
t.Fatalf("eval ModifyTime is unset: %#v", eval)
}
eval, err := state.EvalsByJob(ws, structs.DefaultNamespace, jobID)
must.NoError(t, err)
must.Nil(t, eval)
}

func TestJobEndpoint_Deregister_EvalPriority(t *testing.T) {
Expand Down

0 comments on commit 93ef921

Please sign in to comment.