From 5dc8875bd375644c58c9868712a77e4f517b9f2e Mon Sep 17 00:00:00 2001
From: Aestek <t.gilles@criteo.com>
Date: Fri, 17 Jan 2020 14:26:53 +0100
Subject: [PATCH] agent: do not deregister service checks twice (#6168)

Deregistering a service from the catalog automatically deregisters its
checks, however the agent still performs a deregister call for each
service checks even after the service has been deregistered.
With ACLs enabled this results in logs like:
"message:consul: "Catalog.Deregister" RPC failed to server
server_ip:8300: rpc error making call: rpc error making call: Unknown
check 'check_id'"
This change removes associated checks from the agent state when
deregistering a service, which results in less calls to the servers and
supresses the error logs.
---
 agent/local/state.go | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/agent/local/state.go b/agent/local/state.go
index 228c879dc9..b44fcad88e 100644
--- a/agent/local/state.go
+++ b/agent/local/state.go
@@ -1091,6 +1091,12 @@ func (l *State) deleteService(key structs.ServiceID) error {
 	switch {
 	case err == nil || strings.Contains(err.Error(), "Unknown service"):
 		delete(l.services, key)
+		// service deregister also deletes associated checks
+		for _, c := range l.checks {
+			if c.Deleted && c.Check.ServiceID == key.ID {
+				l.pruneCheck(c.Check.CompoundCheckID())
+			}
+		}
 		l.logger.Printf("[INFO] agent: Deregistered service %q", key.ID)
 		return nil
 
@@ -1125,11 +1131,7 @@ func (l *State) deleteCheck(key structs.CheckID) error {
 	err := l.Delegate.RPC("Catalog.Deregister", &req, &out)
 	switch {
 	case err == nil || strings.Contains(err.Error(), "Unknown check"):
-		c := l.checks[key]
-		if c != nil && c.DeferCheck != nil {
-			c.DeferCheck.Stop()
-		}
-		delete(l.checks, key)
+		l.pruneCheck(key)
 		l.logger.Printf("[INFO] agent: Deregistered check %q", key.String())
 		return nil
 
@@ -1147,6 +1149,14 @@ func (l *State) deleteCheck(key structs.CheckID) error {
 	}
 }
 
+func (l *State) pruneCheck(id structs.CheckID) {
+	c := l.checks[id]
+	if c != nil && c.DeferCheck != nil {
+		c.DeferCheck.Stop()
+	}
+	delete(l.checks, id)
+}
+
 // syncService is used to sync a service to the server
 func (l *State) syncService(key structs.ServiceID) error {
 	// If the service has associated checks that are out of sync,