Skip to content

Commit

Permalink
Fix bug in plan applier introduced in PR-5602
Browse files Browse the repository at this point in the history
This fixes a bug in the state store during plan apply. When
denormalizing preempted allocations it incorrectly set the preemptor's
job during the update. This eventually causes a panic downstream in the
client. Added a test assertion that failed before and passes after this fix
  • Loading branch information
Preetha Appan committed May 16, 2019
1 parent 781c94b commit d3061f9
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
7 changes: 4 additions & 3 deletions nomad/state/state_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -4144,7 +4144,7 @@ func (s *StateSnapshot) DenormalizeAllocationSlice(allocs []*structs.Allocation,

// DenormalizeAllocationDiffSlice queries the Allocation for each AllocationDiff and merges
// the updated attributes with the existing Allocation, and attaches the Job provided
func (s *StateSnapshot) DenormalizeAllocationDiffSlice(allocDiffs []*structs.AllocationDiff, job *structs.Job) ([]*structs.Allocation, error) {
func (s *StateSnapshot) DenormalizeAllocationDiffSlice(allocDiffs []*structs.AllocationDiff, planJob *structs.Job) ([]*structs.Allocation, error) {
// Output index for denormalized Allocations
j := 0

Expand All @@ -4160,15 +4160,16 @@ func (s *StateSnapshot) DenormalizeAllocationDiffSlice(allocDiffs []*structs.All

// Merge the updates to the Allocation
allocCopy := alloc.CopySkipJob()
allocCopy.Job = job

if allocDiff.PreemptedByAllocation != "" {
// If alloc is a preemption
// If alloc is a preemption set the job from the alloc read from the state store
allocCopy.Job = alloc.Job.Copy()
allocCopy.PreemptedByAllocation = allocDiff.PreemptedByAllocation
allocCopy.DesiredDescription = getPreemptedAllocDesiredDescription(allocDiff.PreemptedByAllocation)
allocCopy.DesiredStatus = structs.AllocDesiredStatusEvict
} else {
// If alloc is a stopped alloc
allocCopy.Job = planJob
allocCopy.DesiredDescription = allocDiff.DesiredDescription
allocCopy.DesiredStatus = structs.AllocDesiredStatusStop
if allocDiff.ClientStatus != "" {
Expand Down
2 changes: 2 additions & 0 deletions nomad/state/state_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,8 @@ func TestStateStore_UpsertPlanResults_PreemptedAllocs(t *testing.T) {
require.NoError(err)
require.Equal(preempted.DesiredStatus, structs.AllocDesiredStatusEvict)
require.Equal(preempted.DesiredDescription, fmt.Sprintf("Preempted by alloc ID %v", alloc.ID))
require.Equal(preempted.Job.ID, preemptedAlloc.Job.ID)
require.Equal(preempted.Job, preemptedAlloc.Job)

// Verify eval for preempted job
preemptedJobEval, err := state.EvalByID(ws, eval2.ID)
Expand Down

0 comments on commit d3061f9

Please sign in to comment.