agent: maintenance logging + unique service check IDs

command/agent/agent.go

@@ -31,7 +31,7 @@ const (
 		"and try again."
 
 	// The ID of the faux health checks for maintenance mode
-	serviceMaintCheckID = "_service_maintenance"
+	serviceMaintCheckPrefix = "_service_maintenance"
 	nodeMaintCheckID        = "_node_maintenenace"
 )
 
@@ -1000,6 +1000,11 @@ func (a *Agent) unloadChecks() error {
 	return nil
 }
 
+// serviceMaintCheckID returns the ID of a given service's maintenance check
+func serviceMaintCheckID(serviceID string) string {
+	return fmt.Sprintf("%s:%s", serviceMaintCheckPrefix, serviceID)
+}
+
 // EnableServiceMaintenance will register a false health check against the given
 // service ID with critical status. This will exclude the service from queries.
 func (a *Agent) EnableServiceMaintenance(serviceID string) error {
@@ -1008,15 +1013,16 @@ func (a *Agent) EnableServiceMaintenance(serviceID string) error {
 		return fmt.Errorf("No service registered with ID %q", serviceID)
 	}
 
-	// Ensure maintenance mode is not already enabled
-	if _, ok := a.state.Checks()[serviceMaintCheckID]; ok {
+	// Check if maintenance mode is not already enabled
+	checkID := serviceMaintCheckID(serviceID)
+	if _, ok := a.state.Checks()[checkID]; ok {
 		return nil
 	}
 
 	// Create and register the critical health check
 	check := &structs.HealthCheck{
 		Node:        a.config.NodeName,
-		CheckID:     serviceMaintCheckID,
+		CheckID:     checkID,
 		Name:        "Service Maintenance Mode",
 		Notes:       "Maintenance mode is enabled for this service",
 		ServiceID:   service.ID,
@@ -1024,6 +1030,7 @@ func (a *Agent) EnableServiceMaintenance(serviceID string) error {
 		Status:      structs.HealthCritical,
 	}
 	a.AddCheck(check, nil, true)
+	a.logger.Printf("[INFO] agent: service %q entered maintenance mode", serviceID)
 
 	return nil
 }
@@ -1035,8 +1042,16 @@ func (a *Agent) DisableServiceMaintenance(serviceID string) error {
 		return fmt.Errorf("No service registered with ID %q", serviceID)
 	}
 
+	// Check if maintenance mode is enabled
+	checkID := serviceMaintCheckID(serviceID)
+	if _, ok := a.state.Checks()[checkID]; !ok {
+		return nil
+	}
+
 	// Deregister the maintenance check
-	a.RemoveCheck(serviceMaintCheckID, true)
+	a.RemoveCheck(checkID, true)
+	a.logger.Printf("[INFO] agent: service %q left maintenance mode", serviceID)
+
 	return nil
 }
 
@@ -1056,9 +1071,14 @@ func (a *Agent) EnableNodeMaintenance() {
 		Status:  structs.HealthCritical,
 	}
 	a.AddCheck(check, nil, true)
+	a.logger.Printf("[INFO] agent: node entered maintenance mode")
 }
 
 // DisableNodeMaintenance removes a node from maintenance mode
 func (a *Agent) DisableNodeMaintenance() {
+	if _, ok := a.state.Checks()[nodeMaintCheckID]; !ok {
+		return
+	}
 	a.RemoveCheck(nodeMaintCheckID, true)
+	a.logger.Printf("[INFO] agent: node left maintenance mode")
 }

command/agent/agent_endpoint_test.go

@@ -567,7 +567,8 @@ func TestHTTPAgent_EnableServiceMaintenance(t *testing.T) {
 	}
 
 	// Ensure the maintenance check was registered
-	if _, ok := srv.agent.state.Checks()[serviceMaintCheckID]; !ok {
+	checkID := serviceMaintCheckID("test")
+	if _, ok := srv.agent.state.Checks()[checkID]; !ok {
 		t.Fatalf("should have registered maintenance check")
 	}
 }
@@ -603,7 +604,8 @@ func TestHTTPAgent_DisableServiceMaintenance(t *testing.T) {
 	}
 
 	// Ensure the maintenance check was removed
-	if _, ok := srv.agent.state.Checks()[serviceMaintCheckID]; ok {
+	checkID := serviceMaintCheckID("test")
+	if _, ok := srv.agent.state.Checks()[checkID]; ok {
 		t.Fatalf("should have removed maintenance check")
 	}
 }

command/agent/agent_test.go

@@ -806,7 +806,8 @@ func TestAgent_ServiceMaintenanceMode(t *testing.T) {
 	}
 
 	// Make sure the critical health check was added
-	if _, ok := agent.state.Checks()[serviceMaintCheckID]; !ok {
+	checkID := serviceMaintCheckID("redis")
+	if _, ok := agent.state.Checks()[checkID]; !ok {
 		t.Fatalf("should have registered critical maintenance check")
 	}
 
@@ -816,7 +817,7 @@ func TestAgent_ServiceMaintenanceMode(t *testing.T) {
 	}
 
 	// Ensure the check was deregistered
-	if _, ok := agent.state.Checks()[serviceMaintCheckID]; ok {
+	if _, ok := agent.state.Checks()[checkID]; ok {
 		t.Fatalf("should have deregistered maintenance check")
 	}
 }