Merge branch 'fix-check-removal-1.3.1' into 1.3.1-criteo

criteo-forks · Mar 12, 2019 · 6becd97 · 6becd97
2 parents 685ec3f + 820641e
commit 6becd97
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 4 deletions.
diff --git a/agent/agent.go b/agent/agent.go
@@ -2041,7 +2041,10 @@ func (a *Agent) removeServiceLocked(serviceID string, persist bool) error {
 
 	checks := a.State.Checks()
 	var checkIDs []types.CheckID
-	for id := range checks {
+	for id, check := range checks {
+		if check.ServiceID != serviceID {
+			continue
+		}
 		checkIDs = append(checkIDs, id)
 	}
 

diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -583,6 +583,7 @@ func TestAgent_RemoveService(t *testing.T) {
 
 	// Removing a service with multiple checks works
 	{
+		// add a service to remove
 		srv := &structs.NodeService{
 			ID:      "redis",
 			Service: "redis",
@@ -596,6 +597,20 @@ func TestAgent_RemoveService(t *testing.T) {
 			t.Fatalf("err: %v", err)
 		}
 
+		// add another service that wont be affected
+		srv = &structs.NodeService{
+			ID:      "mysql",
+			Service: "mysql",
+			Port:    3306,
+		}
+		chkTypes = []*structs.CheckType{
+			&structs.CheckType{TTL: time.Minute},
+			&structs.CheckType{TTL: 30 * time.Second},
+		}
+		if err := a.AddService(srv, chkTypes, false, "", ConfigSourceLocal); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
 		// Remove the service
 		if err := a.RemoveService("redis", false); err != nil {
 			t.Fatalf("err: %v", err)
@@ -614,13 +629,33 @@ func TestAgent_RemoveService(t *testing.T) {
 			t.Fatalf("check redis:2 should be removed")
 		}
 
-		// Ensure a TTL is setup
+		// Ensure the redis checks are removed
 		if _, ok := a.checkTTLs["service:redis:1"]; ok {
 			t.Fatalf("check ttl for redis:1 should be removed")
 		}
+		if check := a.State.Check(types.CheckID("service:redis:1")); check != nil {
+			t.Fatalf("check ttl for redis:1 should be removed")
+		}
 		if _, ok := a.checkTTLs["service:redis:2"]; ok {
 			t.Fatalf("check ttl for redis:2 should be removed")
 		}
+		if check := a.State.Check(types.CheckID("service:redis:2")); check != nil {
+			t.Fatalf("check ttl for redis:2 should be removed")
+		}
+
+		// check the mysql service is unnafected
+		if _, ok := a.checkTTLs["service:mysql:1"]; !ok {
+			t.Fatalf("check ttl for mysql:1 should not be removed")
+		}
+		if check := a.State.Check(types.CheckID("service:mysql:1")); check == nil {
+			t.Fatalf("check ttl for mysql:1 should not be removed")
+		}
+		if _, ok := a.checkTTLs["service:mysql:2"]; !ok {
+			t.Fatalf("check ttl for mysql:2 should not be removed")
+		}
+		if check := a.State.Check(types.CheckID("service:mysql:2")); check == nil {
+			t.Fatalf("check ttl for mysql:2 should not be removed")
+		}
 	}
 }
 
@@ -2356,8 +2391,8 @@ func TestAgent_Service_Reap(t *testing.T) {
 	}
 	chkTypes := []*structs.CheckType{
 		&structs.CheckType{
-			Status:                         api.HealthPassing,
-			TTL:                            25 * time.Millisecond,
+			Status: api.HealthPassing,
+			TTL:    25 * time.Millisecond,
 			DeregisterCriticalServiceAfter: 200 * time.Millisecond,
 		},
 	}