Fix: service LocallyRegisteredAsSidecar property is not persisted

When a service is deregistered, we check whever matching services were
registered as sidecar along with it and deregister them as well.
To determine if a service is indeed a sidecar we check the
structs.ServiceNode.LocallyRegisteredAsSidecar property. However, to
avoid interal API leakage, it is excluded from JSON serialization,
meaning it is not persisted to disk either.
When the agent is restarted, this property lost and sidecars are no
longer deregistered along with their parent service.
To fix this, we now specifically save this property in the persisted
service file.
This commit is contained in:
Mathilde Gilles 2020-10-12 21:45:08 +02:00
parent 52451cf846
commit 1c8369b3c3
3 changed files with 85 additions and 3 deletions

3
.changelog/8924.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
connect: fix connect sidecars registered via the API not being automatically deregistered with their parent service after an agent restart by persisting the LocallyRegisteredAsSidecar property.
```

View File

@ -1701,6 +1701,11 @@ type persistedService struct {
Token string Token string
Service *structs.NodeService Service *structs.NodeService
Source string Source string
// whether this service was registered as a sidecar, see structs.NodeService
// we store this field here because it is excluded from json serialization
// to exclude it from API output, but we need it to properly deregister
// persisted sidecars.
LocallyRegisteredAsSidecar bool `json:",omitempty"`
} }
// persistService saves a service definition to a JSON file in the data dir // persistService saves a service definition to a JSON file in the data dir
@ -1712,6 +1717,7 @@ func (a *Agent) persistService(service *structs.NodeService, source configSource
Token: a.State.ServiceToken(service.CompoundServiceID()), Token: a.State.ServiceToken(service.CompoundServiceID()),
Service: service, Service: service,
Source: source.String(), Source: source.String(),
LocallyRegisteredAsSidecar: service.LocallyRegisteredAsSidecar,
} }
encoded, err := json.Marshal(wrapped) encoded, err := json.Marshal(wrapped)
if err != nil { if err != nil {
@ -3160,6 +3166,10 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI
continue continue
} }
} }
// Restore LocallyRegisteredAsSidecar, see persistedService.LocallyRegisteredAsSidecar
p.Service.LocallyRegisteredAsSidecar = p.LocallyRegisteredAsSidecar
serviceID := p.Service.CompoundServiceID() serviceID := p.Service.CompoundServiceID()
source, ok := ConfigSourceFromName(p.Source) source, ok := ConfigSourceFromName(p.Source)

View File

@ -2442,6 +2442,75 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) {
require.Equal(t, expected, result) require.Equal(t, expected, result)
} }
func TestAgent_DeregisterPersistedSidecarAfterRestart(t *testing.T) {
t.Parallel()
nodeID := NodeID()
a := StartTestAgent(t, TestAgent{
HCL: `
node_id = "` + nodeID + `"
node_name = "Node ` + nodeID + `"
server = false
bootstrap = false
enable_central_service_config = false
`})
defer a.Shutdown()
srv := &structs.NodeService{
ID: "svc",
Service: "svc",
Weights: &structs.Weights{
Passing: 2,
Warning: 1,
},
Tags: []string{"tag2"},
Port: 8200,
EnterpriseMeta: *structs.DefaultEnterpriseMeta(),
Connect: structs.ServiceConnect{
SidecarService: &structs.ServiceDefinition{},
},
}
connectSrv, _, _, err := a.sidecarServiceFromNodeService(srv, "")
require.NoError(t, err)
// First persist the check
err = a.AddService(srv, nil, true, "", ConfigSourceLocal)
require.NoError(t, err)
err = a.AddService(connectSrv, nil, true, "", ConfigSourceLocal)
require.NoError(t, err)
// check both services were registered
require.NotNil(t, a.State.Service(srv.CompoundServiceID()))
require.NotNil(t, a.State.Service(connectSrv.CompoundServiceID()))
a.Shutdown()
// Start again with the check registered in config
a2 := StartTestAgent(t, TestAgent{
Name: "Agent2",
DataDir: a.DataDir,
HCL: `
node_id = "` + nodeID + `"
node_name = "Node ` + nodeID + `"
server = false
bootstrap = false
enable_central_service_config = false
`})
defer a2.Shutdown()
// check both services were restored
require.NotNil(t, a2.State.Service(srv.CompoundServiceID()))
require.NotNil(t, a2.State.Service(connectSrv.CompoundServiceID()))
err = a2.RemoveService(srv.CompoundServiceID())
require.NoError(t, err)
// check both services were deregistered
require.Nil(t, a2.State.Service(srv.CompoundServiceID()))
require.Nil(t, a2.State.Service(connectSrv.CompoundServiceID()))
}
func TestAgent_loadChecks_token(t *testing.T) { func TestAgent_loadChecks_token(t *testing.T) {
t.Parallel() t.Parallel()
a := NewTestAgent(t, ` a := NewTestAgent(t, `