Skip to content

Commit

Permalink
Add table tests. Clean up UpdateAlloc code.
Browse files Browse the repository at this point in the history
  • Loading branch information
DerekStrickland committed Apr 4, 2022
1 parent 221919d commit e123452
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 58 deletions.
51 changes: 23 additions & 28 deletions nomad/node_endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1144,41 +1144,54 @@ func (n *Node) UpdateAlloc(args *structs.AllocUpdateRequest, reply *structs.Gene

// if the job has been purged, this will always return error
var job *structs.Job
var jobType string
var jobPriority int

job, err = n.srv.State().JobByID(nil, alloc.Namespace, alloc.JobID)
if err != nil {
n.logger.Debug("UpdateAlloc unable to find job", "job", alloc.JobID, "error", err)
}

// If the job is nil it means it has been de-registered.
if job == nil {
jobType = alloc.Job.Type
jobPriority = alloc.Job.Priority
evalTriggerBy = structs.EvalTriggerJobDeregister
if err == nil {
n.logger.Debug("UpdateAlloc unable to find job", "job", alloc.JobID)
}
allocToUpdate.DesiredStatus = structs.AllocDesiredStatusStop
allocToUpdate.DesiredTransition.NoShutdownDelay = helper.BoolToPtr(true)
n.logger.Debug("UpdateAlloc unable to find job - shutting down alloc", "job", alloc.JobID)
}

var taskGroup *structs.TaskGroup
if job != nil {
jobType = job.Type
jobPriority = job.Priority
taskGroup = job.LookupTaskGroup(alloc.TaskGroup)
}

if taskGroup == nil {
// If we cannot find the task group for a failed alloc we cannot continue, unless it is an orphan.
if evalTriggerBy != structs.EvalTriggerJobDeregister &&
allocToUpdate.ClientStatus == structs.AllocClientStatusFailed &&
alloc.FollowupEvalID == "" &&
taskGroup == nil {
n.logger.Debug("UpdateAlloc unable to find task group for job", "job", alloc.JobID, "alloc", alloc.ID, "task_group", alloc.TaskGroup)
continue
}

var eval *structs.Evaluation
// Add an evaluation if this is a failed alloc that is eligible for rescheduling
if allocToUpdate.ClientStatus == structs.AllocClientStatusFailed &&
// Add an evaluation if this is a failed alloc that is eligible for rescheduling,
// unless it is an orphan.
if evalTriggerBy != structs.EvalTriggerJobDeregister &&
allocToUpdate.ClientStatus == structs.AllocClientStatusFailed &&
alloc.FollowupEvalID == "" &&
taskGroup != nil &&
alloc.RescheduleEligible(taskGroup.ReschedulePolicy, now) {

evalTriggerBy = structs.EvalTriggerRetryFailedAlloc
}

// Add an evaluation if this is a reconnecting allocation, and sync the
// DesiredTransition in case the job was purged or force rescheduled.
if alloc.ClientStatus == structs.AllocClientStatusUnknown {
// If unknown, and not an orphan, set the trigger by.
if evalTriggerBy != structs.EvalTriggerJobDeregister &&
alloc.ClientStatus == structs.AllocClientStatusUnknown {
evalTriggerBy = structs.EvalTriggerReconnect
}

Expand All @@ -1188,24 +1201,6 @@ func (n *Node) UpdateAlloc(args *structs.AllocUpdateRequest, reply *structs.Gene
continue
}

// If we are cleaning up an orphaned alloc, update it
// so that the eval will send a shutdown to the client.
if evalTriggerBy == structs.EvalTriggerJobDeregister {
allocToUpdate.DesiredStatus = structs.AllocDesiredStatusStop
allocToUpdate.DesiredTransition.NoShutdownDelay = helper.BoolToPtr(true)
}

var jobType string
var jobPriority int

if job == nil {
jobType = alloc.Job.Type
jobPriority = alloc.Job.Priority
} else {
jobType = job.Type
jobPriority = job.Priority
}

eval = &structs.Evaluation{
ID: uuid.Generate(),
Namespace: alloc.Namespace,
Expand Down
63 changes: 33 additions & 30 deletions nomad/node_endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3723,7 +3723,7 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
clientStatus string
serverClientStatus string
triggerBy string
deleteJob bool
missingJob bool
missingAlloc bool
invalidTaskGroup bool
}
Expand All @@ -3734,7 +3734,7 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
clientStatus: structs.AllocClientStatusFailed,
serverClientStatus: structs.AllocClientStatusRunning,
triggerBy: structs.EvalTriggerRetryFailedAlloc,
deleteJob: false,
missingJob: false,
missingAlloc: false,
invalidTaskGroup: false,
},
Expand All @@ -3743,16 +3743,16 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
clientStatus: structs.AllocClientStatusRunning,
serverClientStatus: structs.AllocClientStatusUnknown,
triggerBy: structs.EvalTriggerReconnect,
deleteJob: false,
missingJob: false,
missingAlloc: false,
invalidTaskGroup: false,
},
{
name: "orphaned-job",
name: "orphaned-unknown-alloc",
clientStatus: structs.AllocClientStatusRunning,
serverClientStatus: structs.AllocClientStatusRunning,
serverClientStatus: structs.AllocClientStatusUnknown,
triggerBy: structs.EvalTriggerJobDeregister,
deleteJob: true,
missingJob: true,
missingAlloc: false,
invalidTaskGroup: false,
},
Expand All @@ -3761,7 +3761,7 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
clientStatus: structs.AllocClientStatusRunning,
serverClientStatus: structs.AllocClientStatusRunning,
triggerBy: "",
deleteJob: false,
missingJob: false,
missingAlloc: false,
invalidTaskGroup: false,
},
Expand All @@ -3770,7 +3770,7 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
clientStatus: structs.AllocClientStatusComplete,
serverClientStatus: structs.AllocClientStatusComplete,
triggerBy: "",
deleteJob: false,
missingJob: false,
missingAlloc: false,
invalidTaskGroup: false,
},
Expand All @@ -3779,7 +3779,7 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
clientStatus: structs.AllocClientStatusUnknown,
serverClientStatus: "",
triggerBy: "",
deleteJob: false,
missingJob: false,
missingAlloc: true,
invalidTaskGroup: false,
},
Expand All @@ -3788,7 +3788,7 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
clientStatus: structs.AllocClientStatusUnknown,
serverClientStatus: structs.AllocClientStatusRunning,
triggerBy: "",
deleteJob: false,
missingJob: false,
missingAlloc: false,
invalidTaskGroup: true,
},
Expand Down Expand Up @@ -3824,8 +3824,12 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {

job := mock.Job()
job.ID = tc.name + "-test-job"
err := fsmState.UpsertJob(structs.MsgTypeTestSetup, 101, job)
require.NoError(t, err)

var err error
if !tc.missingJob {
err = fsmState.UpsertJob(structs.MsgTypeTestSetup, 101, job)
require.NoError(t, err)
}

serverAlloc := mock.Alloc()
serverAlloc.JobID = job.ID
Expand All @@ -3849,18 +3853,18 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
require.NoError(t, err)
}

