Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for consul EnableTagOverride #5775

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,14 +364,15 @@ type ServiceCheck struct {

// The Service model represents a Consul service definition
type Service struct {
Id string
Name string
Tags []string
CanaryTags []string `mapstructure:"canary_tags"`
PortLabel string `mapstructure:"port"`
AddressMode string `mapstructure:"address_mode"`
Checks []ServiceCheck
CheckRestart *CheckRestart `mapstructure:"check_restart"`
Id string
Name string
Tags []string
CanaryTags []string `mapstructure:"canary_tags"`
PortLabel string `mapstructure:"port"`
AddressMode string `mapstructure:"address_mode"`
Checks []ServiceCheck
CheckRestart *CheckRestart `mapstructure:"check_restart"`
EnableTagOverride bool
}

func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) {
Expand Down
19 changes: 17 additions & 2 deletions command/agent/consul/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,17 @@ func agentServiceUpdateRequired(reg *api.AgentServiceRegistration, svc *api.Agen
reg.Port == svc.Port &&
reg.Address == svc.Address &&
reg.Name == svc.Service &&
reg.EnableTagOverride == svc.EnableTagOverride &&
reflect.DeepEqual(reg.Tags, svc.Tags))
}

// Replace with Consul tags
func replaceServiceTagsWithConsulTags(reg *api.AgentServiceRegistration, svc *api.AgentService) {
if svc != nil {
copy(reg.Tags, svc.Tags)
}
}

// operations are submitted to the main loop via commit() for synchronizing
// with Consul.
type operations struct {
Expand Down Expand Up @@ -485,6 +493,11 @@ func (c *ServiceClient) sync() error {
existingSvc, ok := consulServices[id]

if ok {
if locals.EnableTagOverride {
// if EnableTagOverride is set, then we want to keep the tags updated by external services
replaceServiceTagsWithConsulTags(locals, existingSvc)
}

// There is an existing registration of this service in Consul, so here
// we validate to see if the service has been invalidated to see if it
// should be updated.
Expand Down Expand Up @@ -597,6 +610,7 @@ func (c *ServiceClient) RegisterAgent(role string, services []*structs.Service)
Meta: map[string]string{
"external-source": "nomad",
},
EnableTagOverride: service.EnableTagOverride,
}
ops.regServices = append(ops.regServices, serviceReg)

Expand Down Expand Up @@ -695,6 +709,7 @@ func (c *ServiceClient) serviceRegs(ops *operations, service *structs.Service, t
Meta: map[string]string{
"external-source": "nomad",
},
EnableTagOverride: service.EnableTagOverride,
}
ops.regServices = append(ops.regServices, serviceReg)

Expand Down Expand Up @@ -1110,9 +1125,9 @@ func makeAgentServiceID(role string, service *structs.Service) string {
// Consul. All structs.Service fields are included in the ID's hash except
// Checks. This allows updates to merely compare IDs.
//
// Example Service ID: _nomad-task-b4e61df9-b095-d64e-f241-23860da1375f-redis-http
// Example Service ID: _nomad-task-b4e61df9-b095-d64e-f241-23860da1375f-redis-http-false
func makeTaskServiceID(allocID, taskName string, service *structs.Service, canary bool) string {
return fmt.Sprintf("%s%s-%s-%s", nomadTaskPrefix, allocID, taskName, service.Name)
return fmt.Sprintf("%s%s-%s-%s-%t", nomadTaskPrefix, allocID, taskName, service.Name, service.EnableTagOverride)
}

// makeCheckID creates a unique ID for a check.
Expand Down
103 changes: 103 additions & 0 deletions command/agent/consul/unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,79 @@ func TestConsul_ChangeTags(t *testing.T) {
}
}

func TestConsul_ChangeEnableTagOverride(t *testing.T) {
ctx := setupFake(t)
require := require.New(t)

require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task))
require.NoError(ctx.syncOnce())
require.Equal(1, len(ctx.FakeConsul.services), "Expected 1 service to be registered with Consul")

// Validate the alloc registration
reg1, err := ctx.ServiceClient.AllocRegistrations(ctx.Task.AllocID)
require.NoError(err)
require.NotNil(reg1, "Unexpected nil alloc registration")
require.Equal(1, reg1.NumServices())

for _, v := range ctx.FakeConsul.services {
require.Equal(false, v.EnableTagOverride)
}

// Update the task definition
origTask := ctx.Task.Copy()
ctx.Task.Services[0].EnableTagOverride = true

// Register and sync the update
require.NoError(ctx.ServiceClient.UpdateTask(origTask, ctx.Task))
require.NoError(ctx.syncOnce())
require.Equal(1, len(ctx.FakeConsul.services), "Expected 1 service to be registered with Consul")

// Validate the metadata changed
for _, v := range ctx.FakeConsul.services {
require.Equal(true, v.EnableTagOverride)
}
}

func TestConsul_ChangeTagsExternally(t *testing.T) {
ctx := setupFake(t)
require := require.New(t)

//EnableTagOverride is enabled
ctx.Task.Services[0].EnableTagOverride = true

require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task))
require.NoError(ctx.syncOnce())
require.Equal(1, len(ctx.FakeConsul.services), "Expected 1 service to be registered with Consul")

// Validate the alloc registration
reg1, err := ctx.ServiceClient.AllocRegistrations(ctx.Task.AllocID)
require.NoError(err)
require.NotNil(reg1, "Unexpected nil alloc registration")
require.Equal(1, reg1.NumServices())
require.Equal(0, reg1.NumChecks())

newTags := []string{"new", "tag"}
for _, v := range ctx.FakeConsul.services {
//update the tags in Consul directly
v.Tags = newTags
}

