Skip to content

Commit

Permalink
consul: fixup expected consul tagged_addresses when using ipv6
Browse files Browse the repository at this point in the history
This PR is a continuation of #14917, where we missed the ipv6 cases.

Consul auto-inserts tagged_addresses for keys
- lan_ipv4
- wan_ipv4
- lan_ipv6
- wan_ipv6

even though the service registration coming from Nomad does not contain such
elements. When doing the differential between services Nomad expects to be
registered vs. the services actually registered into Consul, we must first
purge these automatically inserted tagged_addresses if they do not exist in
the Nomad view of the Consul service.
  • Loading branch information
shoenig committed Nov 28, 2022
1 parent 79afeee commit fa3a2b7
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 0 deletions.
3 changes: 3 additions & 0 deletions .changelog/15411.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
consul: Fixed a bug where services would continuously re-register when using ipv6
```
6 changes: 6 additions & 0 deletions command/agent/consul/service_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ func maybeTweakTaggedAddresses(wanted *api.AgentServiceRegistration, existing *a
if _, exists := wanted.TaggedAddresses["wan_ipv4"]; !exists {
delete(existing.TaggedAddresses, "wan_ipv4")
}
if _, exists := wanted.TaggedAddresses["lan_ipv6"]; !exists {
delete(existing.TaggedAddresses, "lan_ipv6")
}
if _, exists := wanted.TaggedAddresses["wan_ipv6"]; !exists {
delete(existing.TaggedAddresses, "wan_ipv6")
}
}
}

Expand Down
107 changes: 107 additions & 0 deletions command/agent/consul/service_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,115 @@ import (
"github.com/hashicorp/nomad/nomad/structs"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"
"golang.org/x/exp/maps"
)

func TestSyncLogic_maybeTweakTaggedAddresses(t *testing.T) {
ci.Parallel(t)

cases := []struct {
name string
wanted map[string]api.ServiceAddress
existing map[string]api.ServiceAddress
id string
exp []string
}{
{
name: "not managed by nomad",
id: "_nomad-other-hello",
wanted: map[string]api.ServiceAddress{
// empty
},
existing: map[string]api.ServiceAddress{
"lan_ipv4": {},
"wan_ipv4": {},
"custom": {},
},
exp: []string{"lan_ipv4", "wan_ipv4", "custom"},
},
{
name: "remove defaults",
id: "_nomad-task-hello",
wanted: map[string]api.ServiceAddress{
"lan_custom": {},
"wan_custom": {},
},
existing: map[string]api.ServiceAddress{
"lan_ipv4": {},
"wan_ipv4": {},
"lan_ipv6": {},
"wan_ipv6": {},
"lan_custom": {},
"wan_custom": {},
},
exp: []string{"lan_custom", "wan_custom"},
},
{
name: "overridden defaults",
id: "_nomad-task-hello",
wanted: map[string]api.ServiceAddress{
"lan_ipv4": {},
"wan_ipv4": {},
"lan_ipv6": {},
"wan_ipv6": {},
"custom": {},
},
existing: map[string]api.ServiceAddress{
"lan_ipv4": {},
"wan_ipv4": {},
"lan_ipv6": {},
"wan_ipv6": {},
"custom": {},
},
exp: []string{"lan_ipv4", "wan_ipv4", "lan_ipv6", "wan_ipv6", "custom"},
},
{
name: "applies to nomad client",
id: "_nomad-client-12345",
wanted: map[string]api.ServiceAddress{
"custom": {},
},
existing: map[string]api.ServiceAddress{
"lan_ipv4": {},
"wan_ipv4": {},
"lan_ipv6": {},
"wan_ipv6": {},
"custom": {},
},
exp: []string{"custom"},
},
{
name: "applies to nomad server",
id: "_nomad-server-12345",
wanted: map[string]api.ServiceAddress{
"custom": {},
},
existing: map[string]api.ServiceAddress{
"lan_ipv4": {},
"wan_ipv4": {},
"lan_ipv6": {},
"wan_ipv6": {},
"custom": {},
},
exp: []string{"custom"},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
asr := &api.AgentServiceRegistration{
ID: tc.id,
TaggedAddresses: maps.Clone(tc.wanted),
}
as := &api.AgentService{
TaggedAddresses: maps.Clone(tc.existing),
}
maybeTweakTaggedAddresses(asr, as)
must.MapContainsKeys(t, as.TaggedAddresses, tc.exp)
})
}
}

func TestSyncLogic_agentServiceUpdateRequired(t *testing.T) {
ci.Parallel(t)

Expand Down

0 comments on commit fa3a2b7

Please sign in to comment.