watcher: send the inconsistency alert once per disagreement (closes #158)
check / check (push) Successful in 1m3s
check / check (push) Successful in 1m3s
detectInconsistencies alerted for neighbouring pairs of nameservers whose records differed, on every DNS check, for as long as they differed. It now also takes the previous hostname state, compares every pair of nameservers, and alerts for a pair that differs unless both were in that state and already differed there, so a nameserver new on a check that answers differently is reported once. The state loaded at startup is the previous state for the first check, so a disagreement saved before a restart is not reported again. The choice of pairs is tested on record data, and the alert through the hostname change detection with the notifier stand-in and no resolver. The README describes the new behaviour. Model: opus-5-5
This commit is contained in:
@@ -74,8 +74,15 @@ rejected.
|
|||||||
This is distinct from "responded with no records."
|
This is distinct from "responded with no records."
|
||||||
- **NS recovery**: A previously-unreachable nameserver starts
|
- **NS recovery**: A previously-unreachable nameserver starts
|
||||||
responding again.
|
responding again.
|
||||||
- **Inconsistency detected**: Two nameservers that previously agreed
|
- **Inconsistency detected**: Two nameservers return different record
|
||||||
now return different record sets for the same hostname.
|
sets for the same hostname and did not already differ on the previous
|
||||||
|
check. Every pair of nameservers is compared. The alert is sent once
|
||||||
|
for each such pair, on the check where they start to disagree, and not
|
||||||
|
again while they keep disagreeing, including after a restart. A
|
||||||
|
nameserver that was not in the previous check (newly added, or back
|
||||||
|
after dropping out) and answers differently is reported on the check
|
||||||
|
where it appears. If a pair agrees again and later disagrees, the
|
||||||
|
alert is sent again.
|
||||||
|
|
||||||
### TCP Port Monitoring
|
### TCP Port Monitoring
|
||||||
|
|
||||||
|
|||||||
@@ -23,6 +23,11 @@ Rationale, Design, TODO, License, Author) if any are still missing.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-09-28: the inconsistency alert is sent once, on the check where two
|
||||||
|
nameservers start to disagree or where a nameserver that disagrees first
|
||||||
|
appears, instead of on every check while they disagree, and not again after
|
||||||
|
a restart. Every pair of nameservers is compared, not only neighbours in
|
||||||
|
sorted order of name (closes #158).
|
||||||
- 2026-09-28: DNS names in record values (CNAME, MX, SRV and NS targets) are
|
- 2026-09-28: DNS names in record values (CNAME, MX, SRV and NS targets) are
|
||||||
lower-cased, so nameservers that answer in different letter case no longer
|
lower-cased, so nameservers that answer in different letter case no longer
|
||||||
count as inconsistent or as a record change (closes #157).
|
count as inconsistent or as a record change (closes #157).
|
||||||
|
|||||||
@@ -0,0 +1,25 @@
|
|||||||
|
package watcher
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
|
||||||
|
"sneak.berlin/go/dnswatcher/internal/state"
|
||||||
|
)
|
||||||
|
|
||||||
|
// NewlyDisagreeingPairs exports newlyDisagreeingPairs for testing.
|
||||||
|
func NewlyDisagreeingPairs(
|
||||||
|
prev *state.HostnameState,
|
||||||
|
current map[string]map[string][]string,
|
||||||
|
) [][2]string {
|
||||||
|
return newlyDisagreeingPairs(prev, current)
|
||||||
|
}
|
||||||
|
|
||||||
|
// DetectHostnameChanges exports detectHostnameChanges for testing.
|
||||||
|
func (w *Watcher) DetectHostnameChanges(
|
||||||
|
ctx context.Context,
|
||||||
|
hostname string,
|
||||||
|
prev *state.HostnameState,
|
||||||
|
current map[string]map[string][]string,
|
||||||
|
) {
|
||||||
|
w.detectHostnameChanges(ctx, hostname, prev, current)
|
||||||
|
}
|
||||||
@@ -0,0 +1,182 @@
|
|||||||
|
package watcher_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"slices"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"sneak.berlin/go/dnswatcher/internal/state"
|
||||||
|
"sneak.berlin/go/dnswatcher/internal/watcher"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
host = "www.example.net"
|
||||||
|
nsA = "a.ns.example.net."
|
||||||
|
nsB = "b.ns.example.net."
|
||||||
|
nsC = "c.ns.example.net."
|
||||||
|
ip1 = "192.0.2.1"
|
||||||
|
ip2 = "192.0.2.2"
|
||||||
|
ip3 = "192.0.2.3"
|
||||||
|
)
|
||||||
|
|
||||||
|
// hostnameState builds the state a check with these records leaves behind.
|
||||||
|
func hostnameState(
|
||||||
|
records map[string]map[string][]string,
|
||||||
|
) *state.HostnameState {
|
||||||
|
hs := &state.HostnameState{
|
||||||
|
RecordsByNameserver: make(map[string]*state.NameserverRecordState),
|
||||||
|
}
|
||||||
|
|
||||||
|
for ns, recs := range records {
|
||||||
|
hs.RecordsByNameserver[ns] = &state.NameserverRecordState{
|
||||||
|
Records: recs,
|
||||||
|
Status: "ok",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return hs
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestNewlyDisagreeingPairs(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
onlyA := map[string]map[string][]string{nsA: {"A": {ip1}}}
|
||||||
|
agree := map[string]map[string][]string{nsA: {"A": {ip1}}, nsB: {"A": {ip1}}}
|
||||||
|
disagree := map[string]map[string][]string{nsA: {"A": {ip1}}, nsB: {"A": {ip2}}}
|
||||||
|
alert := [][2]string{{nsA, nsB}}
|
||||||
|
|
||||||
|
// b already disagrees with a and c; then c changes, so a and c,
|
||||||
|
// which agreed, now differ.
|
||||||
|
bDiffers := map[string]map[string][]string{
|
||||||
|
nsA: {"A": {ip1}}, nsB: {"A": {ip2}}, nsC: {"A": {ip1}},
|
||||||
|
}
|
||||||
|
cChanges := map[string]map[string][]string{
|
||||||
|
nsA: {"A": {ip1}}, nsB: {"A": {ip2}}, nsC: {"A": {ip3}},
|
||||||
|
}
|
||||||
|
|
||||||
|
// Each case starts from the state loaded at startup and runs the
|
||||||
|
// checks in order; want[i] is what check i alerts for.
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
loaded map[string]map[string][]string
|
||||||
|
checks []map[string]map[string][]string
|
||||||
|
want [][][2]string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "disagreement persisting across checks alerts once",
|
||||||
|
loaded: agree,
|
||||||
|
checks: []map[string]map[string][]string{disagree, disagree, disagree},
|
||||||
|
want: [][][2]string{alert, nil, nil},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "disagreement starting on a later check alerts on it",
|
||||||
|
loaded: agree,
|
||||||
|
checks: []map[string]map[string][]string{agree, agree, disagree},
|
||||||
|
want: [][][2]string{nil, nil, alert},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "disagreement in the loaded state does not alert",
|
||||||
|
loaded: disagree,
|
||||||
|
checks: []map[string]map[string][]string{disagree, disagree},
|
||||||
|
want: [][][2]string{nil, nil},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "nameserver new on the first check and disagreeing alerts once",
|
||||||
|
loaded: onlyA,
|
||||||
|
checks: []map[string]map[string][]string{disagree, disagree},
|
||||||
|
want: [][][2]string{alert, nil},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "disagreement after agreeing again alerts again",
|
||||||
|
loaded: agree,
|
||||||
|
checks: []map[string]map[string][]string{disagree, agree, disagree},
|
||||||
|
want: [][][2]string{alert, nil, alert},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "new disagreement while another nameserver differs alerts",
|
||||||
|
loaded: bDiffers,
|
||||||
|
checks: []map[string]map[string][]string{cChanges, cChanges},
|
||||||
|
want: [][][2]string{{{nsA, nsC}}, nil},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
prev := hostnameState(tt.loaded)
|
||||||
|
|
||||||
|
for i, current := range tt.checks {
|
||||||
|
got := watcher.NewlyDisagreeingPairs(prev, current)
|
||||||
|
if !slices.Equal(got, tt.want[i]) {
|
||||||
|
t.Errorf(
|
||||||
|
"check %d: alerted for %v, want %v",
|
||||||
|
i, got, tt.want[i],
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
prev = hostnameState(current)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestInconsistencyAlert(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
onlyA := map[string]map[string][]string{nsA: {"A": {ip1}}}
|
||||||
|
agree := map[string]map[string][]string{nsA: {"A": {ip1}}, nsB: {"A": {ip1}}}
|
||||||
|
disagree := map[string]map[string][]string{nsA: {"A": {ip1}}, nsB: {"A": {ip2}}}
|
||||||
|
|
||||||
|
// Each case starts from the state loaded at startup and then sees
|
||||||
|
// the nameservers disagree on three checks in a row.
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
loaded map[string]map[string][]string
|
||||||
|
want int
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "disagreement lasting several checks alerts once",
|
||||||
|
loaded: agree,
|
||||||
|
want: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "disagreement in the loaded state does not alert",
|
||||||
|
loaded: disagree,
|
||||||
|
want: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "nameserver new on the first check and disagreeing alerts once",
|
||||||
|
loaded: onlyA,
|
||||||
|
want: 1,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// The hostname change detection uses only the notifier.
|
||||||
|
notifier := &mockNotifier{}
|
||||||
|
w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier)
|
||||||
|
|
||||||
|
prev := hostnameState(tt.loaded)
|
||||||
|
|
||||||
|
for range 3 {
|
||||||
|
w.DetectHostnameChanges(t.Context(), host, prev, disagree)
|
||||||
|
prev = hostnameState(disagree)
|
||||||
|
}
|
||||||
|
|
||||||
|
got := 0
|
||||||
|
|
||||||
|
for _, n := range notifier.getNotifications() {
|
||||||
|
if n.Title == "Inconsistency: "+host {
|
||||||
|
got++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if got != tt.want {
|
||||||
|
t.Errorf("sent %d inconsistency alerts, want %d", got, tt.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
+41
-15
@@ -366,7 +366,7 @@ func (w *Watcher) detectHostnameChanges(
|
|||||||
) {
|
) {
|
||||||
w.detectRecordChanges(ctx, hostname, prev, current)
|
w.detectRecordChanges(ctx, hostname, prev, current)
|
||||||
w.detectNSDisappearances(ctx, hostname, prev, current)
|
w.detectNSDisappearances(ctx, hostname, prev, current)
|
||||||
w.detectInconsistencies(ctx, hostname, current)
|
w.detectInconsistencies(ctx, hostname, prev, current)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (w *Watcher) detectRecordChanges(
|
func (w *Watcher) detectRecordChanges(
|
||||||
@@ -448,22 +448,11 @@ func (w *Watcher) detectNSDisappearances(
|
|||||||
func (w *Watcher) detectInconsistencies(
|
func (w *Watcher) detectInconsistencies(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
hostname string,
|
hostname string,
|
||||||
|
prev *state.HostnameState,
|
||||||
current map[string]map[string][]string,
|
current map[string]map[string][]string,
|
||||||
) {
|
) {
|
||||||
nameservers := make([]string, 0, len(current))
|
for _, pair := range newlyDisagreeingPairs(prev, current) {
|
||||||
for ns := range current {
|
ns1, ns2 := pair[0], pair[1]
|
||||||
nameservers = append(nameservers, ns)
|
|
||||||
}
|
|
||||||
|
|
||||||
sort.Strings(nameservers)
|
|
||||||
|
|
||||||
for i := range len(nameservers) - 1 {
|
|
||||||
ns1 := nameservers[i]
|
|
||||||
ns2 := nameservers[i+1]
|
|
||||||
|
|
||||||
if recordsEqual(current[ns1], current[ns2]) {
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
msg := fmt.Sprintf(
|
msg := fmt.Sprintf(
|
||||||
"Hostname: %s\n%s: %v\n%s: %v",
|
"Hostname: %s\n%s: %v\n%s: %v",
|
||||||
@@ -481,6 +470,43 @@ func (w *Watcher) detectInconsistencies(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// newlyDisagreeingPairs returns every pair of nameservers whose records
|
||||||
|
// differ in current, in sorted order of name, except pairs where both
|
||||||
|
// nameservers were in prev and already differed there. A nameserver
|
||||||
|
// missing from prev is paired with every nameserver it differs from.
|
||||||
|
func newlyDisagreeingPairs(
|
||||||
|
prev *state.HostnameState,
|
||||||
|
current map[string]map[string][]string,
|
||||||
|
) [][2]string {
|
||||||
|
nameservers := make([]string, 0, len(current))
|
||||||
|
for ns := range current {
|
||||||
|
nameservers = append(nameservers, ns)
|
||||||
|
}
|
||||||
|
|
||||||
|
sort.Strings(nameservers)
|
||||||
|
|
||||||
|
var pairs [][2]string
|
||||||
|
|
||||||
|
for i, ns1 := range nameservers {
|
||||||
|
for _, ns2 := range nameservers[i+1:] {
|
||||||
|
if recordsEqual(current[ns1], current[ns2]) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
prev1, ok1 := prev.RecordsByNameserver[ns1]
|
||||||
|
prev2, ok2 := prev.RecordsByNameserver[ns2]
|
||||||
|
|
||||||
|
if ok1 && ok2 && !recordsEqual(prev1.Records, prev2.Records) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
pairs = append(pairs, [2]string{ns1, ns2})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return pairs
|
||||||
|
}
|
||||||
|
|
||||||
func (w *Watcher) checkAllPorts(ctx context.Context) {
|
func (w *Watcher) checkAllPorts(ctx context.Context) {
|
||||||
// Phase 1: Build current IP:port → hostname associations
|
// Phase 1: Build current IP:port → hostname associations
|
||||||
// from fresh DNS data.
|
// from fresh DNS data.
|
||||||
|
|||||||
Reference in New Issue
Block a user