From d5c4eca3fb006a1323e2c77b2ca52455a02559cd Mon Sep 17 00:00:00 2001
From: Blaine Gardner <blaine.gardner@ibm.com>
Date: Mon, 2 Jun 2025 14:01:54 -0600
Subject: [PATCH] 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      | 18 ++++++++---
 pkg/daemon/ceph/client/upgrade_test.go | 44 ++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/pkg/daemon/ceph/client/upgrade.go b/pkg/daemon/ceph/client/upgrade.go
index e214320e1..98238ffe2 100644
--- a/pkg/daemon/ceph/client/upgrade.go
+++ b/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
diff --git a/pkg/daemon/ceph/client/upgrade_test.go b/pkg/daemon/ceph/client/upgrade_test.go
index 2c72b0dae..547d4de48 100644
--- a/pkg/daemon/ceph/client/upgrade_test.go
+++ b/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)
+}