core: ensure consistent LeastUptodateDaemonVersion

LeastUptodateDaemonVersion() unmarshals Ceph's daemon versions JSON
output into `map[string]<type>` maps which are not guaranteed to iterate
in the same order as output by Ceph. Fix this method to no longer assume
iteration order is the same order as Ceph's output.

Signed-off-by: Blaine Gardner <blaine.gardner@ibm.com>

pkg/daemon/ceph/client/upgrade.go

@@ -234,15 +234,23 @@ func LeastUptodateDaemonVersion(context *clusterd.Context, clusterInfo *ClusterI
 	if err != nil {
 		return vv, errors.Wrap(err, "failed to find daemon map entry")
 	}
+
+	maxInt := 65535
+
+	vv = cephver.CephVersion{Major: maxInt, Minor: maxInt, Extra: maxInt, Build: maxInt, CommitID: ""}
 	for v := range r {
 		version, err := cephver.ExtractCephVersion(v)
 		if err != nil {
-			return vv, errors.Wrap(err, "failed to extract ceph version")
+			return cephver.CephVersion{}, errors.Wrap(err, "failed to extract ceph version")
 		}
-		vv = *version
-		// break right after the first iteration
-		// the first one is always the least up-to-date
-		break
+
+		if cephver.IsInferior(*version, vv) {
+			vv = *version
+		}
+	}
+
+	if vv.Major == maxInt {
+		return cephver.CephVersion{}, errors.Wrap(err, "failed to determine least ceph version")
 	}
 
 	return vv, nil

pkg/daemon/ceph/client/upgrade_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"context"
 	"encoding/json"
 	"testing"
 	"time"
@@ -25,6 +26,7 @@ import (
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	"github.com/rook/rook/pkg/clusterd"
 	"github.com/rook/rook/pkg/daemon/ceph/client/fake"
+	cephver "github.com/rook/rook/pkg/operator/ceph/version"
 	exectest "github.com/rook/rook/pkg/util/exec/test"
 	"github.com/stretchr/testify/assert"
 )
@@ -352,3 +354,45 @@ func TestOSDUpdateShouldCheckOkToStop(t *testing.T) {
 		assert.True(t, OSDUpdateShouldCheckOkToStop(context, clusterInfo))
 	})
 }
+
+func TestLeastUptodateDaemonVersion(t *testing.T) {
+	clusterCtx := &clusterd.Context{
+		Executor: &exectest.MockExecutor{
+			MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
+				if command != "ceph" || args[0] != "versions" {
+					panic("not a 'ceph versions' call")
+				}
+				return `{
+  "mon": {
+    "ceph version 20.3.0-661-g68f47b56 (68f47b56a9717515844599c880de2b56a7135786) tentacle (dev - Debug)": 2,
+    "ceph version 20.3.0-660-ababababa (abababababababababababababababababababab) tentacle (dev - Debug)": 1
+  },
+  "mgr": {
+    "ceph version 20.3.0-661-g68f47b56 (68f47b56a9717515844599c880de2b56a7135786) tentacle (dev - Debug)": 1
+  },
+  "osd": {
+    "ceph version 20.3.0-661-g68f47b56 (68f47b56a9717515844599c880de2b56a7135786) tentacle (dev - Debug)": 4
+  },
+  "overall": {
+    "ceph version 20.3.0-661-g68f47b56 (68f47b56a9717515844599c880de2b56a7135786) tentacle (dev - Debug)": 8
+  }
+}`, nil
+			},
+		},
+	}
+	clusterInfo := ClusterInfo{}
+	ctx := context.TODO()
+	clusterInfo.Context = ctx
+
+	passed := 0
+	iterations := 100
+	for i := range iterations { // iterate the test to ensure consistent detection (#15930)
+		got, err := LeastUptodateDaemonVersion(clusterCtx, &clusterInfo, "mon")
+		assert.NoError(t, err)
+		pass := assert.Equalf(t, cephver.CephVersion{Major: 20, Minor: 3, Extra: 0, Build: 660, CommitID: "abababababababababababababababababababab"}, got, "i=%d: got %#v", i, got)
+		if pass {
+			passed++
+		}
+	}
+	assert.Equal(t, iterations, passed)
+}