From 3e95ca61ef4b07972bee6942de40ce518596ab85 Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Thu, 9 Jun 2016 02:51:12 -0400 Subject: [PATCH] Per-comment, remove structs.Allocation's Services attribute. Nuke PopulateServiceIDs() now that it's also no longer needed. --- nomad/structs/structs.go | 36 ------------------------------- scheduler/generic_sched.go | 4 ---- scheduler/system_sched.go | 4 ---- scheduler/util.go | 1 - scheduler/util_test.go | 43 ++++++++++++++++++++++---------------- 5 files changed, 25 insertions(+), 63 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index a6d4df8ee..c9382fdc3 100644 --- a/nomad/structs/structs.go +++ b/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+)\\]$") diff --git a/scheduler/generic_sched.go b/scheduler/generic_sched.go index 0a942cd13..584b8a017 100644 --- a/scheduler/generic_sched.go +++ b/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 diff --git a/scheduler/system_sched.go b/scheduler/system_sched.go index 3b8a6f438..42f509b39 100644 --- a/scheduler/system_sched.go +++ b/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 diff --git a/scheduler/util.go b/scheduler/util.go index 1c1c93275..269a4b4f0 100644 --- a/scheduler/util.go +++ b/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 diff --git a/scheduler/util_test.go b/scheduler/util_test.go index 130edce76..0e8a5d5d8 100644 --- a/scheduler/util_test.go +++ b/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) + } } }