Improve logging when deregistering a nonexistent service (#2492)

Log a warning instead of a success message when attempting to deregister a nonexistent service. In Consul 0.8 this can be changed to giving an error outright, but for now we can keep the idempotent delete behavior.

command/agent/agent.go

@@ -906,7 +906,14 @@ func (a *Agent) RemoveService(serviceID string, persist bool) error {
 	}
 
 	// Remove service immediately
-	a.state.RemoveService(serviceID)
+	err := a.state.RemoveService(serviceID)
+
+	// TODO: Return the error instead of just logging here in Consul 0.8
+	// For now, keep the current idempotent behavior on deleting a nonexistent service
+	if err != nil {
+		a.logger.Printf("[WARN] agent: Failed to deregister service %q: %s", serviceID, err)
+		return nil
+	}
 
 	// Remove the service from the data dir
 	if persist {

command/agent/agent_test.go

@@ -935,6 +935,11 @@ func TestAgent_PurgeService(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
+	// Re-add the service
+	if err := agent.AddService(svc, nil, true, ""); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
 	// Removed
 	if err := agent.RemoveService(svc.ID, true); err != nil {
 		t.Fatalf("err: %s", err)

command/agent/local.go

@@ -170,14 +170,20 @@ func (l *localState) AddService(service *structs.NodeService, token string) {
 
 // RemoveService is used to remove a service entry from the local state.
 // The agent will make a best effort to ensure it is deregistered
-func (l *localState) RemoveService(serviceID string) {
+func (l *localState) RemoveService(serviceID string) error {
 	l.Lock()
 	defer l.Unlock()
 
-	delete(l.services, serviceID)
-	delete(l.serviceTokens, serviceID)
-	l.serviceStatus[serviceID] = syncStatus{inSync: false}
-	l.changeMade()
+	if _, ok := l.services[serviceID]; ok {
+		delete(l.services, serviceID)
+		delete(l.serviceTokens, serviceID)
+		l.serviceStatus[serviceID] = syncStatus{inSync: false}
+		l.changeMade()
+	} else {
+		return fmt.Errorf("Service does not exist")
+	}
+
+	return nil
 }
 
 // Services returns the locally registered services that the

command/agent/local_test.go

@@ -1074,6 +1074,10 @@ func TestAgent_serviceTokens(t *testing.T) {
 	l := new(localState)
 	l.Init(config, nil)
 
+	l.AddService(&structs.NodeService{
+		ID: "redis",
+	}, "")
+
 	// Returns default when no token is set
 	if token := l.ServiceToken("redis"); token != "default" {
 		t.Fatalf("bad: %s", token)