Improve drift detection.

CHANGELOG.md

@@ -15,6 +15,11 @@ The format is based on [Keep a Changelog], and this project adheres to
 ### Fixed
 
 - Always use TCP (not UDP) for DNS-SD reconcilation queries to avoid truncated responses
+- Fixed false-negative reported for the `Discoverable` condition
+
+### Changed
+
+- Added brief description of the drifted values when `Discoverable` is `False` due to `LookupResultOutOfSync`
 
 ## [0.4.4] - 2023-11-05
 

crd/discover.go

@@ -1,6 +1,8 @@
 package crd
 
 import (
+	"fmt"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
@@ -84,26 +86,29 @@ func NegativeLookupResultCondition() metav1.Condition {
 // LookupResultOutOfSync records an event indicating that the service instance
 // was discovered via DNS-SD, but the result did not match the advertised DNS
 // records.
-func LookupResultOutOfSync(m manager.Manager, res *DNSSDServiceInstance) {
+func LookupResultOutOfSync(m manager.Manager, res *DNSSDServiceInstance, diff string) {
 	m.
 		GetEventRecorderFor("proclaim-dnssd").
 		Event(
 			res,
 			"Warning",
 			"LookupResultOutOfSync",
-			"instance discovered with incorrect (potentially cached) values",
+			"instance discovered with incorrect (potentially cached) values: "+diff,
 		)
 }
 
 // LookupResultOutOfSyncCondition returns a condition indicating that the
 // instance was found by a DNS-SD lookup operation, but the result did not match
 // the advertised DNS records.
-func LookupResultOutOfSyncCondition() metav1.Condition {
+func LookupResultOutOfSyncCondition(diff string) metav1.Condition {
 	return metav1.Condition{
-		Type:    ConditionTypeDiscoverable,
-		Status:  metav1.ConditionFalse,
-		Reason:  "LookupResultOutOfSync",
-		Message: "DNS-SD lookup result does not match the advertised DNS records",
+		Type:   ConditionTypeDiscoverable,
+		Status: metav1.ConditionFalse,
+		Reason: "LookupResultOutOfSync",
+		Message: fmt.Sprintf(
+			"DNS-SD lookup result does not match the advertised DNS records: %s",
+			diff,
+		),
 	}
 }
 

reconciler/discover.go

@@ -2,9 +2,11 @@ package reconciler
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"time"
 
+	"github.com/dogmatiq/dissolve/dnssd"
 	"github.com/dogmatiq/proclaim/crd"
 	"golang.org/x/exp/slices"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -61,20 +63,47 @@ func (r *Reconciler) computeDiscoverable(
 
 	desired := res.Spec.ToDissolve()
 
+	if drift, ok := compare(observed, desired); !ok {
+		crd.LookupResultOutOfSync(r.Manager, res, drift)
+		return observed.TTL, crd.LookupResultOutOfSyncCondition(drift)
+	}
+
+	d := res.Condition(crd.ConditionTypeDiscoverable)
+	if d.Status != metav1.ConditionTrue {
+		crd.Discovered(r.Manager, res)
+	}
+	return observed.TTL, crd.DiscoveredCondition()
+}
+
+// compare returns a (very) brief human-readable description of the differences
+// between the observed and desired service instance records.
+func compare(observed, desired dnssd.ServiceInstance) (string, bool) {
+	if observed.TargetHost != desired.TargetHost {
+		return fmt.Sprintf("host %q != %q", observed.TargetHost, desired.TargetHost), false
+	}
+
+	if observed.TargetPort != desired.TargetPort {
+		return fmt.Sprintf("port %d != %d", observed.TargetPort, desired.TargetPort), false
+	}
+
+	if observed.Priority != desired.Priority {
+		return fmt.Sprintf("priority %d != %d", observed.Priority, desired.Priority), false
+	}
+
+	if observed.Weight != desired.Weight {
+		return fmt.Sprintf("weight %d != %d", observed.Weight, desired.Weight), false
+	}
+
+	if !dnssd.AttributeCollectionsEqual(observed.Attributes, desired.Attributes) {
+		return "attributes", false
+	}
+
 	// The TTL of the observed instance may be less than the desired TTL based
 	// on how old the DNS server's cache is. So long as the observed TTL does
 	// not *exceed* the desired TTL, we consider the records to be in sync.
-	if observed.TTL <= desired.TTL {
-		desired.TTL = observed.TTL
-		if observed.Equal(desired) {
-			d := res.Condition(crd.ConditionTypeDiscoverable)
-			if d.Status != metav1.ConditionTrue {
-				crd.Discovered(r.Manager, res)
-			}
-			return observed.TTL, crd.DiscoveredCondition()
-		}
+	if observed.TTL > desired.TTL {
+		return fmt.Sprintf("ttl %d > %d", observed.TTL, desired.TTL), false
 	}
 
-	crd.LookupResultOutOfSync(r.Manager, res)
-	return observed.TTL, crd.LookupResultOutOfSyncCondition()
+	return "", true
 }

reconciler/reconciler.go

@@ -62,7 +62,7 @@ func (r *Reconciler) Reconcile(
 }
 
 func (r *Reconciler) initialize(
-	ctx context.Context,
+	_ context.Context,
 	res *crd.DNSSDServiceInstance,
 ) (bool, error) {
 	types := []string{