Skip to content

Commit

Permalink
chore: account for resource sorting in dns upstream resource
Browse files Browse the repository at this point in the history
`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)
  • Loading branch information
DmitriyMV authored and smira committed Sep 13, 2024
1 parent c030eef commit a159ea9
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 15 deletions.
21 changes: 13 additions & 8 deletions internal/app/machined/pkg/controllers/network/dns_resolve_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package network

import (
"cmp"
"context"
"errors"
"fmt"
Expand Down Expand Up @@ -160,14 +161,7 @@ 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())

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
}
prxs, addrs := SortedProxies(upstreams)

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

// SortedProxies returns sorted list of proxies and their addresses.
func SortedProxies(upstreams safe.List[*network.DNSUpstream]) ([]*proxy.Proxy, []string) {
upstreams.SortFunc(func(a, b *network.DNSUpstream) int {
return cmp.Compare(a.TypedSpec().Value.Idx, b.TypedSpec().Value.Idx)
})

//nolint:forcetypeassert
return safe.ToSlice(upstreams, func(d *network.DNSUpstream) *proxy.Proxy { return d.TypedSpec().Value.Prx.(*proxy.Proxy) }),
safe.ToSlice(upstreams, func(d *network.DNSUpstream) string { return d.TypedSpec().Value.Prx.Addr() })
}

func (ctrl *DNSResolveCacheController) writeDNSStatus(ctx context.Context, r controller.Runtime, config runnerConfig) error {
return safe.WriterModify(ctx, r, network.NewDNSResolveCache(fmt.Sprintf("%s-%s", config.net, config.addr)), func(drc *network.DNSResolveCache) error {
drc.TypedSpec().Status = "running"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/cosi-project/runtime/pkg/resource"
"github.com/cosi-project/runtime/pkg/resource/rtestutils"
"github.com/cosi-project/runtime/pkg/safe"
"github.com/miekg/dns"
"github.com/siderolabs/gen/xslices"
"github.com/siderolabs/gen/xtesting/must"
Expand Down Expand Up @@ -284,3 +285,54 @@ func makeAddrs(port string) []netip.AddrPort {
netip.MustParseAddrPort("127.0.0.53:" + port),
}
}

type DNSUpstreams struct {
ctest.DefaultSuite
}

func (suite *DNSUpstreams) TestOrder() {
port := must.Value(getDynamicPort())(suite.T())

cfg := network.NewHostDNSConfig(network.HostDNSConfigID)
cfg.TypedSpec().Enabled = true
cfg.TypedSpec().ListenAddresses = makeAddrs(port)

suite.Require().NoError(suite.State().Create(suite.Ctx(), cfg))

resolverSpec := network.NewResolverStatus(network.NamespaceName, network.ResolverID)

for i, addrs := range [][]string{
{"1.0.0.1", "8.8.8.8", "1.1.1.1"},
{"1.1.1.1", "8.8.8.8", "1.0.0.1", "8.0.0.8"},
{"192.168.0.1"},
} {
resolverSpec.TypedSpec().DNSServers = xslices.Map(addrs, netip.MustParseAddr)

switch i {
case 0:
suite.Require().NoError(suite.State().Create(suite.Ctx(), resolverSpec))
default:
suite.Require().NoError(suite.State().Update(suite.Ctx(), resolverSpec))
}

rtestutils.AssertLength[*network.DNSUpstream](suite.Ctx(), suite.T(), suite.State(), len(addrs))

upstreams, err := safe.ReaderListAll[*network.DNSUpstream](suite.Ctx(), suite.State())
suite.Require().NoError(err)

_, upstreamAddrs := netctrl.SortedProxies(upstreams)

suite.Require().Equal(xslices.Map(addrs, func(t string) string { return t + ":53" }), upstreamAddrs)
}
}

func TestDNSUpstreams(t *testing.T) {
suite.Run(t, &DNSUpstreams{
DefaultSuite: ctest.DefaultSuite{
Timeout: 10 * time.Second,
AfterSetup: func(suite *ctest.DefaultSuite) {
suite.Require().NoError(suite.Runtime().RegisterController(&netctrl.DNSUpstreamController{}))
},
},
})
}
13 changes: 11 additions & 2 deletions internal/app/machined/pkg/controllers/network/dns_upstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -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](
Expand All @@ -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
}

Expand All @@ -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
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/machinery/resources/network/dns_upstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}

Expand Down Expand Up @@ -67,6 +69,10 @@ func (DNSUpstreamExtension) ResourceDefinition() meta.ResourceDefinitionSpec {
Name: "Address",
JSONPath: "{.addr}",
},
{
Name: "Idx",
JSONPath: "{.idx}",
},
},
}
}
Expand Down

0 comments on commit a159ea9

Please sign in to comment.