if tc.deleteJob {
deregisterReq := &structs.JobDeregisterRequest{
JobID: job.ID,
Purge: true,
WriteRequest: structs.WriteRequest{
Region: "global",
},
}
deregisterResp := &structs.JobDeregisterResponse{}
err = msgpackrpc.CallWithCodec(codec, "Job.Deregister", deregisterReq, deregisterResp)
require.NoError(t, err)
}
//if tc.deleteJob {
// deregisterReq := &structs.JobDeregisterRequest{
// JobID: job.ID,
// Purge: true,
// WriteRequest: structs.WriteRequest{
// Region: "global",
// },
// }
// deregisterResp := &structs.JobDeregisterResponse{}
// err = msgpackrpc.CallWithCodec(codec, "Job.Deregister", deregisterReq, deregisterResp)
// require.NoError(t, err)
//}

updateReq := &structs.AllocUpdateRequest{
Alloc: []*structs.Allocation{clientAlloc},
Expand All @@ -3877,8 +3881,11 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
t.Fatalf("too fast: %v", diff)
}

// Skip remaining validation if no alloc at the server.
if tc.missingAlloc {
// If no eval should be created validate, none were and return.
if tc.triggerBy == "" {
evaluations, err := fsmState.EvalsByJob(nil, job.Namespace, job.ID)
require.NoError(t, err)
require.True(t, len(evaluations) == 0)
return
}

Expand All @@ -3887,10 +3894,6 @@ func TestClientEndpoint_UpdateAlloc_Evals_ByTrigger(t *testing.T) {
require.NoError(t, err)
require.Equal(t, tc.clientStatus, updatedAlloc.ClientStatus)

if tc.triggerBy == "" {
return
}

// Assert that exactly one eval with test case TriggeredBy exists
evaluations, err := fsmState.EvalsByJob(nil, job.Namespace, job.ID)
require.NoError(t, err)
Expand Down

0 comments on commit e123452

Please sign in to comment.