// sync the update
origTask := ctx.Task.Copy()
require.NoError(ctx.ServiceClient.UpdateTask(origTask, ctx.Task))
require.NoError(ctx.syncOnce())
require.Equal(1, len(ctx.FakeConsul.services), "Expected 1 service to be registered with Consul")

// Validate the metadata changed
for _, v := range ctx.FakeConsul.services {
require.Equal(newTags, v.Tags, ctx.Task.Services[0].Tags)
}

for _, v := range ctx.ServiceClient.services {
require.Equal(newTags, v.Tags, ctx.Task.Services[0].Tags)
}
}

// TestConsul_ChangePorts asserts that changing the ports on a service updates
// it in Consul. Pre-0.7.1 ports were not part of the service ID and this was a
// slightly different code path than changing tags.
Expand Down Expand Up @@ -1296,6 +1369,36 @@ func TestConsul_CanaryTags_NoTags(t *testing.T) {
require.Len(ctx.FakeConsul.services, 0)
}

// TestConsul_EnableTagOverrideEnabled asserts EnableTagOverride is set when EnableTagOverride=true
func TestConsul_EnableTagOverrideEnabled(t *testing.T) {
t.Parallel()
require := require.New(t)
ctx := setupFake(t)

ctx.Task.Services[0].EnableTagOverride = true

require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task))
require.NoError(ctx.syncOnce())
require.Len(ctx.FakeConsul.services, 1)
for _, service := range ctx.FakeConsul.services {
require.Equal(true, service.EnableTagOverride)
}
}

// TestConsul_EnableTagOverrideIsDisabledByDefault asserts EnableTagOverride is disabled by default
func TestConsul_EnableTagOverrideIsDisabledByDefault(t *testing.T) {
t.Parallel()
require := require.New(t)
ctx := setupFake(t)

require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task))
require.NoError(ctx.syncOnce())
require.Len(ctx.FakeConsul.services, 1)
for _, service := range ctx.FakeConsul.services {
require.Equal(false, service.EnableTagOverride)
}
}

// TestConsul_PeriodicSync asserts that Nomad periodically reconciles with
// Consul.
func TestConsul_PeriodicSync(t *testing.T) {
Expand Down
11 changes: 6 additions & 5 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -763,11 +763,12 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
structsTask.Services = make([]*structs.Service, l)
for i, service := range apiTask.Services {
structsTask.Services[i] = &structs.Service{
Name: service.Name,
PortLabel: service.PortLabel,
Tags: service.Tags,
CanaryTags: service.CanaryTags,
AddressMode: service.AddressMode,
Name: service.Name,
PortLabel: service.PortLabel,
Tags: service.Tags,
CanaryTags: service.CanaryTags,
AddressMode: service.AddressMode,
EnableTagOverride: service.EnableTagOverride,
}

if l := len(service.Checks); l != 0 {
Expand Down
10 changes: 10 additions & 0 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,11 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
},
},
},
{
Id: "id_serviceB",
Name: "serviceB",
EnableTagOverride: true,
},
},
Resources: &api.Resources{
CPU: helper.IntToPtr(100),
Expand Down Expand Up @@ -1698,6 +1703,11 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
},
},
},
{
Name: "serviceB",
EnableTagOverride: true,
AddressMode: "auto",
},
},
Resources: &structs.Resources{
CPU: 100,
Expand Down
30 changes: 30 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3949,6 +3949,12 @@ func TestTaskDiff(t *testing.T) {
Type: DiffTypeAdded,
Name: "Service",
Fields: []*FieldDiff{
{
Type: DiffTypeAdded,
Name: "EnableTagOverride",
Old: "",
New: "false",
},
{
Type: DiffTypeAdded,
Name: "Name",
Expand All @@ -3967,6 +3973,12 @@ func TestTaskDiff(t *testing.T) {
Type: DiffTypeDeleted,
Name: "Service",
Fields: []*FieldDiff{
{
Type: DiffTypeDeleted,
Name: "EnableTagOverride",
Old: "false",
New: "",
},
{
Type: DiffTypeDeleted,
Name: "Name",
Expand Down Expand Up @@ -4016,6 +4028,12 @@ func TestTaskDiff(t *testing.T) {
Name: "AddressMode",
New: "driver",
},
{
Type: DiffTypeNone,
Name: "EnableTagOverride",
Old: "false",
New: "false",
},
{
Type: DiffTypeNone,
Name: "Name",
Expand Down Expand Up @@ -4113,6 +4131,12 @@ func TestTaskDiff(t *testing.T) {
Type: DiffTypeNone,
Name: "AddressMode",
},
{
Type: DiffTypeNone,
Name: "EnableTagOverride",
Old: "false",
New: "false",
},
{
Type: DiffTypeNone,
Name: "Name",
Expand Down Expand Up @@ -4447,6 +4471,12 @@ func TestTaskDiff(t *testing.T) {
Old: "",
New: "",
},
{
Type: DiffTypeNone,
Name: "EnableTagOverride",
Old: "false",
New: "false",
},
{
Type: DiffTypeNone,
Name: "Name",
Expand Down
7 changes: 4 additions & 3 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5113,9 +5113,10 @@ type Service struct {
// this service.
AddressMode string

Tags []string // List of tags for the service
CanaryTags []string // List of tags for the service when it is a canary
Checks []*ServiceCheck // List of checks associated with the service
Tags []string // List of tags for the service
CanaryTags []string // List of tags for the service when it is a canary
Checks []*ServiceCheck // List of checks associated with the service
EnableTagOverride bool // Allow external agents to change the tags for a service
}

func (s *Service) Copy() *Service {
Expand Down