backups: select random node to download backup

When chosing a node to download our backup, we should prevent the same node
from downloading the backups all the time.

In the case we get a "preferred" topology, this is left to the kubernetes
scheduler/CSI controllers: they should select different preferred nodes.

In the case we have no preferred topology given to use, we should do our
own random selection. Otherwise we end up with the same node being chosen
to download backups all the time, while the others sit idle.

Signed-off-by: Moritz Wanzenböck <[email protected]>

CHANGELOG.md

@@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - New parameter `overProvision`: when set available capacity on a node is calculated by taking into account
   the reserved capacity in the pool based on existing volumes.
 
+### Changed
+
+- When not using topology, select a random node to download the backup. This should prevent the same node
+  being used to download all backups.
+
 ## [1.2.3] - 2023-08-31
 
 ### Changed

pkg/client/linstor.go

@@ -25,6 +25,7 @@ import (
 	"fmt"
 	"io"
 	"math"
+	"math/rand"
 	"os"
 	"regexp"
 	"sort"
@@ -2157,6 +2158,16 @@ func (s *Linstor) SortByPreferred(ctx context.Context, nodes []string, remotePol
 
 	order := 0
 
+	if len(preferred) == 0 {
+		// If there is no preferred topology, select a random order, so we don't always end up with the
+		// same selected node for downloading backups.
+		rand.Shuffle(len(nodes), func(i, j int) {
+			nodes[i], nodes[j] = nodes[j], nodes[i]
+		})
+
+		return nodes, nil
+	}
+
 	for _, pref := range preferred {
 		// First add the original node directly
 		nodes, err := s.client.NodesForTopology(ctx, pref.GetSegments())

pkg/client/linstor_test.go

@@ -23,6 +23,7 @@ package client
 import (
 	"context"
 	"encoding/json"
+	"math/rand"
 	"testing"
 
 	lapiconsts "github.com/LINBIT/golinstor"
@@ -368,7 +369,7 @@ func TestLinstor_SortByPreferred(t *testing.T) {
 			name:              "no-preferred",
 			nodes:             []string{"node-a", "node-b", "node-c"},
 			preferredTopology: nil,
-			expected:          []string{"node-a", "node-b", "node-c"},
+			expected:          []string{"node-a", "node-c", "node-b"},
 		},
 		{
 			name:              "one-preferred",
@@ -399,6 +400,7 @@ func TestLinstor_SortByPreferred(t *testing.T) {
 	for i := range testcases {
 		tcase := &testcases[i]
 		t.Run(tcase.name, func(t *testing.T) {
+			rand.Seed(1) // nolint:staticcheck // Deprecated but useful in this case, as we don't want to seed our own RNG just for this one function
 			actual, err := cl.SortByPreferred(context.Background(), tcase.nodes, tcase.policy, tcase.preferredTopology)
 			assert.NoError(t, err)
 			assert.Equal(t, tcase.expected, actual)