Skip to content

Commit

Permalink
chore: improve link spec controller code
Browse files Browse the repository at this point in the history 
`SortBonds` function bothered me since the last time I refactored this part.

We always know that it only accepts `network.LinkSpec`s, but we accepted the slice of untyped Resources because
this is what `List` method returns. Now we can do better, since `safe.List` now supports `Swap` method.

We can utilize `sort.Interface` and pass `safe.List` directly to `SortBonds`.

Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
  • Loading branch information
@DmitriyMV
DmitriyMV committed Jul 5, 2024
1 parent 0454130 commit 076f3c4
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 63 deletions.
6 changes: 3 additions & 3 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ require (
github.com/containernetworking/plugins v1.5.1
github.com/coredns/coredns v1.11.2
github.com/coreos/go-iptables v0.7.0
github.com/cosi-project/runtime v0.5.0
github.com/cosi-project/runtime v0.5.1
github.com/distribution/reference v0.6.0
github.com/docker/docker v27.0.3+incompatible
github.com/docker/go-connections v0.5.0
Expand Down Expand Up @@ -171,7 +171,7 @@ require (
golang.org/x/text v0.16.0
golang.org/x/time v0.5.0
golang.zx2c4.com/wireguard/wgctrl v0.0.0-20230429144221-925a1e7659e6
google.golang.org/grpc v1.64.0
google.golang.org/grpc v1.65.0
google.golang.org/protobuf v1.34.2
gopkg.in/yaml.v3 v3.0.1
k8s.io/klog/v2 v2.130.1
Expand Down Expand Up @@ -209,7 +209,7 @@ require (
github.com/aws/aws-sdk-go-v2/service/sts v1.30.1 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/chai2010/gettext-go v1.0.2 // indirect
github.com/cilium/ebpf v0.12.3 // indirect
github.com/cloudflare/circl v1.3.9 // indirect
Expand Down
12 changes: 6 additions & 6 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ github.com/bwesterb/go-ristretto v1.2.3/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7N
github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/chai2010/gettext-go v1.0.2 h1:1Lwwip6Q2QGsAdl/ZKPCwTe9fe0CjlUbqj5bFNSjIRk=
github.com/chai2010/gettext-go v1.0.2/go.mod h1:y+wnP2cHYaVj19NZhYKAwEMH2CI1gNHeQQ+5AjwawxA=
github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
Expand Down Expand Up @@ -187,8 +187,8 @@ github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr
github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03VsM8rvUec=
github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs=
github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
github.com/cosi-project/runtime v0.5.0 h1:Thin3w4ZQbRgHIv2h6a+zNGvp7N85eO8VEz0Kp859Jo=
github.com/cosi-project/runtime v0.5.0/go.mod h1:GH/EZSmCeMpOFNeZYZnlmdB14mj0zLyREA4PqOI0ubg=
github.com/cosi-project/runtime v0.5.1 h1:07r4lk8sgiyhLdRuqZidB6qV3jFpKYhGccWdhTYHYcc=
github.com/cosi-project/runtime v0.5.1/go.mod h1:m+bkfUzKYeUyoqYAQBxdce3bfgncG8BsqcbfKRbvJKs=
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/cpuguy83/go-md2man/v2 v2.0.4 h1:wfIWP927BUkWJb2NmU/kNDYIBTh/ziUX91+lVfRxZq4=
github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
Expand Down Expand Up @@ -1173,8 +1173,8 @@ google.golang.org/grpc v1.31.1/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM
google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc=
google.golang.org/grpc v1.34.0/go.mod h1:WotjhfgOW/POjDeRt8vscBtXq+2VjORFy659qA51WJ8=
google.golang.org/grpc v1.35.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU=
google.golang.org/grpc v1.64.0 h1:KH3VH9y/MgNQg1dE7b3XfVK0GsPSIzJwdF617gUSbvY=
google.golang.org/grpc v1.64.0/go.mod h1:oxjF8E3FBnjp+/gVFYdWacaLDx9na1aqy9oovLpxQYg=
google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc=
google.golang.org/grpc v1.65.0/go.mod h1:WgYC2ypjlB0EiQi6wdKixMqukr6lBc0Vo+oOgjrM5ZQ=
google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM=
Expand Down
2 changes: 0 additions & 2 deletions hack/docgen/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8=
github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
github.com/gomarkdown/markdown v0.0.0-20240419095408-642f0ee99ae2 h1:yEt5djSYb4iNtmV9iJGVday+i4e9u6Mrn5iP64HH5QM=
github.com/gomarkdown/markdown v0.0.0-20240419095408-642f0ee99ae2/go.mod h1:JDGcbDT52eL4fju3sZ4TeHGsQwhG9nbDV21aMyhwPoA=
github.com/gomarkdown/markdown v0.0.0-20240626202925-2eda941fd024 h1:saBP362Qm7zDdDXqv61kI4rzhmLFq3Z1gx34xpl6cWE=
github.com/gomarkdown/markdown v0.0.0-20240626202925-2eda941fd024/go.mod h1:JDGcbDT52eL4fju3sZ4TeHGsQwhG9nbDV21aMyhwPoA=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
Expand Down
56 changes: 36 additions & 20 deletions internal/app/machined/pkg/controllers/network/link_spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,15 @@ func (ctrl *LinkSpecController) Run(ctx context.Context, r controller.Runtime, l
}

// list source network configuration resources
list, err := r.List(ctx, resource.NewMetadata(network.NamespaceName, network.LinkSpecType, "", resource.VersionUndefined))
list, err := safe.ReaderList[*network.LinkSpec](ctx, r, resource.NewMetadata(network.NamespaceName, network.LinkSpecType, "", resource.VersionUndefined))
if err != nil {
return fmt.Errorf("error listing source network addresses: %w", err)
}

// add finalizers for all live resources
for _, res := range list.Items {
for it := list.Iterator(); it.Next(); {
res := it.Value()

if res.Metadata().Phase() != resource.PhaseRunning {
continue
}
Expand All @@ -123,10 +125,10 @@ func (ctrl *LinkSpecController) Run(ctx context.Context, r controller.Runtime, l
// loop over links and make reconcile decision
var multiErr *multierror.Error

SortBonds(list.Items)
SortBonds(&list)

for _, res := range list.Items {
link := res.(*network.LinkSpec) //nolint:forcetypeassert,errcheck
for it := list.Iterator(); it.Next(); {
link := it.Value()

if err = ctrl.syncLink(ctx, r, logger, conn, wgClient, &links, link); err != nil {
multiErr = multierror.Append(multiErr, err)
Expand All @@ -143,23 +145,37 @@ func (ctrl *LinkSpecController) Run(ctx context.Context, r controller.Runtime, l

// SortBonds sort resources in increasing order, except it places slave interfaces right after the bond
// in proper order.
func SortBonds(items []resource.Resource) {
sort.Slice(items, func(i, j int) bool {
left := items[i].(*network.LinkSpec).TypedSpec()
right := items[j].(*network.LinkSpec).TypedSpec()

l := ordered.MakeTriple(left.Name, 0, "")
if left.BondSlave.MasterName != "" {
l = ordered.MakeTriple(left.BondSlave.MasterName, left.BondSlave.SlaveIndex, left.Name)
}
func SortBonds(items LinkSpecs) {
sort.Sort(&struct {
LinkSpecs
lessLinkSpecs
}{items, lessLinkSpecs{items}})
}

r := ordered.MakeTriple(right.Name, 0, "")
if right.BondSlave.MasterName != "" {
r = ordered.MakeTriple(right.BondSlave.MasterName, right.BondSlave.SlaveIndex, right.Name)
}
type lessLinkSpecs struct{ LinkSpecs }

func (ls lessLinkSpecs) Less(i, j int) bool {
left := ls.Get(i).TypedSpec()
right := ls.Get(j).TypedSpec()

l := ordered.MakeTriple(left.Name, 0, "")
if left.BondSlave.MasterName != "" {
l = ordered.MakeTriple(left.BondSlave.MasterName, left.BondSlave.SlaveIndex, left.Name)
}

r := ordered.MakeTriple(right.Name, 0, "")
if right.BondSlave.MasterName != "" {
r = ordered.MakeTriple(right.BondSlave.MasterName, right.BondSlave.SlaveIndex, right.Name)
}

return l.LessThan(r)
}

return l.LessThan(r)
})
// LinkSpecs is a sortable collection of network.LinkSpec.
type LinkSpecs interface {
Len() int
Swap(i, j int)
Get(i int) *network.LinkSpec
}

func findLink(links []rtnetlink.LinkMessage, name string) *rtnetlink.LinkMessage {
Expand Down
37 changes: 17 additions & 20 deletions internal/app/machined/pkg/controllers/network/link_spec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/cosi-project/runtime/pkg/controller/runtime"
"github.com/cosi-project/runtime/pkg/resource"
"github.com/cosi-project/runtime/pkg/safe"
"github.com/cosi-project/runtime/pkg/state"
"github.com/cosi-project/runtime/pkg/state/impl/inmem"
"github.com/cosi-project/runtime/pkg/state/impl/namespaced"
Expand Down Expand Up @@ -918,7 +919,7 @@ func TestLinkSpecSuite(t *testing.T) {
}

func TestSortBonds(t *testing.T) {
expectedSlice := []network.LinkSpecSpec{
expected := toResources([]network.LinkSpecSpec{
{
Name: "A",
}, {
Expand Down Expand Up @@ -948,34 +949,30 @@ func TestSortBonds(t *testing.T) {
SlaveIndex: 2,
},
},
}
})

seed := time.Now().Unix()

rnd := rand.New(rand.NewPCG(uint64(time.Now().Unix()), uint64(time.Now().Unix())))

for i := range 100 {
res := toResources(expectedSlice)
rnd.Shuffle(len(res), func(i, j int) { res[i], res[j] = res[j], res[i] })
netctrl.SortBonds(res)
sorted := toSpecs(res)
require.Equal(t, expectedSlice, sorted, "failed with seed %d iteration %d", seed, i)
}
}
res := safe.NewList[*network.LinkSpec](resource.List{
Items: safe.ToSlice(expected, func(r *network.LinkSpec) resource.Resource { return r }),
})

func toResources(slice []network.LinkSpecSpec) []resource.Resource {
return xslices.Map(slice, func(spec network.LinkSpecSpec) resource.Resource {
link := network.NewLinkSpec(network.NamespaceName, "bar")
*link.TypedSpec() = spec

return link
})
rnd.Shuffle(res.Len(), res.Swap)
netctrl.SortBonds(&res)
require.Equal(t, expected, res, "failed with seed %d iteration %d", seed, i)
}
}

func toSpecs(slice []resource.Resource) []network.LinkSpecSpec {
return xslices.Map(slice, func(r resource.Resource) network.LinkSpecSpec {
v := r.Spec().(*network.LinkSpecSpec) //nolint:errcheck
func toResources(slice []network.LinkSpecSpec) safe.List[*network.LinkSpec] {
return safe.NewList[*network.LinkSpec](resource.List{
Items: xslices.Map(slice, func(spec network.LinkSpecSpec) resource.Resource {
link := network.NewLinkSpec(network.NamespaceName, "bar")
*link.TypedSpec() = spec

return *v
return link
}),
})
}
2 changes: 1 addition & 1 deletion internal/app/machined/pkg/controllers/network/nftables_chain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (ctrl *NfTablesChainController) Run(ctx context.Context, r controller.Runti
return fmt.Errorf("error flushing nftables: %w", err)
}

chainNames, _ := safe.Map(list, func(chain *network.NfTablesChain) (string, error) { return chain.Metadata().ID(), nil }) //nolint:errcheck // doesn't fail
chainNames := safe.ToSlice(list, func(chain *network.NfTablesChain) string { return chain.Metadata().ID() })
logger.Info("nftables chains updated", zap.Strings("chains", chainNames))

r.ResetRestartBackoff()
Expand Down
8 changes: 3 additions & 5 deletions internal/app/storaged/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (s *Server) Disks(ctx context.Context, in *emptypb.Empty) (reply *storage.D
return nil, err
}

diskConv := func(d *block.Disk) (*storage.Disk, error) {
diskConv := func(d *block.Disk) *storage.Disk {
var diskType storage.Disk_DiskType

switch {
Expand Down Expand Up @@ -68,15 +68,13 @@ func (s *Server) Disks(ctx context.Context, in *emptypb.Empty) (reply *storage.D
SystemDisk: systemDisk != nil && d.Metadata().ID() == systemDisk.TypedSpec().DiskID,
Subsystem: d.TypedSpec().SubSystem,
Readonly: d.TypedSpec().Readonly,
}, nil
}
}

diskList, _ := safe.Map(disks, diskConv) //nolint:errcheck

reply = &storage.DisksResponse{
Messages: []*storage.Disks{
{
Disks: diskList,
Disks: safe.ToSlice(disks, diskConv),
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/machinery/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ replace gopkg.in/yaml.v3 => github.com/unix4ever/yaml v0.0.0-20220527175918-f17b
require (
github.com/blang/semver/v4 v4.0.0
github.com/containerd/go-cni v1.1.10
github.com/cosi-project/runtime v0.5.0
github.com/cosi-project/runtime v0.5.1
github.com/dustin/go-humanize v1.0.1
github.com/emicklei/dot v1.6.2
github.com/evanphx/json-patch v5.9.0+incompatible
Expand All @@ -29,7 +29,7 @@ require (
github.com/siderolabs/protoenc v0.2.1
github.com/stretchr/testify v1.9.0
google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094
google.golang.org/grpc v1.64.0
google.golang.org/grpc v1.65.0
google.golang.org/protobuf v1.34.2
gopkg.in/yaml.v3 v3.0.1
)
Expand Down
8 changes: 4 additions & 4 deletions pkg/machinery/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ github.com/containerd/go-cni v1.1.10 h1:c2U73nld7spSWfiJwSh/8W9DK+/qQwYM2rngIhCy
github.com/containerd/go-cni v1.1.10/go.mod h1:/Y/sL8yqYQn1ZG1om1OncJB1W4zN3YmjfP/ShCzG/OY=
github.com/containernetworking/cni v1.2.2 h1:9IbP6KJQQxVKo4hhnm8r50YcVKrJbJu3Dqw+Rbt1vYk=
github.com/containernetworking/cni v1.2.2/go.mod h1:DuLgF+aPd3DzcTQTtp/Nvl1Kim23oFKdm2okJzBQA5M=
github.com/cosi-project/runtime v0.5.0 h1:Thin3w4ZQbRgHIv2h6a+zNGvp7N85eO8VEz0Kp859Jo=
github.com/cosi-project/runtime v0.5.0/go.mod h1:GH/EZSmCeMpOFNeZYZnlmdB14mj0zLyREA4PqOI0ubg=
github.com/cosi-project/runtime v0.5.1 h1:07r4lk8sgiyhLdRuqZidB6qV3jFpKYhGccWdhTYHYcc=
github.com/cosi-project/runtime v0.5.1/go.mod h1:m+bkfUzKYeUyoqYAQBxdce3bfgncG8BsqcbfKRbvJKs=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
Expand Down Expand Up @@ -183,8 +183,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20240617180043-68d350f18fd4 h1:
google.golang.org/genproto/googleapis/api v0.0.0-20240617180043-68d350f18fd4/go.mod h1:px9SlOOZBg1wM1zdnr8jEL4CNGUBZ+ZKYtNPApNQc4c=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 h1:BwIjyKYGsK9dMCBOorzRri8MQwmi7mT9rGHsCEinZkA=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094/go.mod h1:Ue6ibwXGpU+dqIcODieyLOcgj7z8+IcskoNIgZxtrFY=
google.golang.org/grpc v1.64.0 h1:KH3VH9y/MgNQg1dE7b3XfVK0GsPSIzJwdF617gUSbvY=
google.golang.org/grpc v1.64.0/go.mod h1:oxjF8E3FBnjp+/gVFYdWacaLDx9na1aqy9oovLpxQYg=
google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc=
google.golang.org/grpc v1.65.0/go.mod h1:WgYC2ypjlB0EiQi6wdKixMqukr6lBc0Vo+oOgjrM5ZQ=
google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg=
google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down

0 comments on commit 076f3c4

Please sign in to comment.