Merge pull request #15943 from rook/mergify/bp/release-1.16/pr-15931

core: ensure consistent LeastUptodateDaemonVersion (backport #15931)

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)
+}