Skip to content

Commit

Permalink
fix consul checks
Browse files Browse the repository at this point in the history
  • Loading branch information
shoenig committed Apr 20, 2022
1 parent 3572b45 commit f5c9d86
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 29 deletions.
6 changes: 1 addition & 5 deletions api/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ type ServiceRegistration struct {
// This value can only be read; it is ignored on job submission.
Address string

// Advertise is the IP address or FQDN to use for this service during service
// registration.
Advertise string

// Port is the port number on which this service registration is bound. It
// is determined by a combination of factors on the client.
Port int
Expand Down Expand Up @@ -236,7 +232,7 @@ type Service struct {
EnableTagOverride bool `mapstructure:"enable_tag_override" hcl:"enable_tag_override,optional"`
PortLabel string `mapstructure:"port" hcl:"port,optional"`
AddressMode string `mapstructure:"address_mode" hcl:"address_mode,optional"`
Advertise string `hcl:"advertise,optional"`
Address string `hcl:"address,optional"`
Checks []ServiceCheck `hcl:"check,block"`
CheckRestart *CheckRestart `mapstructure:"check_restart" hcl:"check_restart,block"`
Connect *ConsulConnect `hcl:"connect,block"`
Expand Down
2 changes: 1 addition & 1 deletion client/serviceregistration/nsd/nsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (s *ServiceRegistrationHandler) generateNomadServiceRegistration(

// Determine the address to advertise based on the mode.
ip, port, err := serviceregistration.GetAddress(
serviceSpec.Advertise, addrMode, serviceSpec.PortLabel, workload.Networks,
serviceSpec.Address, addrMode, serviceSpec.PortLabel, workload.Networks,
workload.DriverNetwork, workload.Ports, workload.NetworkStatus)
if err != nil {
return nil, fmt.Errorf("unable to get address for service %q: %v", serviceSpec.Name, err)
Expand Down
2 changes: 1 addition & 1 deletion client/taskenv/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func InterpolateServices(taskEnv *TaskEnv, services []*structs.Service) []*struc

service.Name = taskEnv.ReplaceEnv(service.Name)
service.PortLabel = taskEnv.ReplaceEnv(service.PortLabel)
service.Advertise = taskEnv.ReplaceEnv(service.Advertise)
service.Address = taskEnv.ReplaceEnv(service.Address)
service.Tags = taskEnv.ParseAndReplace(service.Tags)
service.CanaryTags = taskEnv.ParseAndReplace(service.CanaryTags)
service.Meta = interpolateMapStringString(taskEnv, service.Meta)
Expand Down
16 changes: 10 additions & 6 deletions command/agent/consul/service_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -948,13 +948,11 @@ func (c *ServiceClient) serviceRegs(

// Determine the address to advertise based on the mode
ip, port, err := serviceregistration.GetAddress(
service.Advertise, addrMode, service.PortLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus)
service.Address, addrMode, service.PortLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus)
if err != nil {
return nil, fmt.Errorf("unable to get address for service %q: %v", service.Name, err)
}

c.logger.Info("SET ADDRESS", "ip", ip)

// Determine whether to use tags or canary_tags
var tags []string
if workload.Canary && len(service.CanaryTags) > 0 {
Expand Down Expand Up @@ -1075,13 +1073,19 @@ func (c *ServiceClient) checkRegs(serviceID string, service *structs.Service,

addrMode := check.AddressMode
if addrMode == "" {
// pre-#3380 compat
addrMode = structs.AddressModeHost
if service.Address != "" {
// if the service is using a custom address, enable the check
// to use that address
addrMode = structs.AddressModeAuto
} else {
// otherwise default to the host address
addrMode = structs.AddressModeHost
}
}

var err error
ip, port, err = serviceregistration.GetAddress(
"", addrMode, portLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus)
service.Address, addrMode, portLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus)
if err != nil {
return nil, fmt.Errorf("error getting address for check %q: %v", check.Name, err)
}
Expand Down
2 changes: 1 addition & 1 deletion command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ func ApiServicesToStructs(in []*api.Service, group bool) []*structs.Service {
CanaryTags: s.CanaryTags,
EnableTagOverride: s.EnableTagOverride,
AddressMode: s.AddressMode,
Advertise: s.Advertise,
Address: s.Address,
Meta: helper.CopyMapStringString(s.Meta),
CanaryMeta: helper.CopyMapStringString(s.CanaryMeta),
OnUpdate: s.OnUpdate,
Expand Down
8 changes: 4 additions & 4 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2514,7 +2514,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
CanaryTags: []string{"d", "e"},
EnableTagOverride: true,
PortLabel: "1234",
Advertise: "group.example.com",
Address: "group.example.com",
Meta: map[string]string{
"servicemeta": "foobar",
},
Expand Down Expand Up @@ -2607,7 +2607,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
CanaryTags: []string{"3", "4"},
EnableTagOverride: true,
PortLabel: "foo",
Advertise: "task.example.com",
Address: "task.example.com",
Meta: map[string]string{
"servicemeta": "foobar",
},
Expand Down Expand Up @@ -2912,7 +2912,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
EnableTagOverride: true,
PortLabel: "1234",
AddressMode: "auto",
Advertise: "group.example.com",
Address: "group.example.com",
Meta: map[string]string{
"servicemeta": "foobar",
},
Expand Down Expand Up @@ -3006,7 +3006,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
EnableTagOverride: true,
PortLabel: "foo",
AddressMode: "auto",
Advertise: "task.example.com",
Address: "task.example.com",
Meta: map[string]string{
"servicemeta": "foobar",
},
Expand Down
9 changes: 8 additions & 1 deletion command/service_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (s *ServiceInfoCommand) formatOutput(jobIDs []string, jobServices map[strin
outputTable = append(outputTable, fmt.Sprintf(
"%s|%s|[%s]|%s|%s",
service.JobID,
fmt.Sprintf("%s:%v", service.Address, service.Port),
formatAddress(service.Address, service.Port),
strings.Join(service.Tags, ","),
limit(service.NodeID, shortId),
limit(service.AllocID, shortId),
Expand All @@ -173,6 +173,13 @@ func (s *ServiceInfoCommand) formatOutput(jobIDs []string, jobServices map[strin
s.Ui.Output(formatList(outputTable))
}

func formatAddress(address string, port int) string {
if port == 0 {
return address
}
return fmt.Sprintf("%s:%d", address, port)
}

// formatOutput produces the verbose output of service registration info for a
// specific service by its name.
func (s *ServiceInfoCommand) formatVerboseOutput(jobIDs []string, jobServices map[string][]*api.ServiceRegistration) {
Expand Down
6 changes: 3 additions & 3 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5642,7 +5642,7 @@ func TestTaskDiff(t *testing.T) {
Name: "foo",
PortLabel: "bar",
AddressMode: "driver",
Advertise: "a.example.com",
Address: "a.example.com",
TaskName: "task1",
},
},
Expand Down Expand Up @@ -7444,7 +7444,7 @@ func TestServicesDiff(t *testing.T) {
Name: "webapp",
PortLabel: "http",
AddressMode: "host",
Advertise: "a.example.com",
Address: "a.example.com",
EnableTagOverride: true,
Tags: []string{"prod"},
CanaryTags: []string{"canary"},
Expand All @@ -7455,7 +7455,7 @@ func TestServicesDiff(t *testing.T) {
Name: "webapp-2",
PortLabel: "https",
AddressMode: "alloc",
Advertise: "b.example.com",
Address: "b.example.com",
EnableTagOverride: false,
Tags: []string{"prod", "dev"},
CanaryTags: []string{"qa"},
Expand Down
8 changes: 4 additions & 4 deletions nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ type Service struct {

// Advertise enables explicitly setting and advertise address used in service
// registration. AddressMode must be "auto" if Advertise is set.
Advertise string
Address string

// EnableTagOverride will disable Consul's anti-entropy mechanism for the
// tags of this service. External updates to the service definition via
Expand Down Expand Up @@ -583,7 +583,7 @@ func (s *Service) Validate() error {
switch s.AddressMode {
case "", AddressModeAuto:
case AddressModeHost, AddressModeDriver, AddressModeAlloc:
if s.Advertise != "" {
if s.Address != "" {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Service address_mode must be %q if address is set", AddressModeAuto))
}
default:
Expand Down Expand Up @@ -692,7 +692,7 @@ func (s *Service) Hash(allocID, taskName string, canary bool) string {
hashString(h, s.Name)
hashString(h, s.PortLabel)
hashString(h, s.AddressMode)
hashString(h, s.Advertise)
hashString(h, s.Address)
hashTags(h, s.Tags)
hashTags(h, s.CanaryTags)
hashBool(h, canary, "Canary")
Expand Down Expand Up @@ -775,7 +775,7 @@ func (s *Service) Equals(o *Service) bool {
return false
}

if s.Advertise != o.Advertise {
if s.Address != o.Address {
return false
}

Expand Down
6 changes: 3 additions & 3 deletions nomad/structs/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func TestService_Hash(t *testing.T) {
// on comparing the resulting hash output

t.Run("mod advertise", func(t *testing.T) {
try(t, func(s *svc) { s.Advertise = "example.com" })
try(t, func(s *svc) { s.Address = "example.com" })
})

t.Run("mod name", func(t *testing.T) {
Expand Down Expand Up @@ -1586,7 +1586,7 @@ func TestService_Validate(t *testing.T) {

func TestService_Advertise(t *testing.T) {
try := func(mode, advertise string, exp error) {
s := &Service{Name: "s1", Provider: "consul", AddressMode: mode, Advertise: advertise}
s := &Service{Name: "s1", Provider: "consul", AddressMode: mode, Address: advertise}
result := s.Validate()
if exp == nil {
require.NoError(t, result)
Expand Down Expand Up @@ -1636,7 +1636,7 @@ func TestService_Equals(t *testing.T) {
o.Name = "diff"
assertDiff()

o.Advertise = "diff"
o.Address = "diff"
assertDiff()

o.PortLabel = "diff"
Expand Down

0 comments on commit f5c9d86

Please sign in to comment.