cmd/k8s-operator: stabilize StaticEndpoints order in ProxyGroup reconciles (#19755)

findStaticEndpoints built its return slice by iterating nodes.Items in
the order returned by r.List, which is not guaranteed to be stable
across calls. When the resulting set of addresses already matched the
existing config Secret, the slice could still permute between
reconciles, making the marshalled config Secret differ byte-for-byte.
That tripped the DeepEqual check on the config Secret, which rewrote
the Secret, which fired a watch event, which re-enqueued the
ProxyGroup, looping forever.

Detect this case and return the existing currAddrs slice unchanged
when the resulting set is the same, preserving the "use the currently
used IPs first" intent without spurious writes.

Fixes #19700

Signed-off-by: Jason Dillingham <jasonmdillingham@gmail.com>

cmd/k8s-operator/proxygroup.go

@@ -1104,9 +1104,35 @@ func (r *ProxyGroupReconciler) findStaticEndpoints(ctx context.Context, existing
 		return nil, &FindStaticEndpointErr{msg: fmt.Sprintf("failed to find any `status.addresses` of type %q on nodes using configured Selectors on `spec.staticEndpoints.nodePort.selectors` for ProxyClass %q", corev1.NodeExternalIP, proxyClass.Name)}
 	}
 
+	// If we ended up selecting the same set of addresses already in use, keep
+	// the existing order. nodes.Items from r.List is not guaranteed to be in
+	// a stable order across calls, so without this the slice can permute on
+	// each reconcile, making the marshalled config Secret differ byte-for-byte
+	// even though nothing has effectively changed. That trips the DeepEqual
+	// check on the config Secret, which writes the Secret, which fires a
+	// watch event, which re-enqueues the ProxyGroup, and so on.
+	if len(currAddrs) > 0 && sameAddrPortSet(endpoints, currAddrs) {
+		return currAddrs, nil
+	}
+
 	return endpoints, nil
 }
 
+// sameAddrPortSet reports whether a and b contain the same AddrPorts,
+// ignoring order. Both slices are assumed to be free of duplicates, which
+// holds for callers in this package.
+func sameAddrPortSet(a, b []netip.AddrPort) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for _, x := range a {
+		if !slices.Contains(b, x) {
+			return false
+		}
+	}
+	return true
+}
+
 func getStaticEndpointAddress(a *corev1.NodeAddress, port uint16) *netip.AddrPort {
 	addr, err := netip.ParseAddr(a.Address)
 	if err != nil {

cmd/k8s-operator/proxygroup_test.go

@@ -811,6 +811,90 @@ func TestProxyGroupWithStaticEndpoints(t *testing.T) {
 	}
 }
 
+// TestFindStaticEndpointsStableOrder verifies that findStaticEndpoints returns
+// the existing endpoint order from the config Secret when the resulting set of
+// addresses is unchanged. nodes.Items from r.List is not order-stable across
+// calls, so without this guarantee the slice can permute on each reconcile,
+// triggering a spurious config Secret rewrite which fires a watch event that
+// re-enqueues the ProxyGroup, looping forever (issue #19700).
+func TestFindStaticEndpointsStableOrder(t *testing.T) {
+	const (
+		addrA = "10.0.0.1"
+		addrB = "10.0.0.2"
+		port  = uint16(30001)
+	)
+
+	pc := &tsapi.ProxyClass{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-pc"},
+		Spec: tsapi.ProxyClassSpec{
+			StaticEndpoints: &tsapi.StaticEndpointsConfig{
+				NodePort: &tsapi.NodePortConfig{
+					Ports:    []tsapi.PortRange{{Port: port}},
+					Selector: map[string]string{"foo/bar": "baz"},
+				},
+			},
+		},
+	}
+
+	// Existing config Secret already pins the order [B, A]. The fake client
+	// lists nodes in name order ([node-a, node-b]) so without the stable-order
+	// guard findStaticEndpoints would return [A, B], differing from currAddrs
+	// and causing a spurious Secret rewrite.
+	currAddrs := []netip.AddrPort{
+		netip.MustParseAddrPort(addrB + ":30001"),
+		netip.MustParseAddrPort(addrA + ":30001"),
+	}
+	cfg := ipn.ConfigVAlpha{StaticEndpoints: currAddrs}
+	cfgJSON, err := json.Marshal(cfg)
+	if err != nil {
+		t.Fatalf("marshal config: %v", err)
+	}
+	existingSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-0-config", Namespace: tsNamespace},
+		Data:       map[string][]byte{tsoperator.TailscaledConfigFileName(106): cfgJSON},
+	}
+
+	nodes := []*corev1.Node{
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "node-a", Labels: map[string]string{"foo/bar": "baz"}},
+			Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{
+				{Type: corev1.NodeExternalIP, Address: addrA},
+			}},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "node-b", Labels: map[string]string{"foo/bar": "baz"}},
+			Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{
+				{Type: corev1.NodeExternalIP, Address: addrB},
+			}},
+		},
+	}
+
+	fc := fake.NewClientBuilder().
+		WithScheme(tsapi.GlobalScheme).
+		WithObjects(pc, nodes[0], nodes[1], existingSecret).
+		Build()
+
+	zl, _ := zap.NewDevelopment()
+	r := &ProxyGroupReconciler{Client: fc}
+
+	got, err := r.findStaticEndpoints(t.Context(), existingSecret, pc, port, zl.Sugar())
+	if err != nil {
+		t.Fatalf("findStaticEndpoints: %v", err)
+	}
+	if !slices.Equal(got, currAddrs) {
+		t.Errorf("findStaticEndpoints returned %v, want %v (order must match currAddrs to avoid reconcile churn)", got, currAddrs)
+	}
+
+	// Repeat to confirm the result is stable across calls.
+	got2, err := r.findStaticEndpoints(t.Context(), existingSecret, pc, port, zl.Sugar())
+	if err != nil {
+		t.Fatalf("findStaticEndpoints (2nd call): %v", err)
+	}
+	if !slices.Equal(got, got2) {
+		t.Errorf("findStaticEndpoints not stable across calls: first=%v second=%v", got, got2)
+	}
+}
+
 func TestProxyGroup(t *testing.T) {
 	pc := &tsapi.ProxyClass{
 		ObjectMeta: metav1.ObjectMeta{