watcher: a removed nameserver is an NS Change, not an NS Failure (closes #262)

A nameserver that left a domain's delegation was reported twice: as the
domain's NS Change, and by the check of the domain's own records as an
error-level NS Failure saying it disappeared. That check now sends
nothing for a domain. For a hostname, the nameservers that leave its
zone are sent as one NS Change at warning level, with a Hostname: line
and a Removed: line, as decided on the issue; nothing is sent for one
that joins. The README says what each case gets.

A nameserver that was failing before it left is now listed too; the
NS Failure skipped it.
A domain with no delegation of its own gets nothing when a nameserver
leaves the zone it is in.

Model: opus-5-5

internal/watcher/message_test.go

@@ -81,14 +81,12 @@ func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 	)
 
 	// nsA's address changes, which also makes it differ from nsC; nsB
-	// fails; nsC answers again; nsD is gone.
-	nsD := "d.ns.example.net."
+	// fails; nsC answers again.
 	w.DetectHostnameChanges(t.Context(), domain,
 		saved(map[string]*state.NameserverRecordState{
 			nsA: answered(map[string][]string{"A": {ip1}}),
 			nsB: answered(map[string][]string{"A": {ip1}}),
 			nsC: failed(),
-			nsD: answered(map[string][]string{"A": {ip1}}),
 		}),
 		saved(map[string]*state.NameserverRecordState{
 			nsA: answered(map[string][]string{"A": {ip2}}),
@@ -102,11 +100,10 @@ func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 		t.Context(), domain, cnameState(ip1), cnameState(ip2),
 	)
 
-	// NS Failure is sent for nsB failing and for nsD being gone.
 	want := map[string]int{
 		"Record Change":        1,
 		"Inconsistency":        1,
-		"NS Failure":           2,
+		"NS Failure":           1,
 		"NS Recovery":          1,
 		"CNAME Address Change": 1,
 	}
@@ -128,6 +125,41 @@ func TestDomainRecordNotificationsNameTheDomain(t *testing.T) {
 	}
 }
 
+// nsB, which answered, and nsC, which did not, are no longer among the
+// nameservers. For the hostname one NS Change lists both; for the
+// configured apex domain nothing is sent, because the check of its
+// delegation sends its NS Change.
+func TestRemovedNameserversAreAnNSChange(t *testing.T) {
+	t.Parallel()
+
+	notifier := &mockNotifier{}
+	w := watcher.NewForTest(
+		&config.Config{Domains: []string{domain}},
+		nil, nil, nil, nil, notifier,
+	)
+
+	prev := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(map[string][]string{"A": {ip1}}),
+		nsB: answered(map[string][]string{"A": {ip1}}),
+		nsC: failed(),
+	})
+	current := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(map[string][]string{"A": {ip1}}),
+	})
+
+	w.DetectHostnameChanges(t.Context(), domain, prev, current)
+	w.DetectHostnameChanges(t.Context(), host, prev, current)
+
+	title := "NS Change: " + host
+	want := `Hostname: www.example.net
+Removed: b.ns.example.net., c.ns.example.net.`
+
+	got := notifier.getNotifications()
+	if len(got) != 1 || got[0].Title != title || got[0].Message != want {
+		t.Errorf("sent %v, want one %q with message:\n%s", got, title, want)
+	}
+}
+
 // The startup notification counts the configured domains and hostnames,
 // although the state's hostnames also hold the apex domain's own
 // records. Nothing is looked up: the watcher has no resolver.

internal/watcher/watcher.go

@@ -772,28 +772,45 @@ func (w *Watcher) detectRecordChanges(
 	}
 }
 
+// detectNSDisappearances sends one NS Change for a hostname, listing
+// the nameservers of the previous check that are no longer those of its
+// zone. Nothing is sent for a configured apex domain, whose removed
+// nameservers detectNSChanges notifies.
 func (w *Watcher) detectNSDisappearances(
 	ctx context.Context,
 	hostname string,
 	prev, current *state.HostnameState,
 ) {
-	for ns, prevNS := range prev.RecordsByNameserver {
-		if _, ok := current.RecordsByNameserver[ns]; ok || prevNS.Status != statusOK {
-			continue
-		}
-
-		msg := fmt.Sprintf(
-			"%s\nNameserver: %s disappeared",
-			w.nameLine(hostname), ns,
-		)
-
-		w.notify.SendNotification(
-			ctx,
-			"NS Failure: "+hostname,
-			msg,
-			"error",
-		)
+	if w.isDomain(hostname) {
+		return
 	}
+
+	var removed []string
+
+	for ns := range prev.RecordsByNameserver {
+		if _, ok := current.RecordsByNameserver[ns]; !ok {
+			removed = append(removed, ns)
+		}
+	}
+
+	if len(removed) == 0 {
+		return
+	}
+
+	slices.Sort(removed)
+
+	msg := fmt.Sprintf(
+		"Hostname: %s\nRemoved: %s",
+		hostname,
+		strings.Join(removed, ", "),
+	)
+
+	w.notify.SendNotification(
+		ctx,
+		"NS Change: "+hostname,
+		msg,
+		"warning",
+	)
 }
 
 // detectNSFailures notifies when a nameserver that answered on the

internal/watcher/watcher_test.go

@@ -1049,7 +1049,7 @@ func TestSendTestNotification_Disabled(t *testing.T) {
 	}
 }
 
-func TestNSFailureAndRecovery(t *testing.T) {
+func TestNSChangeAndRecovery(t *testing.T) {
 	t.Parallel()
 
 	cfg := defaultTestConfig(t)
@@ -1059,7 +1059,7 @@ func TestNSFailureAndRecovery(t *testing.T) {
 
 	// The saved state has every nameserver live DNS lists as one that
 	// did not answer, and, as answering, one that live DNS does not
-	// list, which then disappears.
+	// list, which the check then finds removed.
 	_, deps := runChecks(t, cfg, func(deps *testDeps) {
 		byNameserver := map[string]*state.NameserverRecordState{
 			oldNS1: answered(map[string][]string{"A": {oldIP}}),
@@ -1071,7 +1071,7 @@ func TestNSFailureAndRecovery(t *testing.T) {
 		deps.state.SetHostnameState(testHost, saved(byNameserver))
 	})
 
-	assertNotified(t, deps, "NS Failure: "+testHost, "error")
+	assertNotified(t, deps, "NS Change: "+testHost, "warning")
 	assertNotified(t, deps, "NS Recovery: "+testHost, "success")
 
 	// A nameserver that did not answer has no records to compare, so