Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: account for resource sorting in dns upstream resource #9310

Merged
merged 1 commit into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading