From 2ed5b108c580a0c0f9b71644520112e4c183c920 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@hashicorp.com>
Date: Thu, 22 Oct 2020 13:26:55 -0400
Subject: [PATCH] Merge pull request #8924 from
 ShimmerGlass/fix-sidecar-deregister-after-restart

Fix: service LocallyRegisteredAsSidecar property is not persisted
---
 .changelog/8924.txt |  3 ++
 agent/agent.go      | 16 +++++++++--
 agent/agent_test.go | 69 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 3 deletions(-)
 create mode 100644 .changelog/8924.txt

diff --git a/.changelog/8924.txt b/.changelog/8924.txt
new file mode 100644
index 0000000000..1832faa75a
--- /dev/null
+++ b/.changelog/8924.txt
@@ -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.
+```
diff --git a/agent/agent.go b/agent/agent.go
index d85583f9e6..66364f4a08 100644
--- a/agent/agent.go
+++ b/agent/agent.go
@@ -1731,6 +1731,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
@@ -1739,9 +1744,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 {
@@ -3190,6 +3196,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)
diff --git a/agent/agent_test.go b/agent/agent_test.go
index 0851c0c9c7..eeecf7963f 100644
--- a/agent/agent_test.go
+++ b/agent/agent_test.go
@@ -2441,6 +2441,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, `