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

Fix: service LocallyRegisteredAsSidecar property is not persisted #8924

Merged
Merged
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
3 changes: 3 additions & 0 deletions .changelog/8924.txt
Original file line number Diff line number Diff line change
@@ -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.
```
16 changes: 13 additions & 3 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,11 @@ type persistedService struct {
Token string
Service *structs.NodeService
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
Expand All @@ -1709,9 +1714,10 @@ func (a *Agent) persistService(service *structs.NodeService, source configSource
svcPath := filepath.Join(a.config.DataDir, servicesDir, svcID.StringHash())

wrapped := persistedService{
Token: a.State.ServiceToken(service.CompoundServiceID()),
Service: service,
Source: source.String(),
Token: a.State.ServiceToken(service.CompoundServiceID()),
Service: service,
Source: source.String(),
LocallyRegisteredAsSidecar: service.LocallyRegisteredAsSidecar,
}
encoded, err := json.Marshal(wrapped)
if err != nil {
Expand Down Expand Up @@ -3160,6 +3166,10 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI
continue
}
}

// Restore LocallyRegisteredAsSidecar, see persistedService.LocallyRegisteredAsSidecar
p.Service.LocallyRegisteredAsSidecar = p.LocallyRegisteredAsSidecar

serviceID := p.Service.CompoundServiceID()

source, ok := ConfigSourceFromName(p.Source)
Expand Down
69 changes: 69 additions & 0 deletions agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2442,6 +2442,75 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) {
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) {
t.Parallel()
a := NewTestAgent(t, `
Expand Down