compute-domain-daemon: avoid SIGUSR1 on peer removals only

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

Author: AkshatDudeja77

cmd/compute-domain-daemon/main.go

@@ -359,6 +359,24 @@ func IMEXDaemonUpdateLoopWithIPs(ctx context.Context, controller *Controller, cl
 	}
 }
 
+// shouldSendSIGUSR1 determines whether the IMEX daemon should be
+// signaled to re-resolve and reconnect to peers.
+//
+// fresh indicates whether the IMEX daemon process was just started.
+//
+// The signal is sent only when:
+// - the mapping was updated,
+// - the process is not fresh, and
+// - at least one new IP (peer) was added.
+func shouldSendSIGUSR1(oldIPs, newIPs IPSet, fresh bool) bool {
+	if fresh {
+		return false
+	}
+
+	added, _ := oldIPs.Diff(newIPs)
+	return len(added) > 0
+}
+
 // 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) --
@@ -374,6 +392,10 @@ func IMEXDaemonUpdateLoopWithDNSNames(ctx context.Context, controller *Controlle
 			klog.Infof("shutdown: stop IMEXDaemonUpdateLoopWithDNSNames")
 			return nil
 		case daemons := <-controller.GetDaemonInfoUpdateChan():
+			oldIPs := make(map[string]struct{}, len(dnsNameManager.ipToDNSName))
+			for ip := range dnsNameManager.ipToDNSName {
+				oldIPs[ip] = struct{}{}
+			}
 			updated, err := dnsNameManager.UpdateDNSNameMappings(daemons)
 			if err != nil {
 				return fmt.Errorf("failed to update DNS name => IP mappings: %w", err)
@@ -390,13 +412,16 @@ func IMEXDaemonUpdateLoopWithDNSNames(ctx context.Context, controller *Controlle
 			}
 
 			dnsNameManager.LogDNSNameMappings()
+			// Skip sending SIGUSR1 when:
+			// - the process is fresh (has newly been started), or
+			// - this was a noop update, or
+			// - no new peers were added (i.e. the update only removes nodes or keeps the set unchanged).
+			newIPs := make(IPSet, len(dnsNameManager.ipToDNSName))
+			for ip := range dnsNameManager.ipToDNSName {
+				newIPs[ip] = struct{}{}
+			}
 
-			// 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 {
+			if !updated || !shouldSendSIGUSR1(IPSet(oldIPs), newIPs, fresh) {
 				break
 			}
 

cmd/compute-domain-daemon/main_test.go

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

internal/common/util.go

@@ -34,10 +34,6 @@ const dumpPath = "/tmp/goroutine-stacks.dump"
 // output didn't work.
 func StartDebugSignalHandlers() {
 	go func() {
-		if runtime.GOOS == "windows" {
-			klog.Infof("Debug signal handlers are disabled on Windows")
-			return
-		}
 		c := make(chan os.Signal, 1)
 		signal.Notify(c, syscall.SIGUSR2)
 		for range c {