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