luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Merge pull request #2592 from hashicorp/b-gc-race 

Protect against nil job in new allocation
This commit is contained in:
Alex Dadgar 2017-05-01 13:54:43 -07:00 committed by GitHub
parent 7276e24763 8169590d76
commit aed852782f
6 changed files with 69 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
nomad/plan_apply_test.go
View File

@ -119,7 +119,6 @@ func TestPlanApply_applyPlan(t *testing.T) {
job := allocEvict.Job
allocEvict.Job = nil
alloc2 := mock.Alloc()
alloc2.Job = nil
s1.State().UpsertJobSummary(1500, mock.JobSummary(alloc2.JobID))
plan = &structs.PlanResult{
NodeUpdate: map[string][]*structs.Allocation{

15
nomad/state/state_store.go
View File

@ -1150,6 +1150,21 @@ func (s *StateStore) UpsertAllocs(index uint64, allocs []*structs.Allocation) er
alloc.CreateIndex = index
alloc.ModifyIndex = index
alloc.AllocModifyIndex = index
// Issue https://github.com/hashicorp/nomad/issues/2583 uncovered
// the a race between a forced garbage collection and the scheduler
// marking an allocation as terminal. The issue is that the
// allocation from the scheduler has its job normalized and the FSM
// will only denormalize if the allocation is not terminal. However
// if the allocation is garbage collected, that will result in a
// allocation being upserted for the first time without a job
// attached. By returning an error here, it will cause the FSM to
// error, causing the plan_apply to error and thus causing the
// evaluation to be failed. This will force an index refresh that
// should solve this issue.
if alloc.Job == nil {
return fmt.Errorf("attempting to upsert allocation %q without a job", alloc.ID)
}
} else {
alloc.CreateIndex = exist.CreateIndex
alloc.ModifyIndex = index

14
nomad/state/state_store_test.go
View File

@ -5,6 +5,7 @@ import (
"os"
"reflect"
"sort"
"strings"
"testing"
"time"
@ -2665,6 +2666,19 @@ func TestStateStore_UpsertAlloc_Alloc(t *testing.T) {
}
}
// Testing to ensure we keep issue
// https://github.com/hashicorp/nomad/issues/2583 fixed
func TestStateStore_UpsertAlloc_No_Job(t *testing.T) {
state := testStateStore(t)
alloc := mock.Alloc()
alloc.Job = nil
err := state.UpsertAllocs(999, []*structs.Allocation{alloc})
if err == nil || !strings.Contains(err.Error(), "without a job") {
t.Fatalf("expect err: %v", err)
}
}
func TestStateStore_UpsertAlloc_NoEphemeralDisk(t *testing.T) {
state := testStateStore(t)
alloc := mock.Alloc()

7
scheduler/context_test.go
View File

@ -53,11 +53,13 @@ func TestEvalContext_ProposedAlloc(t *testing.T) {
}
// Add existing allocations
j1, j2 := mock.Job(), mock.Job()
alloc1 := &structs.Allocation{
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[0].Node.ID,
JobID: structs.GenerateUUID(),
JobID: j1.ID,
Job: j1,
Resources: &structs.Resources{
CPU: 2048,
MemoryMB: 2048,
@ -70,7 +72,8 @@ func TestEvalContext_ProposedAlloc(t *testing.T) {
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[1].Node.ID,
JobID: structs.GenerateUUID(),
JobID: j2.ID,
Job: j2,
Resources: &structs.Resources{
CPU: 1024,
MemoryMB: 1024,

25
scheduler/feasible_test.go
View File

@ -463,6 +463,7 @@ func TestDistinctHostsIterator_JobDistinctHosts(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
},
@ -470,6 +471,7 @@ func TestDistinctHostsIterator_JobDistinctHosts(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: "ignore 2",
Job: job,
ID: structs.GenerateUUID(),
},
}
@ -477,6 +479,7 @@ func TestDistinctHostsIterator_JobDistinctHosts(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
},
@ -484,6 +487,7 @@ func TestDistinctHostsIterator_JobDistinctHosts(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: "ignore 2",
Job: job,
ID: structs.GenerateUUID(),
},
}
@ -658,6 +662,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: alloc1ID,
NodeID: nodes[0].ID,
},
@ -666,6 +671,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: "ignore 2",
Job: job,
ID: structs.GenerateUUID(),
NodeID: nodes[0].ID,
},
@ -674,6 +680,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
NodeID: nodes[2].ID,
},
@ -682,6 +689,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: "ignore 2",
Job: job,
ID: structs.GenerateUUID(),
NodeID: nodes[2].ID,
},
@ -693,6 +701,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: job.ID,
Job: job,
ID: stoppingAllocID,
NodeID: nodes[4].ID,
},
@ -704,6 +713,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: alloc1ID,
EvalID: structs.GenerateUUID(),
NodeID: nodes[0].ID,
@ -712,6 +722,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[1].ID,
@ -721,6 +732,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: "ignore 2",
Job: job,
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[1].ID,
@ -728,6 +740,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[3].ID,
@ -737,6 +750,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: "ignore 2",
Job: job,
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[3].ID,
@ -744,6 +758,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: job.ID,
Job: job,
ID: stoppingAllocID,
EvalID: structs.GenerateUUID(),
NodeID: nodes[4].ID,
@ -803,6 +818,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty_RemoveAndReplace(t *testin
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
NodeID: nodes[0].ID,
},
@ -813,6 +829,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty_RemoveAndReplace(t *testin
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: stoppingAllocID,
NodeID: nodes[0].ID,
},
@ -822,6 +839,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty_RemoveAndReplace(t *testin
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: stoppingAllocID,
EvalID: structs.GenerateUUID(),
NodeID: nodes[0].ID,
@ -886,6 +904,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty_Infeasible(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
NodeID: nodes[0].ID,
},
@ -894,6 +913,7 @@ func TestDistinctPropertyIterator_JobDistinctProperty_Infeasible(t *testing.T) {
&structs.Allocation{
TaskGroup: tg2.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[1].ID,
@ -961,6 +981,7 @@ func TestDistinctPropertyIterator_TaskGroupDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
NodeID: nodes[0].ID,
},
@ -972,6 +993,7 @@ func TestDistinctPropertyIterator_TaskGroupDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: stoppingAllocID,
NodeID: nodes[2].ID,
},
@ -981,6 +1003,7 @@ func TestDistinctPropertyIterator_TaskGroupDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[1].ID,
@ -990,6 +1013,7 @@ func TestDistinctPropertyIterator_TaskGroupDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: "ignore 2",
Job: job,
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[2].ID,
@ -998,6 +1022,7 @@ func TestDistinctPropertyIterator_TaskGroupDistinctProperty(t *testing.T) {
&structs.Allocation{
TaskGroup: tg1.Name,
JobID: job.ID,
Job: job,
ID: stoppingAllocID,
EvalID: structs.GenerateUUID(),
NodeID: nodes[2].ID,

14
scheduler/rank_test.go
View File

@ -202,11 +202,13 @@ func TestBinPackIterator_ExistingAlloc(t *testing.T) {
static := NewStaticRankIterator(ctx, nodes)
// Add existing allocations
j1, j2 := mock.Job(), mock.Job()
alloc1 := &structs.Allocation{
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[0].Node.ID,
JobID: structs.GenerateUUID(),
JobID: j1.ID,
Job: j1,
Resources: &structs.Resources{
CPU: 2048,
MemoryMB: 2048,
@ -219,7 +221,8 @@ func TestBinPackIterator_ExistingAlloc(t *testing.T) {
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[1].Node.ID,
JobID: structs.GenerateUUID(),
JobID: j2.ID,
Job: j2,
Resources: &structs.Resources{
CPU: 1024,
MemoryMB: 1024,
@ -286,11 +289,13 @@ func TestBinPackIterator_ExistingAlloc_PlannedEvict(t *testing.T) {
static := NewStaticRankIterator(ctx, nodes)
// Add existing allocations
j1, j2 := mock.Job(), mock.Job()
alloc1 := &structs.Allocation{
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[0].Node.ID,
JobID: structs.GenerateUUID(),
JobID: j1.ID,
Job: j1,
Resources: &structs.Resources{
CPU: 2048,
MemoryMB: 2048,
@ -303,7 +308,8 @@ func TestBinPackIterator_ExistingAlloc_PlannedEvict(t *testing.T) {
ID: structs.GenerateUUID(),
EvalID: structs.GenerateUUID(),
NodeID: nodes[1].Node.ID,
JobID: structs.GenerateUUID(),
JobID: j2.ID,
Job: j2,
Resources: &structs.Resources{
CPU: 1024,
MemoryMB: 1024,