Merge pull request #1357 from hashicorp/b-agent-spam

Prevents agents from considering Raft information when doing sync checks.

command/agent/local.go

@@ -3,7 +3,6 @@ package agent
 import (
 	"fmt"
 	"log"
-	"reflect"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -385,7 +384,7 @@ func (l *localState) setSyncState() error {
 		if existing.EnableTagOverride {
 			existing.Tags = service.Tags
 		}
-		equal := reflect.DeepEqual(existing, service)
+		equal := existing.IsSame(service)
 		l.serviceStatus[id] = syncStatus{inSync: equal}
 	}
 
@@ -419,13 +418,13 @@ func (l *localState) setSyncState() error {
 		// If our definition is different, we need to update it
 		var equal bool
 		if l.config.CheckUpdateInterval == 0 {
-			equal = reflect.DeepEqual(existing, check)
+			equal = existing.IsSame(check)
 		} else {
 			eCopy := new(structs.HealthCheck)
 			*eCopy = *existing
 			eCopy.Output = ""
 			check.Output = ""
-			equal = reflect.DeepEqual(eCopy, check)
+			equal = eCopy.IsSame(check)
 		}
 
 		// Update the status

consul/state/state_store.go

@@ -1079,7 +1079,7 @@ func (s *StateStore) ensureCheckTxn(tx *memdb.Txn, idx uint64, watches *DumbWatc
 
 	// Persist the check registration in the db.
 	if err := tx.Insert("checks", hc); err != nil {
-		return fmt.Errorf("failed inserting service: %s", err)
+		return fmt.Errorf("failed inserting check: %s", err)
 	}
 	if err := tx.Insert("index", &IndexEntry{"checks", idx}); err != nil {
 		return fmt.Errorf("failed updating index: %s", err)

consul/structs/structs.go

@@ -3,6 +3,7 @@ package structs
 import (
 	"bytes"
 	"fmt"
+	"reflect"
 	"time"
 
 	"github.com/hashicorp/consul/acl"
@@ -318,6 +319,23 @@ type NodeService struct {
 	RaftIndex
 }
 
+// IsSame checks if one NodeService is the same as another, without looking
+// at the Raft information (that's why we didn't call it IsEqual). This is
+// useful for seeing if an update would be idempotent for all the functional
+// parts of the structure.
+func (s *NodeService) IsSame(other *NodeService) bool {
+	if s.ID != other.ID ||
+		s.Service != other.Service ||
+		!reflect.DeepEqual(s.Tags, other.Tags) ||
+		s.Address != other.Address ||
+		s.Port != other.Port ||
+		s.EnableTagOverride != other.EnableTagOverride {
+		return false
+	}
+
+	return true
+}
+
 // ToServiceNode converts the given node service to a service node.
 func (s *NodeService) ToServiceNode(node, address string) *ServiceNode {
 	return &ServiceNode{
@@ -354,6 +372,26 @@ type HealthCheck struct {
 
 	RaftIndex
 }
+
+// IsSame checks if one HealthCheck is the same as another, without looking
+// at the Raft information (that's why we didn't call it IsEqual). This is
+// useful for seeing if an update would be idempotent for all the functional
+// parts of the structure.
+func (c *HealthCheck) IsSame(other *HealthCheck) bool {
+	if c.Node != other.Node ||
+		c.CheckID != other.CheckID ||
+		c.Name != other.Name ||
+		c.Status != other.Status ||
+		c.Notes != other.Notes ||
+		c.Output != other.Output ||
+		c.ServiceID != other.ServiceID ||
+		c.ServiceName != other.ServiceName {
+		return false
+	}
+
+	return true
+}
+
 type HealthChecks []*HealthCheck
 
 // CheckServiceNode is used to provide the node, its service

consul/structs/structs_test.go

@@ -95,6 +95,120 @@ func TestStructs_ServiceNode_Conversions(t *testing.T) {
 	}
 }
 
+func TestStructs_NodeService_IsSame(t *testing.T) {
+	ns := &NodeService{
+		ID:                "node1",
+		Service:           "theservice",
+		Tags:              []string{"foo", "bar"},
+		Address:           "127.0.0.1",
+		Port:              1234,
+		EnableTagOverride: true,
+	}
+	if !ns.IsSame(ns) {
+		t.Fatalf("should be equal to itself")
+	}
+
+	other := &NodeService{
+		ID:                "node1",
+		Service:           "theservice",
+		Tags:              []string{"foo", "bar"},
+		Address:           "127.0.0.1",
+		Port:              1234,
+		EnableTagOverride: true,
+		RaftIndex: RaftIndex{
+			CreateIndex: 1,
+			ModifyIndex: 2,
+		},
+	}
+	if !ns.IsSame(other) || !other.IsSame(ns) {
+		t.Fatalf("should not care about Raft fields")
+	}
+
+	check := func(twiddle, restore func()) {
+		if !ns.IsSame(other) || !other.IsSame(ns) {
+			t.Fatalf("should be the same")
+		}
+
+		twiddle()
+		if ns.IsSame(other) || other.IsSame(ns) {
+			t.Fatalf("should not be the same")
+		}
+
+		restore()
+		if !ns.IsSame(other) || !other.IsSame(ns) {
+			t.Fatalf("should be the same")
+		}
+	}
+
+	check(func() { other.ID = "XXX" }, func() { other.ID = "node1" })
+	check(func() { other.Service = "XXX" }, func() { other.Service = "theservice" })
+	check(func() { other.Tags = nil }, func() { other.Tags = []string{"foo", "bar"} })
+	check(func() { other.Tags = []string{"foo"} }, func() { other.Tags = []string{"foo", "bar"} })
+	check(func() { other.Address = "XXX" }, func() { other.Address = "127.0.0.1" })
+	check(func() { other.Port = 9999 }, func() { other.Port = 1234 })
+	check(func() { other.EnableTagOverride = false }, func() { other.EnableTagOverride = true })
+}
+
+func TestStructs_HealthCheck_IsSame(t *testing.T) {
+	hc := &HealthCheck{
+		Node:        "node1",
+		CheckID:     "check1",
+		Name:        "thecheck",
+		Status:      HealthPassing,
+		Notes:       "it's all good",
+		Output:      "lgtm",
+		ServiceID:   "service1",
+		ServiceName: "theservice",
+	}
+	if !hc.IsSame(hc) {
+		t.Fatalf("should be equal to itself")
+	}
+
+	other := &HealthCheck{
+		Node:        "node1",
+		CheckID:     "check1",
+		Name:        "thecheck",
+		Status:      HealthPassing,
+		Notes:       "it's all good",
+		Output:      "lgtm",
+		ServiceID:   "service1",
+		ServiceName: "theservice",
+		RaftIndex: RaftIndex{
+			CreateIndex: 1,
+			ModifyIndex: 2,
+		},
+	}
+	if !hc.IsSame(other) || !other.IsSame(hc) {
+		t.Fatalf("should not care about Raft fields")
+	}
+
+	check := func(field *string) {
+		if !hc.IsSame(other) || !other.IsSame(hc) {
+			t.Fatalf("should be the same")
+		}
+
+		old := *field
+		*field = "XXX"
+		if hc.IsSame(other) || other.IsSame(hc) {
+			t.Fatalf("should not be the same")
+		}
+		*field = old
+
+		if !hc.IsSame(other) || !other.IsSame(hc) {
+			t.Fatalf("should be the same")
+		}
+	}
+
+	check(&other.Node)
+	check(&other.CheckID)
+	check(&other.Name)
+	check(&other.Status)
+	check(&other.Notes)
+	check(&other.Output)
+	check(&other.ServiceID)
+	check(&other.ServiceName)
+}
+
 func TestStructs_DirEntry_Clone(t *testing.T) {
 	e := &DirEntry{
 		LockIndex: 5,