Per-comment, remove structs.Allocation's Services attribute.

Nuke PopulateServiceIDs() now that it's also no longer needed.

nomad/structs/structs.go

@@ -2311,9 +2311,6 @@ type Allocation struct {
 	// task. These should sum to the total Resources.
 	TaskResources map[string]*Resources
 
-	// Services is a map of service names to service ids
-	Services map[string]string
-
 	// Metrics associated with this allocation
 	Metrics *AllocMetric
 
@@ -2363,14 +2360,6 @@ func (a *Allocation) Copy() *Allocation {
 		na.TaskResources = tr
 	}
 
-	if a.Services != nil {
-		s := make(map[string]string, len(na.Services))
-		for service, id := range na.Services {
-			s[service] = id
-		}
-		na.Services = s
-	}
-
 	na.Metrics = na.Metrics.Copy()
 
 	if a.TaskStates != nil {
@@ -2439,31 +2428,6 @@ func (a *Allocation) Stub() *AllocListStub {
 	}
 }
 
-// PopulateServiceIDs generates the service IDs for all the service definitions
-// in that Allocation
-func (a *Allocation) PopulateServiceIDs(tg *TaskGroup) {
-	// Retain the old services, and re-initialize. We may be removing
-	// services, so we cannot update the existing map.
-	previous := a.Services
-	a.Services = make(map[string]string)
-
-	for _, task := range tg.Tasks {
-		for _, service := range task.ConsulServices {
-			// Retain the service if an ID is already generated
-			if id, ok := previous[service.Name]; ok {
-				a.Services[service.Name] = id
-				continue
-			}
-
-			// If the service hasn't been generated an ID, we generate one.
-			// We add a prefix to the Service ID so that we can know that this service
-			// is managed by Nomad since Consul can also have service which are not
-			// managed by Nomad
-			a.Services[service.Name] = fmt.Sprintf("%s-%s", NomadConsulPrefix, GenerateUUID())
-		}
-	}
-}
-
 var (
 	// AllocationIndexRegex is a regular expression to find the allocation index.
 	AllocationIndexRegex = regexp.MustCompile(".+\\[(\\d+)\\]$")

scheduler/generic_sched.go

@@ -424,10 +424,6 @@ func (s *GenericScheduler) computePlacements(place []allocTuple) error {
 				ClientStatus:  structs.AllocClientStatusPending,
 			}
 
-			// Generate service IDs tasks in this allocation
-			// COMPAT - This is no longer required and would be removed in v0.4
-			alloc.PopulateServiceIDs(missing.TaskGroup)
-
 			s.plan.AppendAlloc(alloc)
 		} else {
 			// Lazy initialize the failed map

scheduler/system_sched.go

@@ -264,10 +264,6 @@ func (s *SystemScheduler) computePlacements(place []allocTuple) error {
 				ClientStatus:  structs.AllocClientStatusPending,
 			}
 
-			// Generate service IDs tasks in this allocation
-			// COMPAT - This is no longer required and would be removed in v0.4
-			alloc.PopulateServiceIDs(missing.TaskGroup)
-
 			s.plan.AppendAlloc(alloc)
 		} else {
 			// Lazy initialize the failed map

scheduler/util.go

@@ -455,7 +455,6 @@ func inplaceUpdate(ctx Context, eval *structs.Evaluation, job *structs.Job,
 		newAlloc.Metrics = ctx.Metrics()
 		newAlloc.DesiredStatus = structs.AllocDesiredStatusRun
 		newAlloc.ClientStatus = structs.AllocClientStatusPending
-		newAlloc.PopulateServiceIDs(update.TaskGroup)
 		ctx.Plan().AppendAlloc(newAlloc)
 
 		// Remove this allocation from the slice

scheduler/util_test.go

@@ -664,16 +664,8 @@ func TestInplaceUpdate_Success(t *testing.T) {
 		DesiredStatus: structs.AllocDesiredStatusRun,
 	}
 	alloc.TaskResources = map[string]*structs.Resources{"web": alloc.Resources}
-	alloc.PopulateServiceIDs(job.TaskGroups[0])
 	noErr(t, state.UpsertAllocs(1001, []*structs.Allocation{alloc}))
 
-	webFeSrvID := alloc.Services["web-frontend"]
-	adminSrvID := alloc.Services["web-admin"]
-
-	if webFeSrvID == "" || adminSrvID == "" {
-		t.Fatal("Service ID needs to be generated for service")
-	}
-
 	// Create a new task group that updates the resources.
 	tg := &structs.TaskGroup{}
 	*tg = *job.TaskGroups[0]
@@ -716,20 +708,35 @@ func TestInplaceUpdate_Success(t *testing.T) {
 	}
 
 	// Get the alloc we inserted.
-	a := ctx.plan.NodeAllocation[alloc.NodeID][0]
-	if len(a.Services) != 3 {
-		t.Fatalf("Expected number of services: %v, Actual: %v", 3, len(a.Services))
+	a := inplace[0].Alloc // TODO(sean@): Verify this is correct vs: ctx.plan.NodeAllocation[alloc.NodeID][0]
+	if a.Job == nil {
+		t.Fatalf("bad")
 	}
 
-	// Test that the service id for the old service is still the same
-	if a.Services["web-frontend"] != webFeSrvID {
-		t.Fatalf("Expected service ID: %v, Actual: %v", webFeSrvID, a.Services["web-frontend"])
+	if len(a.Job.TaskGroups) != 1 {
+		t.Fatalf("bad")
 	}
 
-	// Test that the map doesn't contain the service ID of the admin Service
-	// anymore
-	if _, ok := a.Services["web-admin"]; ok {
-		t.Fatal("Service shouldn't be present")
+	if len(a.Job.TaskGroups[0].Tasks) != 1 {
+		t.Fatalf("bad")
+	}
+
+	if len(a.Job.TaskGroups[0].Tasks[0].ConsulServices) != 3 {
+		t.Fatalf("Expected number of services: %v, Actual: %v", 3, len(a.Job.TaskGroups[0].Tasks[0].ConsulServices))
+	}
+
+	serviceNames := make(map[string]struct{}, 3)
+	for _, consulService := range a.Job.TaskGroups[0].Tasks[0].ConsulServices {
+		serviceNames[consulService.Name] = struct{}{}
+	}
+	if len(serviceNames) != 3 {
+		t.Fatalf("bad")
+	}
+
+	for _, name := range []string{"dummy-service", "dummy-service2", "web-frontend"} {
+		if _, found := serviceNames[name]; !found {
+			t.Errorf("Expected consul service name missing: %v", name)
+		}
 	}
 }