From 076f3c4f20006f732fa07ada14f45458dc65a9e8 Mon Sep 17 00:00:00 2001 From: Dmitriy Matrenichev Date: Thu, 4 Jul 2024 23:19:16 +0300 Subject: [PATCH] chore: improve link spec controller code `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 --- go.mod | 6 +- go.sum | 12 ++-- hack/docgen/go.sum | 2 - .../pkg/controllers/network/link_spec.go | 56 ++++++++++++------- .../pkg/controllers/network/link_spec_test.go | 37 ++++++------ .../pkg/controllers/network/nftables_chain.go | 2 +- internal/app/storaged/server.go | 8 +-- pkg/machinery/go.mod | 4 +- pkg/machinery/go.sum | 8 +-- 9 files changed, 72 insertions(+), 63 deletions(-) diff --git a/go.mod b/go.mod index eb4a4b09c3..447bb88a32 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -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 @@ -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 diff --git a/go.sum b/go.sum index 8e41d28ad9..c85f484476 100644 --- a/go.sum +++ b/go.sum @@ -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= @@ -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= @@ -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= diff --git a/hack/docgen/go.sum b/hack/docgen/go.sum index 5e223ce7f1..cfd462016e 100644 --- a/hack/docgen/go.sum +++ b/hack/docgen/go.sum @@ -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= diff --git a/internal/app/machined/pkg/controllers/network/link_spec.go b/internal/app/machined/pkg/controllers/network/link_spec.go index 12b8d2f3fc..ec7739e3bf 100644 --- a/internal/app/machined/pkg/controllers/network/link_spec.go +++ b/internal/app/machined/pkg/controllers/network/link_spec.go @@ -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 } @@ -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) @@ -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 { diff --git a/internal/app/machined/pkg/controllers/network/link_spec_test.go b/internal/app/machined/pkg/controllers/network/link_spec_test.go index c017a6da0d..4e4eebab09 100644 --- a/internal/app/machined/pkg/controllers/network/link_spec_test.go +++ b/internal/app/machined/pkg/controllers/network/link_spec_test.go @@ -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" @@ -918,7 +919,7 @@ func TestLinkSpecSuite(t *testing.T) { } func TestSortBonds(t *testing.T) { - expectedSlice := []network.LinkSpecSpec{ + expected := toResources([]network.LinkSpecSpec{ { Name: "A", }, { @@ -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 + }), }) } diff --git a/internal/app/machined/pkg/controllers/network/nftables_chain.go b/internal/app/machined/pkg/controllers/network/nftables_chain.go index 9fbbc70a10..f4485e5fa2 100644 --- a/internal/app/machined/pkg/controllers/network/nftables_chain.go +++ b/internal/app/machined/pkg/controllers/network/nftables_chain.go @@ -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() diff --git a/internal/app/storaged/server.go b/internal/app/storaged/server.go index 3ba9d78899..529bedad1d 100644 --- a/internal/app/storaged/server.go +++ b/internal/app/storaged/server.go @@ -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 { @@ -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), }, }, } diff --git a/pkg/machinery/go.mod b/pkg/machinery/go.mod index 1517e3d057..67a7b53138 100644 --- a/pkg/machinery/go.mod +++ b/pkg/machinery/go.mod @@ -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 @@ -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 ) diff --git a/pkg/machinery/go.sum b/pkg/machinery/go.sum index fcd6274165..4780d11fb8 100644 --- a/pkg/machinery/go.sum +++ b/pkg/machinery/go.sum @@ -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= @@ -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=