Merge pull request #6240 from EvanErcolano/remove-unused-param-consul…

…-client Remove unused canary param from MakeTaskServiceID
hashicorp · Sep 2, 2019 · 798b605 · 798b605
2 parents 6ac3a4a + 8598618
commit 798b605
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 17 deletions.
diff --git a/client/allocrunner/taskrunner/envoybootstrap_hook.go b/client/allocrunner/taskrunner/envoybootstrap_hook.go
@@ -80,8 +80,7 @@ func (h *envoyBootstrapHook) Prestart(ctx context.Context, req *interfaces.TaskP
 	// it to the secrets directory like Vault tokens.
 	fn := filepath.Join(req.TaskDir.SecretsDir, "envoy_bootstrap.json")
 
-	canary := h.alloc.DeploymentStatus.IsCanary()
-	id := agentconsul.MakeTaskServiceID(h.alloc.ID, "group-"+tg.Name, service, canary)
+	id := agentconsul.MakeTaskServiceID(h.alloc.ID, "group-"+tg.Name, service)
 	h.logger.Debug("bootstrapping envoy", "sidecar_for", service.Name, "boostrap_file", fn, "sidecar_for_id", id, "grpc_addr", grpcAddr)
 
 	// Since Consul services are registered asynchronously with this task

diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go
@@ -704,7 +704,7 @@ func (c *ServiceClient) serviceRegs(ops *operations, service *structs.Service, t
 	*ServiceRegistration, error) {
 
 	// Get the services ID
-	id := MakeTaskServiceID(task.AllocID, task.Name, service, task.Canary)
+	id := MakeTaskServiceID(task.AllocID, task.Name, service)
 	sreg := &ServiceRegistration{
 		serviceID: id,
 		checkIDs:  make(map[string]struct{}, len(service.Checks)),
@@ -974,7 +974,7 @@ func (c *ServiceClient) RegisterTask(task *TaskServices) error {
 	// Start watching checks. Done after service registrations are built
 	// since an error building them could leak watches.
 	for _, service := range task.Services {
-		serviceID := MakeTaskServiceID(task.AllocID, task.Name, service, task.Canary)
+		serviceID := MakeTaskServiceID(task.AllocID, task.Name, service)
 		for _, check := range service.Checks {
 			if check.TriggersRestarts() {
 				checkID := makeCheckID(serviceID, check)
@@ -997,11 +997,11 @@ func (c *ServiceClient) UpdateTask(old, newTask *TaskServices) error {
 
 	existingIDs := make(map[string]*structs.Service, len(old.Services))
 	for _, s := range old.Services {
-		existingIDs[MakeTaskServiceID(old.AllocID, old.Name, s, old.Canary)] = s
+		existingIDs[MakeTaskServiceID(old.AllocID, old.Name, s)] = s
 	}
 	newIDs := make(map[string]*structs.Service, len(newTask.Services))
 	for _, s := range newTask.Services {
-		newIDs[MakeTaskServiceID(newTask.AllocID, newTask.Name, s, newTask.Canary)] = s
+		newIDs[MakeTaskServiceID(newTask.AllocID, newTask.Name, s)] = s
 	}
 
 	// Loop over existing Service IDs to see if they have been removed
@@ -1098,7 +1098,7 @@ func (c *ServiceClient) UpdateTask(old, newTask *TaskServices) error {
 	// Start watching checks. Done after service registrations are built
 	// since an error building them could leak watches.
 	for _, service := range newIDs {
-		serviceID := MakeTaskServiceID(newTask.AllocID, newTask.Name, service, newTask.Canary)
+		serviceID := MakeTaskServiceID(newTask.AllocID, newTask.Name, service)
 		for _, check := range service.Checks {
 			if check.TriggersRestarts() {
 				checkID := makeCheckID(serviceID, check)
@@ -1116,7 +1116,7 @@ func (c *ServiceClient) RemoveTask(task *TaskServices) {
 	ops := operations{}
 
 	for _, service := range task.Services {
-		id := MakeTaskServiceID(task.AllocID, task.Name, service, task.Canary)
+		id := MakeTaskServiceID(task.AllocID, task.Name, service)
 		ops.deregServices = append(ops.deregServices, id)
 
 		for _, check := range service.Checks {
@@ -1281,7 +1281,7 @@ func makeAgentServiceID(role string, service *structs.Service) string {
 // Consul.
 //
 //	Example Service ID: _nomad-task-b4e61df9-b095-d64e-f241-23860da1375f-redis-http-http
-func MakeTaskServiceID(allocID, taskName string, service *structs.Service, canary bool) string {
+func MakeTaskServiceID(allocID, taskName string, service *structs.Service) string {
 	return fmt.Sprintf("%s%s-%s-%s-%s", nomadTaskPrefix, allocID, taskName, service.Name, service.PortLabel)
 }
 

diff --git a/command/agent/consul/group_test.go b/command/agent/consul/group_test.go
@@ -72,7 +72,7 @@ func TestConsul_Connect(t *testing.T) {
 
 	// required by isNomadSidecar assertion below
 	serviceRegMap := map[string]*api.AgentServiceRegistration{
-		MakeTaskServiceID(alloc.ID, "group-"+alloc.TaskGroup, tg.Services[0], false): nil,
+		MakeTaskServiceID(alloc.ID, "group-"+alloc.TaskGroup, tg.Services[0]): nil,
 	}
 
 	require.NoError(t, serviceClient.RegisterGroup(alloc))
@@ -90,7 +90,7 @@ func TestConsul_Connect(t *testing.T) {
 		require.NoError(t, err)
 		require.Len(t, services, 2)
 
-		serviceID := MakeTaskServiceID(alloc.ID, "group-"+alloc.TaskGroup, tg.Services[0], false)
+		serviceID := MakeTaskServiceID(alloc.ID, "group-"+alloc.TaskGroup, tg.Services[0])
 		connectID := serviceID + "-sidecar-proxy"
 
 		require.Contains(t, services, serviceID)

diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go
@@ -1711,7 +1711,7 @@ func TestConsul_ServiceDeregistration_OutProbation(t *testing.T) {
 		},
 	}
 	remainingTaskServiceID := MakeTaskServiceID(remainingTask.AllocID,
-		remainingTask.Name, remainingTask.Services[0], false)
+		remainingTask.Name, remainingTask.Services[0])
 
 	require.NoError(ctx.ServiceClient.RegisterTask(remainingTask))
 	require.NoError(ctx.syncOnce())
@@ -1734,7 +1734,7 @@ func TestConsul_ServiceDeregistration_OutProbation(t *testing.T) {
 		},
 	}
 	explicitlyRemovedTaskServiceID := MakeTaskServiceID(explicitlyRemovedTask.AllocID,
-		explicitlyRemovedTask.Name, explicitlyRemovedTask.Services[0], false)
+		explicitlyRemovedTask.Name, explicitlyRemovedTask.Services[0])
 
 	require.NoError(ctx.ServiceClient.RegisterTask(explicitlyRemovedTask))
 
@@ -1759,7 +1759,7 @@ func TestConsul_ServiceDeregistration_OutProbation(t *testing.T) {
 		},
 	}
 	outofbandTaskServiceID := MakeTaskServiceID(outofbandTask.AllocID,
-		outofbandTask.Name, outofbandTask.Services[0], false)
+		outofbandTask.Name, outofbandTask.Services[0])
 
 	require.NoError(ctx.ServiceClient.RegisterTask(outofbandTask))
 	require.NoError(ctx.syncOnce())
@@ -1820,7 +1820,7 @@ func TestConsul_ServiceDeregistration_InProbation(t *testing.T) {
 		},
 	}
 	remainingTaskServiceID := MakeTaskServiceID(remainingTask.AllocID,
-		remainingTask.Name, remainingTask.Services[0], false)
+		remainingTask.Name, remainingTask.Services[0])
 
 	require.NoError(ctx.ServiceClient.RegisterTask(remainingTask))
 	require.NoError(ctx.syncOnce())
@@ -1843,7 +1843,7 @@ func TestConsul_ServiceDeregistration_InProbation(t *testing.T) {
 		},
 	}
 	explicitlyRemovedTaskServiceID := MakeTaskServiceID(explicitlyRemovedTask.AllocID,
-		explicitlyRemovedTask.Name, explicitlyRemovedTask.Services[0], false)
+		explicitlyRemovedTask.Name, explicitlyRemovedTask.Services[0])
 
 	require.NoError(ctx.ServiceClient.RegisterTask(explicitlyRemovedTask))
 
@@ -1868,7 +1868,7 @@ func TestConsul_ServiceDeregistration_InProbation(t *testing.T) {
 		},
 	}
 	outofbandTaskServiceID := MakeTaskServiceID(outofbandTask.AllocID,
-		outofbandTask.Name, outofbandTask.Services[0], false)
+		outofbandTask.Name, outofbandTask.Services[0])
 
 	require.NoError(ctx.ServiceClient.RegisterTask(outofbandTask))
 	require.NoError(ctx.syncOnce())