Skip to content

Commit a159ea9

Browse files
Dmitriy Matrenichevsmira
authored andcommitted
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 #9274 Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com> (cherry picked from commit 79cd031)
1 parent c030eef commit a159ea9

5 files changed

Lines changed: 85 additions & 15 deletions

File tree

internal/app/machined/pkg/controllers/network/dns_resolve_cache.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package network
66

77
import (
8+
"cmp"
89
"context"
910
"errors"
1011
"fmt"
@@ -160,14 +161,7 @@ func (ctrl *DNSResolveCacheController) Run(ctx context.Context, r controller.Run
160161
return fmt.Errorf("error getting resolver status: %w", err)
161162
}
162163

163-
addrs, prxs := make([]string, 0, upstreams.Len()), make([]*proxy.Proxy, 0, upstreams.Len())
164-
165-
for it := upstreams.Iterator(); it.Next(); {
166-
prx := it.Value().TypedSpec().Value.Prx
167-
168-
addrs = append(addrs, prx.Addr())
169-
prxs = append(prxs, prx.(*proxy.Proxy)) //nolint:forcetypeassert
170-
}
164+
prxs, addrs := SortedProxies(upstreams)
171165

172166
if ctrl.handler.SetProxy(prxs) {
173167
ctrl.Logger.Info("updated dns server nameservers", zap.Strings("addrs", addrs))
@@ -179,6 +173,17 @@ func (ctrl *DNSResolveCacheController) Run(ctx context.Context, r controller.Run
179173
}
180174
}
181175

176+
// SortedProxies returns sorted list of proxies and their addresses.
177+
func SortedProxies(upstreams safe.List[*network.DNSUpstream]) ([]*proxy.Proxy, []string) {
178+
upstreams.SortFunc(func(a, b *network.DNSUpstream) int {
179+
return cmp.Compare(a.TypedSpec().Value.Idx, b.TypedSpec().Value.Idx)
180+
})
181+
182+
//nolint:forcetypeassert
183+
return safe.ToSlice(upstreams, func(d *network.DNSUpstream) *proxy.Proxy { return d.TypedSpec().Value.Prx.(*proxy.Proxy) }),
184+
safe.ToSlice(upstreams, func(d *network.DNSUpstream) string { return d.TypedSpec().Value.Prx.Addr() })
185+
}
186+
182187
func (ctrl *DNSResolveCacheController) writeDNSStatus(ctx context.Context, r controller.Runtime, config runnerConfig) error {
183188
return safe.WriterModify(ctx, r, network.NewDNSResolveCache(fmt.Sprintf("%s-%s", config.net, config.addr)), func(drc *network.DNSResolveCache) error {
184189
drc.TypedSpec().Status = "running"

internal/app/machined/pkg/controllers/network/dns_resolve_cache_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/cosi-project/runtime/pkg/resource"
1616
"github.com/cosi-project/runtime/pkg/resource/rtestutils"
17+
"github.com/cosi-project/runtime/pkg/safe"
1718
"github.com/miekg/dns"
1819
"github.com/siderolabs/gen/xslices"
1920
"github.com/siderolabs/gen/xtesting/must"
@@ -284,3 +285,54 @@ func makeAddrs(port string) []netip.AddrPort {
284285
netip.MustParseAddrPort("127.0.0.53:" + port),
285286
}
286287
}
288+
289+
type DNSUpstreams struct {
290+
ctest.DefaultSuite
291+
}
292+
293+
func (suite *DNSUpstreams) TestOrder() {
294+
port := must.Value(getDynamicPort())(suite.T())
295+
296+
cfg := network.NewHostDNSConfig(network.HostDNSConfigID)
297+
cfg.TypedSpec().Enabled = true
298+
cfg.TypedSpec().ListenAddresses = makeAddrs(port)
299+
300+
suite.Require().NoError(suite.State().Create(suite.Ctx(), cfg))
301+
302+
resolverSpec := network.NewResolverStatus(network.NamespaceName, network.ResolverID)
303+
304+
for i, addrs := range [][]string{
305+
{"1.0.0.1", "8.8.8.8", "1.1.1.1"},
306+
{"1.1.1.1", "8.8.8.8", "1.0.0.1", "8.0.0.8"},
307+
{"192.168.0.1"},
308+
} {
309+
resolverSpec.TypedSpec().DNSServers = xslices.Map(addrs, netip.MustParseAddr)
310+
311+
switch i {
312+
case 0:
313+
suite.Require().NoError(suite.State().Create(suite.Ctx(), resolverSpec))
314+
default:
315+
suite.Require().NoError(suite.State().Update(suite.Ctx(), resolverSpec))
316+
}
317+
318+
rtestutils.AssertLength[*network.DNSUpstream](suite.Ctx(), suite.T(), suite.State(), len(addrs))
319+
320+
upstreams, err := safe.ReaderListAll[*network.DNSUpstream](suite.Ctx(), suite.State())
321+
suite.Require().NoError(err)
322+
323+
_, upstreamAddrs := netctrl.SortedProxies(upstreams)
324+
325+
suite.Require().Equal(xslices.Map(addrs, func(t string) string { return t + ":53" }), upstreamAddrs)
326+
}
327+
}
328+
329+
func TestDNSUpstreams(t *testing.T) {
330+
suite.Run(t, &DNSUpstreams{
331+
DefaultSuite: ctest.DefaultSuite{
332+
Timeout: 10 * time.Second,
333+
AfterSetup: func(suite *ctest.DefaultSuite) {
334+
suite.Require().NoError(suite.Runtime().RegisterController(&netctrl.DNSUpstreamController{}))
335+
},
336+
},
337+
})
338+
}

internal/app/machined/pkg/controllers/network/dns_upstream.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func (ctrl *DNSUpstreamController) run(ctx context.Context, r controller.Runtime
103103
return err
104104
}
105105

106-
for _, s := range rs.TypedSpec().DNSServers {
106+
for i, s := range rs.TypedSpec().DNSServers {
107107
remoteAddr := s.String()
108108

109109
if err = safe.WriterModify[*network.DNSUpstream](
@@ -114,6 +114,14 @@ func (ctrl *DNSUpstreamController) run(ctx context.Context, r controller.Runtime
114114
touchedIDs[u.Metadata().ID()] = struct{}{}
115115

116116
if u.TypedSpec().Value.Prx != nil {
117+
// Found upstream, update index
118+
if u.TypedSpec().Value.Idx != i {
119+
old := u.TypedSpec().Value.Idx
120+
u.TypedSpec().Value.Idx = i
121+
122+
l.Info("updated dns upstream idx", zap.String("addr", remoteAddr), zap.Int("was", old), zap.Int("now", i))
123+
}
124+
117125
return nil
118126
}
119127

@@ -122,8 +130,9 @@ func (ctrl *DNSUpstreamController) run(ctx context.Context, r controller.Runtime
122130
prx.Start(500 * time.Millisecond)
123131

124132
u.TypedSpec().Value.Prx = prx
133+
u.TypedSpec().Value.Idx = i
125134

126-
l.Info("created dns upstream", zap.String("addr", remoteAddr))
135+
l.Info("created dns upstream", zap.String("addr", remoteAddr), zap.Int("idx", i))
127136

128137
return nil
129138
},

internal/app/machined/pkg/controllers/network/resolver_merge.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,7 @@ func (ctrl *ResolverMergeController) Run(ctx context.Context, r controller.Runti
9494
}
9595

9696
if final.DNSServers != nil {
97-
if err = r.Modify(ctx, network.NewResolverSpec(network.NamespaceName, network.ResolverID), func(res resource.Resource) error {
98-
spec := res.(*network.ResolverSpec) //nolint:errcheck,forcetypeassert
99-
97+
if err = safe.WriterModify(ctx, r, network.NewResolverSpec(network.NamespaceName, network.ResolverID), func(spec *network.ResolverSpec) error {
10098
*spec.TypedSpec() = final
10199

102100
return nil
@@ -150,9 +148,9 @@ func mergeDNSServers(dst *[]netip.Addr, src []netip.Addr) {
150148
// and same vice versa for IPv6
151149
switch {
152150
case dstHasV4 && !srcHasV4:
153-
*dst = append(slices.Clone(src), filterIPFamily(*dst, true)...)
151+
*dst = slices.Concat(src, filterIPFamily(*dst, true))
154152
case dstHasV6 && !srcHasV6:
155-
*dst = append(slices.Clone(src), filterIPFamily(*dst, false)...)
153+
*dst = slices.Concat(src, filterIPFamily(*dst, false))
156154
default:
157155
*dst = src
158156
}

pkg/machinery/resources/network/dns_upstream.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type DNSUpstreamSpecSpec struct {
2929
// We could use a generic struct here, but without generic aliases the usage would look ugly.
3030
// Once generic aliases are here, redo the type above as `type DNSUpstream[P Proxy] = typed.Resource[...]`.
3131
Prx Proxy
32+
Idx int
3233
}
3334

3435
// MarshalYAML implements yaml.Marshaler interface.
@@ -38,6 +39,7 @@ func (d *DNSUpstreamSpecSpec) MarshalYAML() (any, error) {
3839
return map[string]string{
3940
"healthy": strconv.FormatBool(d.Prx.Fails() == 0),
4041
"addr": d.Prx.Addr(),
42+
"idx": strconv.Itoa(d.Idx),
4143
}, nil
4244
}
4345

@@ -67,6 +69,10 @@ func (DNSUpstreamExtension) ResourceDefinition() meta.ResourceDefinitionSpec {
6769
Name: "Address",
6870
JSONPath: "{.addr}",
6971
},
72+
{
73+
Name: "Idx",
74+
JSONPath: "{.idx}",
75+
},
7076
},
7177
}
7278
}

0 commit comments

Comments
 (0)