compute-domain-daemon: refine SIGUSR1 logic and add unit tests

Signed-off-by: AkshatDudeja77 <akshat.dudeja77@gmail.com>

Author: AkshatDudeja77

cmd/compute-domain-daemon/main.go

@@ -359,6 +359,20 @@ func IMEXDaemonUpdateLoopWithIPs(ctx context.Context, controller *Controller, cl
 	}
 }
 
+func shouldSendSIGUSR1(oldIPs, newIPs map[string]struct{}, updated, fresh bool) bool {
+	if !updated || fresh {
+		return false
+	}
+
+	for ip := range newIPs {
+		if _, existed := oldIPs[ip]; !existed {
+			return true
+		}
+	}
+
+	return false
+}
+
 // IMEXDaemonUpdateLoopWithDNSNames reacts to ComputeDomain status changes by
 // updating the /etc/hosts file with IP to DNS name mappings. This relies on
 // the IMEX daemon to pick up these changes automatically (and quickly) --
@@ -395,20 +409,7 @@ func IMEXDaemonUpdateLoopWithDNSNames(ctx context.Context, controller *Controlle
 
 			dnsNameManager.LogDNSNameMappings()
 
-			onlyRemovals := true
-			for ip := range dnsNameManager.ipToDNSName {
-				if _, existed := oldIPs[ip]; !existed {
-					onlyRemovals = false
-					break
-				}
-			}
-
-			// Skip sending SIGUSR1 when the process is fresh (has newly been
-			// created) or when this was a noop update. TODO: review skipping
-			// this also if the new set of IP addresses only strictly removes
-			// addresses compared to the old set (then we don't need to force
-			// the daemon to re-resolve & re-connect).
-			if !updated || fresh || onlyRemovals {
+			if !shouldSendSIGUSR1(oldIPs, dnsNameManager.ipToDNSName, updated, fresh) {
 				break
 			}
 

cmd/compute-domain-daemon/main_test.go

@@ -0,0 +1,88 @@
+package main
+
+import "testing"
+
+func set(items ...string) map[string]struct{} {
+	m := make(map[string]struct{}, len(items))
+	for _, i := range items {
+		m[i] = struct{}{}
+	}
+	return m
+}
+
+func TestShouldSendSIGUSR1(t *testing.T) {
+	tests := []struct {
+		name    string
+		old     map[string]struct{}
+		new     map[string]struct{}
+		updated bool
+		fresh   bool
+		want    bool
+	}{
+		{
+			name:    "no change, not updated",
+			old:     set("A", "B", "C"),
+			new:     set("A", "B", "C"),
+			updated: false,
+			fresh:   false,
+			want:    false,
+		},
+		{
+			name:    "no change, updated",
+			old:     set("A", "B", "C"),
+			new:     set("A", "B", "C"),
+			updated: true,
+			fresh:   false,
+			want:    false,
+		},
+		{
+			name:    "pure removal",
+			old:     set("A", "B", "C"),
+			new:     set("A", "B"),
+			updated: true,
+			fresh:   false,
+			want:    false,
+		},
+		{
+			name:    "addition",
+			old:     set("A", "B"),
+			new:     set("A", "B", "C"),
+			updated: true,
+			fresh:   false,
+			want:    true,
+		},
+		{
+			name:    "replacement same size",
+			old:     set("A", "B", "C"),
+			new:     set("A", "B", "D"),
+			updated: true,
+			fresh:   false,
+			want:    true,
+		},
+		{
+			name:    "remove and add",
+			old:     set("A", "B", "C"),
+			new:     set("A", "D"),
+			updated: true,
+			fresh:   false,
+			want:    true,
+		},
+		{
+			name:    "fresh process",
+			old:     set("A", "B"),
+			new:     set("A", "B", "C"),
+			updated: true,
+			fresh:   true,
+			want:    false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := shouldSendSIGUSR1(tt.old, tt.new, tt.updated, tt.fresh)
+			if got != tt.want {
+				t.Fatalf("got %v, want %v", got, tt.want)
+			}
+		})
+	}
+}