From 08ee400fdbde368a54d6777cc31ceb91e1968ad2 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 13 Dec 2024 14:43:31 +0400 Subject: [PATCH] test: fix flaky test NodeAddressSort There were two issues which showed up specifically under `race` tests: 1. As the address resources are added while the controller is running, and `default` address is immutable (by design), insert the future default address first, otherwise the controller might pick up another one it sees first randomly. 2. There was a bug in accumulative address handling when the sort only took into account addresses ignoring prefix lengths. Signed-off-by: Andrey Smirnov --- .../pkg/controllers/network/node_address.go | 14 +++++++------- .../pkg/controllers/network/node_address_test.go | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/app/machined/pkg/controllers/network/node_address.go b/internal/app/machined/pkg/controllers/network/node_address.go index b1950e500..ee2bcb2c9 100644 --- a/internal/app/machined/pkg/controllers/network/node_address.go +++ b/internal/app/machined/pkg/controllers/network/node_address.go @@ -224,7 +224,7 @@ func (ctrl *NodeAddressController) Run(ctx context.Context, r controller.Runtime touchedIDs[network.NodeAddressRoutedID] = struct{}{} - if err = ctrl.updateAccumulativeAddresses(ctx, r, network.NodeAddressAccumulativeID, accumulative, algo); err != nil { + if err = ctrl.updateAccumulativeAddresses(ctx, r, network.NodeAddressAccumulativeID, accumulative, algo, compareFunc); err != nil { return err } @@ -247,7 +247,7 @@ func (ctrl *NodeAddressController) Run(ctx context.Context, r controller.Runtime return err } - if err = ctrl.updateAccumulativeAddresses(ctx, r, network.FilteredNodeAddressID(network.NodeAddressAccumulativeID, filterID), filteredAccumulative, algo); err != nil { + if err = ctrl.updateAccumulativeAddresses(ctx, r, network.FilteredNodeAddressID(network.NodeAddressAccumulativeID, filterID), filteredAccumulative, algo, compareFunc); err != nil { return err } @@ -293,17 +293,17 @@ func (ctrl *NodeAddressController) updateCurrentAddresses(ctx context.Context, r return nil } -func (ctrl *NodeAddressController) updateAccumulativeAddresses(ctx context.Context, r controller.Runtime, id resource.ID, accumulative []netip.Prefix, algo nethelpers.AddressSortAlgorithm) error { +func (ctrl *NodeAddressController) updateAccumulativeAddresses( + ctx context.Context, r controller.Runtime, id resource.ID, accumulative []netip.Prefix, algo nethelpers.AddressSortAlgorithm, compare func(a, b netip.Prefix) int, +) error { if err := safe.WriterModify(ctx, r, network.NewNodeAddress(network.NamespaceName, id), func(r *network.NodeAddress) error { spec := r.TypedSpec() for _, ip := range accumulative { // find insert position using binary search - pos, _ := slices.BinarySearchFunc(spec.Addresses, ip.Addr(), func(prefix netip.Prefix, addr netip.Addr) int { - return prefix.Addr().Compare(ip.Addr()) - }) + pos, _ := slices.BinarySearchFunc(spec.Addresses, ip, compare) - if pos < len(spec.Addresses) && spec.Addresses[pos].Addr().Compare(ip.Addr()) == 0 { + if pos < len(spec.Addresses) && compare(spec.Addresses[pos], ip) == 0 { continue } diff --git a/internal/app/machined/pkg/controllers/network/node_address_test.go b/internal/app/machined/pkg/controllers/network/node_address_test.go index 7fbea879a..ead2f4c08 100644 --- a/internal/app/machined/pkg/controllers/network/node_address_test.go +++ b/internal/app/machined/pkg/controllers/network/node_address_test.go @@ -227,10 +227,10 @@ func (suite *NodeAddressSuite) TestSortAlgorithmV2() { suite.Require().NoError(suite.State().Create(suite.Ctx(), sortAlgorithm)) for _, addr := range []string{ + "1.2.3.4/26", // insert default address first, otherwise the test would be flaky, as default address is immutable "10.3.4.1/24", "10.3.4.5/24", "10.3.4.5/32", - "1.2.3.4/26", "192.168.35.11/24", "192.168.36.10/24", "127.0.0.1/8", @@ -269,7 +269,7 @@ func (suite *NodeAddressSuite) TestSortAlgorithmV2() { ) case network.NodeAddressAccumulativeID: asrt.Equal( - "1.2.3.4/26 10.0.0.2/8 10.3.4.1/24 10.3.4.5/32 192.168.3.7/24 192.168.35.11/24 192.168.36.10/24 fd01:cafe::5054:ff:fe1f:c7bd/64 fd01:cafe::f14c:9fa1:8496:557f/128", + "1.2.3.4/26 10.3.4.5/32 10.3.4.1/24 10.3.4.5/24 10.0.0.2/8 192.168.3.7/24 192.168.35.11/24 192.168.36.10/24 fd01:cafe::f14c:9fa1:8496:557f/128 fd01:cafe::5054:ff:fe1f:c7bd/64", stringifyIPs(addrs), ) }