remove backcompat shims for plan normalization

In #5602 we introduced plan normalization, where we reduced the size of the
required Raft log entry for plan apply by dropping the job spec from the
plan. This required backwards compatibility shims that we intended to remove in
Nomad 0.11. It's been long impossible to upgrade to any currently supported
version of Nomad from that far back, so let's remove these backwards
compatibility shims.

Ref: #5602

Author: tgross

nomad/acl_test.go

@@ -152,10 +152,8 @@ func TestAuthenticate_mTLS(t *testing.T) {
 	must.NoError(t, err, must.Sprint("could not sign claims"))
 
 	planReq := &structs.ApplyPlanResultsRequest{
-		AllocUpdateRequest: structs.AllocUpdateRequest{
-			Alloc: []*structs.Allocation{alloc1, alloc2},
-			Job:   job,
-		},
+		AllocsUpdated: []*structs.Allocation{alloc1, alloc2},
+		Job:           job,
 	}
 	_, _, err = leader.raftApply(structs.ApplyPlanResultsRequestType, planReq)
 	must.NoError(t, err)

nomad/eval_endpoint_test.go

@@ -397,11 +397,9 @@ func TestEvalEndpoint_Dequeue_UpdateWaitIndex(t *testing.T) {
 
 	// Create a plan result and apply it with a later index
 	res := structs.ApplyPlanResultsRequest{
-		AllocUpdateRequest: structs.AllocUpdateRequest{
-			Alloc: []*structs.Allocation{alloc},
-			Job:   job,
-		},
-		EvalID: eval.ID,
+		AllocsUpdated: []*structs.Allocation{alloc},
+		Job:           job,
+		EvalID:        eval.ID,
 	}
 	assert := assert.New(t)
 	err := state.UpsertPlanResults(structs.MsgTypeTestSetup, 1000, &res)

nomad/fsm.go

@@ -989,7 +989,7 @@ func (n *nomadFSM) applyAllocClientUpdate(msgType structs.MessageType, buf []byt
 		return err
 	}
 
-	// Update any evals
+	// Update any evals that were added by the RPC handler
 	if len(req.Evals) > 0 {
 		if err := n.upsertEvals(msgType, index, req.Evals); err != nil {
 			n.logger.Error("applyAllocClientUpdate failed to update evaluations", "error", err)

nomad/fsm_test.go

@@ -1635,13 +1635,12 @@ func TestFSM_ApplyPlanResults(t *testing.T) {
 	eval2.JobID = job2.ID
 
 	req := structs.ApplyPlanResultsRequest{
-		AllocUpdateRequest: structs.AllocUpdateRequest{
-			Job:   job,
-			Alloc: []*structs.Allocation{alloc},
-		},
-		Deployment:      d,
-		EvalID:          eval.ID,
-		NodePreemptions: []*structs.Allocation{alloc1, alloc2},
+		Job:           job,
+		AllocsUpdated: []*structs.Allocation{alloc},
+		Deployment:    d,
+		EvalID:        eval.ID,
+		AllocsPreempted: []*structs.AllocationDiff{
+			alloc1.AllocationDiff(), alloc2.AllocationDiff()},
 		PreemptionEvals: []*structs.Evaluation{eval1, eval2},
 	}
 	buf, err := structs.Encode(structs.ApplyPlanResultsRequestType, req)
@@ -1703,11 +1702,9 @@ func TestFSM_ApplyPlanResults(t *testing.T) {
 	evictAlloc.Job = nil
 	evictAlloc.DesiredStatus = structs.AllocDesiredStatusEvict
 	req2 := structs.ApplyPlanResultsRequest{
-		AllocUpdateRequest: structs.AllocUpdateRequest{
-			Job:   job,
-			Alloc: []*structs.Allocation{evictAlloc},
-		},
-		EvalID: eval.ID,
+		Job:           job,
+		AllocsUpdated: []*structs.Allocation{evictAlloc},
+		EvalID:        eval.ID,
 	}
 	buf, err = structs.Encode(structs.ApplyPlanResultsRequestType, req2)
 	assert.Nil(err)

nomad/plan_apply.go

@@ -244,9 +244,7 @@ func (p *planner) applyPlan(plan *structs.Plan, result *structs.PlanResult, snap
 
 	// Setup the update request
 	req := structs.ApplyPlanResultsRequest{
-		AllocUpdateRequest: structs.AllocUpdateRequest{
-			Job: plan.Job,
-		},
+		Job:               plan.Job,
 		Deployment:        result.Deployment,
 		DeploymentUpdates: result.DeploymentUpdates,
 		IneligibleNodes:   result.IneligibleNodes,
@@ -256,74 +254,38 @@ func (p *planner) applyPlan(plan *structs.Plan, result *structs.PlanResult, snap
 
 	preemptedJobIDs := make(map[structs.NamespacedID]struct{})
 
-	if p.srv.peersCache.ServersMeetMinimumVersion(p.srv.Region(), MinVersionPlanNormalization, true) {
-		// Initialize the allocs request using the new optimized log entry format.
-		// Determine the minimum number of updates, could be more if there
-		// are multiple updates per node
-		req.AllocsStopped = make([]*structs.AllocationDiff, 0, len(result.NodeUpdate))
-		req.AllocsUpdated = make([]*structs.Allocation, 0, len(result.NodeAllocation))
-		req.AllocsPreempted = make([]*structs.AllocationDiff, 0, len(result.NodePreemptions))
-
-		for _, updateList := range result.NodeUpdate {
-			for _, stoppedAlloc := range updateList {
-				req.AllocsStopped = append(req.AllocsStopped, normalizeStoppedAlloc(stoppedAlloc, unixNow))
-			}
-		}
-
-		for _, allocList := range result.NodeAllocation {
-			req.AllocsUpdated = append(req.AllocsUpdated, allocList...)
-		}
-
-		// Set the time the alloc was applied for the first time. This can be used
-		// to approximate the scheduling time.
-		updateAllocTimestamps(req.AllocsUpdated, unixNow)
+	// Initialize the allocs request using the new optimized log entry format.
+	// Determine the minimum number of updates, could be more if there
+	// are multiple updates per node
+	req.AllocsStopped = make([]*structs.AllocationDiff, 0, len(result.NodeUpdate))
+	req.AllocsUpdated = make([]*structs.Allocation, 0, len(result.NodeAllocation))
+	req.AllocsPreempted = make([]*structs.AllocationDiff, 0, len(result.NodePreemptions))
 
-		err := signAllocIdentities(p.srv.encrypter, plan.Job, req.AllocsUpdated, now)
-		if err != nil {
-			return nil, err
-		}
-
-		for _, preemptions := range result.NodePreemptions {
-			for _, preemptedAlloc := range preemptions {
-				req.AllocsPreempted = append(req.AllocsPreempted, normalizePreemptedAlloc(preemptedAlloc, unixNow))
-
-				// Gather jobids to create follow up evals
-				appendNamespacedJobID(preemptedJobIDs, preemptedAlloc)
-			}
+	for _, updateList := range result.NodeUpdate {
+		for _, stoppedAlloc := range updateList {
+			req.AllocsStopped = append(req.AllocsStopped, normalizeStoppedAlloc(stoppedAlloc, unixNow))
 		}
-	} else {
-		// COMPAT 0.11: This branch is deprecated and will only be used to support
-		// application of older log entries. Expected to be removed in a future version.
-
-		// Determine the minimum number of updates, could be more if there
-		// are multiple updates per node
-		minUpdates := len(result.NodeUpdate)
-		minUpdates += len(result.NodeAllocation)
+	}
 
-		// Initialize using the older log entry format for Alloc and NodePreemptions
-		req.Alloc = make([]*structs.Allocation, 0, minUpdates)
-		req.NodePreemptions = make([]*structs.Allocation, 0, len(result.NodePreemptions))
+	for _, allocList := range result.NodeAllocation {
+		req.AllocsUpdated = append(req.AllocsUpdated, allocList...)
+	}
 
-		for _, updateList := range result.NodeUpdate {
-			req.Alloc = append(req.Alloc, updateList...)
-		}
-		for _, allocList := range result.NodeAllocation {
-			req.Alloc = append(req.Alloc, allocList...)
-		}
+	// Set the time the alloc was applied for the first time. This can be used
+	// to approximate the scheduling time.
+	updateAllocTimestamps(req.AllocsUpdated, unixNow)
 
-		for _, preemptions := range result.NodePreemptions {
-			req.NodePreemptions = append(req.NodePreemptions, preemptions...)
-		}
+	err := signAllocIdentities(p.srv.encrypter, plan.Job, req.AllocsUpdated, now)
+	if err != nil {
+		return nil, err
+	}
 
-		// Set the time the alloc was applied for the first time. This can be used
-		// to approximate the scheduling time.
-		updateAllocTimestamps(req.Alloc, unixNow)
+	for _, preemptions := range result.NodePreemptions {
+		for _, preemptedAlloc := range preemptions {
+			req.AllocsPreempted = append(req.AllocsPreempted, normalizePreemptedAlloc(preemptedAlloc, unixNow))
 
-		// Set modify time for preempted allocs if any
-		// Also gather jobids to create follow up evals
-		for _, alloc := range req.NodePreemptions {
-			alloc.ModifyTime = unixNow
-			appendNamespacedJobID(preemptedJobIDs, alloc)
+			// Gather jobids to create follow up evals
+			appendNamespacedJobID(preemptedJobIDs, preemptedAlloc)
 		}
 	}
 

nomad/plan_normalization_test.go

@@ -505,12 +505,10 @@ func TestEventsFromChanges_ApplyPlanResultsRequestType(t *testing.T) {
 
 	msgType := structs.ApplyPlanResultsRequestType
 	req := &structs.ApplyPlanResultsRequest{
-		AllocUpdateRequest: structs.AllocUpdateRequest{
-			Alloc: []*structs.Allocation{alloc, alloc2},
-			Job:   job,
-		},
-		Deployment: d,
-		EvalID:     eval.ID,
+		AllocsUpdated: []*structs.Allocation{alloc, alloc2},
+		Job:           job,
+		Deployment:    d,
+		EvalID:        eval.ID,
 	}
 
 	must.NoError(t, s.UpsertPlanResults(msgType, 100, req))

nomad/state/events_test.go

@@ -374,12 +374,6 @@ func (s *StateStore) UpsertPlanResults(msgType structs.MessageType, index uint64
 		return err
 	}
 
-	// COMPAT 0.11: Remove this denormalization when NodePreemptions is removed
-	results.NodePreemptions, err = snapshot.DenormalizeAllocationSlice(results.NodePreemptions)
-	if err != nil {
-		return err
-	}
-
 	txn := s.db.WriteTxnMsgT(msgType, index)
 	defer txn.Abort()
 
@@ -418,27 +412,13 @@ func (s *StateStore) UpsertPlanResults(msgType structs.MessageType, index uint64
 	}
 
 	numAllocs := 0
-	if len(results.Alloc) > 0 || len(results.NodePreemptions) > 0 {
-		// COMPAT 0.11: This branch will be removed, when Alloc is removed
-		// Attach the job to all the allocations. It is pulled out in the payload to
-		// avoid the redundancy of encoding, but should be denormalized prior to
-		// being inserted into MemDB.
-		addComputedAllocAttrs(results.Alloc, results.Job)
-		numAllocs = len(results.Alloc) + len(results.NodePreemptions)
-	} else {
-		// Attach the job to all the allocations. It is pulled out in the payload to
-		// avoid the redundancy of encoding, but should be denormalized prior to
-		// being inserted into MemDB.
-		addComputedAllocAttrs(results.AllocsUpdated, results.Job)
-		numAllocs = len(allocsStopped) + len(results.AllocsUpdated) + len(allocsPreempted)
-	}
+	// Attach the job to all the allocations. It is pulled out in the payload to
+	// avoid the redundancy of encoding, but should be denormalized prior to
+	// being inserted into MemDB.
+	addComputedAllocAttrs(results.AllocsUpdated, results.Job)
+	numAllocs = len(allocsStopped) + len(results.AllocsUpdated) + len(allocsPreempted)
 
 	allocsToUpsert := make([]*structs.Allocation, 0, numAllocs)
-
-	// COMPAT 0.11: Both these appends should be removed when Alloc and NodePreemptions are removed
-	allocsToUpsert = append(allocsToUpsert, results.Alloc...)
-	allocsToUpsert = append(allocsToUpsert, results.NodePreemptions...)
-
 	allocsToUpsert = append(allocsToUpsert, allocsStopped...)
 	allocsToUpsert = append(allocsToUpsert, results.AllocsUpdated...)
 	allocsToUpsert = append(allocsToUpsert, allocsPreempted...)

nomad/state/state_store.go

@@ -695,9 +695,7 @@ func TestAlloc_ServiceRegistrationLifecycle(t *testing.T) {
 	index++
 	must.NoError(t, store.UpsertPlanResults(structs.MsgTypeTestSetup, index,
 		&structs.ApplyPlanResultsRequest{
-			AllocUpdateRequest: structs.AllocUpdateRequest{
-				AllocsStopped: []*structs.AllocationDiff{&diff},
-			},
+			AllocsStopped: []*structs.AllocationDiff{&diff},
 		}))
 
 	iter, err = store.GetServiceRegistrationsByAllocID(nil, alloc1.ID)