chore: account for resource sorting in dns upstream resource

`List` returns a sorted (by id) list of resources. This doesn't work when the order of dns upstreams is important. Because of that add an `Idx` field to the "DNSUpstreams.net.talos.dev" resource, so we can preserve order. Fixes siderolabs#9274 Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
DmitriyMV · Sep 12, 2024 · 951f781 · 951f781
1 parent e17fafa
commit 951f781
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 14 deletions.
diff --git a/internal/app/machined/pkg/controllers/network/dns_resolve_cache.go b/internal/app/machined/pkg/controllers/network/dns_resolve_cache.go
@@ -5,6 +5,7 @@
 package network
 
 import (
+	"cmp"
 	"context"
 	"errors"
 	"fmt"
@@ -160,14 +161,13 @@ func (ctrl *DNSResolveCacheController) Run(ctx context.Context, r controller.Run
 			return fmt.Errorf("error getting resolver status: %w", err)
 		}
 
-		addrs, prxs := make([]string, 0, upstreams.Len()), make([]*proxy.Proxy, 0, upstreams.Len())
+		upstreams.SortFunc(func(a, b *network.DNSUpstream) int {
+			return cmp.Compare(a.TypedSpec().Value.Idx, b.TypedSpec().Value.Idx)
+		})
 
-		for it := upstreams.Iterator(); it.Next(); {
-			prx := it.Value().TypedSpec().Value.Prx
-
-			addrs = append(addrs, prx.Addr())
-			prxs = append(prxs, prx.(*proxy.Proxy)) //nolint:forcetypeassert
-		}
+		//nolint:forcetypeassert
+		prxs := safe.ToSlice(upstreams, func(d *network.DNSUpstream) *proxy.Proxy { return d.TypedSpec().Value.Prx.(*proxy.Proxy) })
+		addrs := safe.ToSlice(upstreams, func(d *network.DNSUpstream) string { return d.TypedSpec().Value.Prx.Addr() })
 
 		if ctrl.handler.SetProxy(prxs) {
 			ctrl.Logger.Info("updated dns server nameservers", zap.Strings("addrs", addrs))

diff --git a/internal/app/machined/pkg/controllers/network/dns_upstream.go b/internal/app/machined/pkg/controllers/network/dns_upstream.go
@@ -103,7 +103,7 @@ func (ctrl *DNSUpstreamController) run(ctx context.Context, r controller.Runtime
 		return err
 	}
 
-	for _, s := range rs.TypedSpec().DNSServers {
+	for i, s := range rs.TypedSpec().DNSServers {
 		remoteAddr := s.String()
 
 		if err = safe.WriterModify[*network.DNSUpstream](
@@ -114,6 +114,14 @@ func (ctrl *DNSUpstreamController) run(ctx context.Context, r controller.Runtime
 				touchedIDs[u.Metadata().ID()] = struct{}{}
 
 				if u.TypedSpec().Value.Prx != nil {
+					// Found upstream, update index
+					if u.TypedSpec().Value.Idx != i {
+						old := u.TypedSpec().Value.Idx
+						u.TypedSpec().Value.Idx = i
+
+						l.Info("updated dns upstream idx", zap.String("addr", remoteAddr), zap.Int("was", old), zap.Int("now", i))
+					}
+
 					return nil
 				}
 
@@ -122,8 +130,9 @@ func (ctrl *DNSUpstreamController) run(ctx context.Context, r controller.Runtime
 				prx.Start(500 * time.Millisecond)
 
 				u.TypedSpec().Value.Prx = prx
+				u.TypedSpec().Value.Idx = i
 
-				l.Info("created dns upstream", zap.String("addr", remoteAddr))
+				l.Info("created dns upstream", zap.String("addr", remoteAddr), zap.Int("idx", i))
 
 				return nil
 			},

diff --git a/internal/app/machined/pkg/controllers/network/resolver_merge.go b/internal/app/machined/pkg/controllers/network/resolver_merge.go
@@ -94,9 +94,7 @@ func (ctrl *ResolverMergeController) Run(ctx context.Context, r controller.Runti
 		}
 
 		if final.DNSServers != nil {
-			if err = r.Modify(ctx, network.NewResolverSpec(network.NamespaceName, network.ResolverID), func(res resource.Resource) error {
-				spec := res.(*network.ResolverSpec) //nolint:errcheck,forcetypeassert
-
+			if err = safe.WriterModify(ctx, r, network.NewResolverSpec(network.NamespaceName, network.ResolverID), func(spec *network.ResolverSpec) error {
 				*spec.TypedSpec() = final
 
 				return nil
@@ -150,9 +148,9 @@ func mergeDNSServers(dst *[]netip.Addr, src []netip.Addr) {
 	// and same vice versa for IPv6
 	switch {
 	case dstHasV4 && !srcHasV4:
-		*dst = append(slices.Clone(src), filterIPFamily(*dst, true)...)
+		*dst = slices.Concat(src, filterIPFamily(*dst, true))
 	case dstHasV6 && !srcHasV6:
-		*dst = append(slices.Clone(src), filterIPFamily(*dst, false)...)
+		*dst = slices.Concat(src, filterIPFamily(*dst, false))
 	default:
 		*dst = src
 	}

diff --git a/pkg/machinery/resources/network/dns_upstream.go b/pkg/machinery/resources/network/dns_upstream.go
@@ -29,6 +29,7 @@ type DNSUpstreamSpecSpec struct {
 	// We could use a generic struct here, but without generic aliases the usage would look ugly.
 	// Once generic aliases are here, redo the type above as `type DNSUpstream[P Proxy] = typed.Resource[...]`.
 	Prx Proxy
+	Idx int
 }
 
 // MarshalYAML implements yaml.Marshaler interface.
@@ -38,6 +39,7 @@ func (d *DNSUpstreamSpecSpec) MarshalYAML() (any, error) {
 	return map[string]string{
 		"healthy": strconv.FormatBool(d.Prx.Fails() == 0),
 		"addr":    d.Prx.Addr(),
+		"idx":     strconv.Itoa(d.Idx),
 	}, nil
 }