From 49dc03e8ebbd114ca33e23c4f17e5dee93bbe390 Mon Sep 17 00:00:00 2001 From: Santosh Pillai Date: Mon, 7 Jun 2021 15:20:50 +0530 Subject: [PATCH] ceph: compare ceph commit hash for upgrade check Currently we only check the Major, Minor, Extra and Build number in the ceph version to determine if all daemons should be upgraded or not. This PR also checks the commit hash for upgrading all the daemons. This will be useful in cases where build number has not changed due to hotfix. For example: existing ceph version - ceph version 14.2.11-139.el8cp (b8e1f91c99491fb2e5ede748a1c0738ed158d0f5) nautilus (stable) new ceph version with hotfix - ceph version 14.2.11-139.0.hotfix.bz1959254.el8cp (5c0dc966af809fd1d429ec7bac48962a746af243) nautilus (stable) Signed-off-by: Santosh Pillai --- pkg/operator/ceph/cluster/version_test.go | 37 ++++++++++++- pkg/operator/ceph/version/version.go | 63 ++++++++++++++--------- pkg/operator/ceph/version/version_test.go | 62 +++++++++++----------- 3 files changed, 108 insertions(+), 54 deletions(-) diff --git a/pkg/operator/ceph/cluster/version_test.go b/pkg/operator/ceph/cluster/version_test.go index e403797c9..151e27f2c 100755 --- a/pkg/operator/ceph/cluster/version_test.go +++ b/pkg/operator/ceph/cluster/version_test.go @@ -95,7 +95,8 @@ func TestDiffImageSpecAndClusterRunningVersion(t *testing.T) { assert.True(t, m) // 5 test - spec version and running cluster versions are identical --> we upgrade - fakeImageVersion = cephver.CephVersion{Major: 14, Minor: 2, Extra: 2} + fakeImageVersion = cephver.CephVersion{Major: 14, Minor: 2, Extra: 2, + CommitID: "3a54b2b6d167d4a2a19e003a705696d4fe619afc"} fakeRunningVersions = []byte(` { "overall": { @@ -109,6 +110,40 @@ func TestDiffImageSpecAndClusterRunningVersion(t *testing.T) { m, err = diffImageSpecAndClusterRunningVersion(fakeImageVersion, dummyRunningVersions5) assert.NoError(t, err) assert.False(t, m) + + // 6 test - spec version and running cluster have different commit ID + fakeImageVersion = cephver.CephVersion{Major: 14, Minor: 2, Extra: 11, Build: 139, + CommitID: "5c0dc966af809fd1d429ec7bac48962a746af243"} + fakeRunningVersions = []byte(` + { + "overall": { + "ceph version 14.2.11-139.el8cp (3a54b2b6d167d4a2a19e003a705696d4fe619afc) nautilus (stable)": 2 + } + }`) + var dummyRunningVersions6 cephv1.CephDaemonsVersions + err = json.Unmarshal([]byte(fakeRunningVersions), &dummyRunningVersions6) + assert.NoError(t, err) + + m, err = diffImageSpecAndClusterRunningVersion(fakeImageVersion, dummyRunningVersions6) + assert.NoError(t, err) + assert.True(t, m) + + // 7 test - spec version and running cluster have same commit ID + fakeImageVersion = cephver.CephVersion{Major: 14, Minor: 2, Extra: 11, Build: 139, + CommitID: "3a54b2b6d167d4a2a19e003a705696d4fe619afc"} + fakeRunningVersions = []byte(` + { + "overall": { + "ceph version 14.2.11-139.el8cp (3a54b2b6d167d4a2a19e003a705696d4fe619afc) nautilus (stable)": 2 + } + }`) + var dummyRunningVersions7 cephv1.CephDaemonsVersions + err = json.Unmarshal([]byte(fakeRunningVersions), &dummyRunningVersions7) + assert.NoError(t, err) + + m, err = diffImageSpecAndClusterRunningVersion(fakeImageVersion, dummyRunningVersions7) + assert.NoError(t, err) + assert.False(t, m) } func TestMinVersion(t *testing.T) { diff --git a/pkg/operator/ceph/version/version.go b/pkg/operator/ceph/version/version.go index aab37fc11..fd1336061 100644 --- a/pkg/operator/ceph/version/version.go +++ b/pkg/operator/ceph/version/version.go @@ -27,10 +27,11 @@ import ( // CephVersion represents the Ceph version format type CephVersion struct { - Major int - Minor int - Extra int - Build int + Major int + Minor int + Extra int + Build int + CommitID string } const ( @@ -39,15 +40,15 @@ const ( var ( // Minimum supported version is 14.2.5 - Minimum = CephVersion{14, 2, 5, 0} + Minimum = CephVersion{14, 2, 5, 0, ""} // Nautilus Ceph version - Nautilus = CephVersion{14, 0, 0, 0} + Nautilus = CephVersion{14, 0, 0, 0, ""} // Octopus Ceph version - Octopus = CephVersion{15, 0, 0, 0} + Octopus = CephVersion{15, 0, 0, 0, ""} // Pacific Ceph version - Pacific = CephVersion{16, 0, 0, 0} + Pacific = CephVersion{16, 0, 0, 0, ""} // Quincy Ceph version - Quincy = CephVersion{17, 0, 0, 0} + Quincy = CephVersion{17, 0, 0, 0, ""} // cephVolumeLVMDiskSortingCephVersion introduced a major regression in c-v and thus is not suitable for production cephVolumeLVMDiskSortingCephVersion = CephVersion{Major: 14, Minor: 2, Extra: 13} @@ -65,6 +66,11 @@ var ( // So we need to detect the build version change buildVersionPattern = regexp.MustCompile(`ceph version (\d+)\.(\d+)\.(\d+)\-(\d+)`) + // for parsing the commit hash in the ceph --version output. For example: + // input = `ceph version 14.2.11-139 (5c0dc966af809fd1d429ec7bac48962a746af243) nautilus (stable)` + // output = [(5c0dc966af809fd1d429ec7bac48962a746af243) 5c0dc966af809fd1d429ec7bac48962a746af243] + commitIDPattern = regexp.MustCompile(`\(([^)]+)\)`) + logger = capnslog.NewPackageLogger("github.com/rook/rook", "cephver") ) @@ -98,37 +104,43 @@ func (v *CephVersion) ReleaseName() string { // ExtractCephVersion extracts the major, minor and extra digit of a Ceph release func ExtractCephVersion(src string) (*CephVersion, error) { var build int - m := versionPattern.FindStringSubmatch(src) - if m == nil { + var commitID string + versionMatch := versionPattern.FindStringSubmatch(src) + if versionMatch == nil { return nil, errors.Errorf("failed to parse version from: %q", src) } - major, err := strconv.Atoi(m[1]) + major, err := strconv.Atoi(versionMatch[1]) if err != nil { - return nil, errors.Errorf("failed to parse version major part: %q", m[1]) + return nil, errors.Errorf("failed to parse version major part: %q", versionMatch[1]) } - minor, err := strconv.Atoi(m[2]) + minor, err := strconv.Atoi(versionMatch[2]) if err != nil { - return nil, errors.Errorf("failed to parse version minor part: %q", m[2]) + return nil, errors.Errorf("failed to parse version minor part: %q", versionMatch[2]) } - extra, err := strconv.Atoi(m[3]) + extra, err := strconv.Atoi(versionMatch[3]) if err != nil { - return nil, errors.Errorf("failed to parse version extra part: %q", m[3]) + return nil, errors.Errorf("failed to parse version extra part: %q", versionMatch[3]) } // See if we are running on a build release - mm := buildVersionPattern.FindStringSubmatch(src) + buildVersionMatch := buildVersionPattern.FindStringSubmatch(src) // We don't need to handle any error here, so let's jump in only when "mm" has content - if mm != nil { - build, err = strconv.Atoi(mm[4]) + if buildVersionMatch != nil { + build, err = strconv.Atoi(buildVersionMatch[4]) if err != nil { - logger.Warningf("failed to convert version build number part %q to an integer, ignoring", mm[4]) + logger.Warningf("failed to convert version build number part %q to an integer, ignoring", buildVersionMatch[4]) } } - return &CephVersion{major, minor, extra, build}, nil + commitIDMatch := commitIDPattern.FindStringSubmatch(src) + if commitIDMatch != nil { + commitID = commitIDMatch[1] + } + + return &CephVersion{major, minor, extra, build, commitID}, nil } // Supported checks if a given release is supported @@ -228,7 +240,9 @@ func IsIdentical(a, b CephVersion) bool { if a.Minor == b.Minor { if a.Extra == b.Extra { if a.Build == b.Build { - return true + if a.CommitID == b.CommitID { + return true + } } } } @@ -260,6 +274,9 @@ func IsSuperior(a, b CephVersion) bool { if a.Build > b.Build { return true } + if a.CommitID != b.CommitID { + return true + } } } } diff --git a/pkg/operator/ceph/version/version_test.go b/pkg/operator/ceph/version/version_test.go index 51425d41f..bd7dbcc24 100644 --- a/pkg/operator/ceph/version/version_test.go +++ b/pkg/operator/ceph/version/version_test.go @@ -26,7 +26,7 @@ import ( func TestToString(t *testing.T) { assert.Equal(t, "14.0.0-0 nautilus", Nautilus.String()) assert.Equal(t, "15.0.0-0 octopus", Octopus.String()) - received := CephVersion{-1, 0, 0, 0} + received := CephVersion{-1, 0, 0, 0, ""} expected := fmt.Sprintf("-1.0.0-0 %s", unknownVersionString) assert.Equal(t, expected, received.String()) @@ -40,14 +40,16 @@ func TestCephVersionFormatted(t *testing.T) { func TestReleaseName(t *testing.T) { assert.Equal(t, "nautilus", Nautilus.ReleaseName()) assert.Equal(t, "octopus", Octopus.ReleaseName()) - ver := CephVersion{-1, 0, 0, 0} + ver := CephVersion{-1, 0, 0, 0, ""} assert.Equal(t, unknownVersionString, ver.ReleaseName()) } -func extractVersionHelper(t *testing.T, text string, major, minor, extra, build int) { +func extractVersionHelper(t *testing.T, text string, + major, minor, extra, build int, + commitID string) { v, err := ExtractCephVersion(text) if assert.NoError(t, err) { - assert.Equal(t, *v, CephVersion{major, minor, extra, build}) + assert.Equal(t, *v, CephVersion{major, minor, extra, build, commitID}) } } @@ -58,19 +60,19 @@ func TestExtractVersion(t *testing.T) { root@7a97f5a78bc6:/# ceph --version ceph version 13.2.6 (ae699615bac534ea496ee965ac6192cb7e0e07c1) mimic (stable) ` - extractVersionHelper(t, v0c, 13, 2, 6, 0) - extractVersionHelper(t, v0d, 13, 2, 6, 0) + extractVersionHelper(t, v0c, 13, 2, 6, 0, "ae699615bac534ea496ee965ac6192cb7e0e07c1") + extractVersionHelper(t, v0d, 13, 2, 6, 0, "ae699615bac534ea496ee965ac6192cb7e0e07c1") // development build - v1c := "ceph version 14.1.33-403-g7ba6bece41" + v1c := "ceph version 14.1.33-403-g7ba6bece41 (7ba6bece4187eda5d05a9b84211fe6ba8dd287bd) nautilus (rc)" v1d := ` bin/ceph --version *** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH *** ceph version 14.1.33-403-g7ba6bece41 (7ba6bece4187eda5d05a9b84211fe6ba8dd287bd) nautilus (rc) ` - extractVersionHelper(t, v1c, 14, 1, 33, 403) - extractVersionHelper(t, v1d, 14, 1, 33, 403) + extractVersionHelper(t, v1c, 14, 1, 33, 403, "7ba6bece4187eda5d05a9b84211fe6ba8dd287bd") + extractVersionHelper(t, v1d, 14, 1, 33, 403, "7ba6bece4187eda5d05a9b84211fe6ba8dd287bd") // build without git version info. it is possible to build the ceph tree // without a version number, but none of the container builds do this. @@ -129,13 +131,13 @@ func TestVersionAtLeast(t *testing.T) { assert.True(t, Octopus.IsAtLeast(Nautilus)) assert.True(t, Octopus.IsAtLeast(Octopus)) - assert.True(t, (&CephVersion{1, 0, 0, 0}).IsAtLeast(CephVersion{0, 0, 0, 0})) - assert.False(t, (&CephVersion{0, 0, 0, 0}).IsAtLeast(CephVersion{1, 0, 0, 0})) - assert.True(t, (&CephVersion{1, 1, 0, 0}).IsAtLeast(CephVersion{1, 0, 0, 0})) - assert.False(t, (&CephVersion{1, 0, 0, 0}).IsAtLeast(CephVersion{1, 1, 0, 0})) - assert.True(t, (&CephVersion{1, 1, 1, 0}).IsAtLeast(CephVersion{1, 1, 0, 0})) - assert.False(t, (&CephVersion{1, 1, 0, 0}).IsAtLeast(CephVersion{1, 1, 1, 0})) - assert.True(t, (&CephVersion{1, 1, 1, 0}).IsAtLeast(CephVersion{1, 1, 1, 0})) + assert.True(t, (&CephVersion{1, 0, 0, 0, ""}).IsAtLeast(CephVersion{0, 0, 0, 0, ""})) + assert.False(t, (&CephVersion{0, 0, 0, 0, ""}).IsAtLeast(CephVersion{1, 0, 0, 0, ""})) + assert.True(t, (&CephVersion{1, 1, 0, 0, ""}).IsAtLeast(CephVersion{1, 0, 0, 0, ""})) + assert.False(t, (&CephVersion{1, 0, 0, 0, ""}).IsAtLeast(CephVersion{1, 1, 0, 0, ""})) + assert.True(t, (&CephVersion{1, 1, 1, 0, ""}).IsAtLeast(CephVersion{1, 1, 0, 0, ""})) + assert.False(t, (&CephVersion{1, 1, 0, 0, ""}).IsAtLeast(CephVersion{1, 1, 1, 0, ""})) + assert.True(t, (&CephVersion{1, 1, 1, 0, ""}).IsAtLeast(CephVersion{1, 1, 1, 0, ""})) } func TestVersionAtLeastX(t *testing.T) { @@ -148,26 +150,26 @@ func TestVersionAtLeastX(t *testing.T) { } func TestIsIdentical(t *testing.T) { - assert.True(t, IsIdentical(CephVersion{14, 2, 2, 0}, CephVersion{14, 2, 2, 0})) - assert.False(t, IsIdentical(CephVersion{14, 2, 2, 0}, CephVersion{15, 2, 2, 0})) + assert.True(t, IsIdentical(CephVersion{14, 2, 2, 0, ""}, CephVersion{14, 2, 2, 0, ""})) + assert.False(t, IsIdentical(CephVersion{14, 2, 2, 0, ""}, CephVersion{15, 2, 2, 0, ""})) } func TestIsSuperior(t *testing.T) { - assert.False(t, IsSuperior(CephVersion{14, 2, 2, 0}, CephVersion{14, 2, 2, 0})) - assert.False(t, IsSuperior(CephVersion{14, 2, 2, 0}, CephVersion{15, 2, 2, 0})) - assert.True(t, IsSuperior(CephVersion{15, 2, 2, 0}, CephVersion{14, 2, 2, 0})) - assert.True(t, IsSuperior(CephVersion{15, 2, 2, 0}, CephVersion{15, 1, 3, 0})) - assert.True(t, IsSuperior(CephVersion{15, 2, 2, 0}, CephVersion{15, 2, 1, 0})) - assert.True(t, IsSuperior(CephVersion{15, 2, 2, 1}, CephVersion{15, 2, 1, 0})) + assert.False(t, IsSuperior(CephVersion{14, 2, 2, 0, ""}, CephVersion{14, 2, 2, 0, ""})) + assert.False(t, IsSuperior(CephVersion{14, 2, 2, 0, ""}, CephVersion{15, 2, 2, 0, ""})) + assert.True(t, IsSuperior(CephVersion{15, 2, 2, 0, ""}, CephVersion{14, 2, 2, 0, ""})) + assert.True(t, IsSuperior(CephVersion{15, 2, 2, 0, ""}, CephVersion{15, 1, 3, 0, ""})) + assert.True(t, IsSuperior(CephVersion{15, 2, 2, 0, ""}, CephVersion{15, 2, 1, 0, ""})) + assert.True(t, IsSuperior(CephVersion{15, 2, 2, 1, ""}, CephVersion{15, 2, 1, 0, ""})) } func TestIsInferior(t *testing.T) { - assert.False(t, IsInferior(CephVersion{14, 2, 2, 0}, CephVersion{14, 2, 2, 0})) - assert.False(t, IsInferior(CephVersion{15, 2, 2, 0}, CephVersion{14, 2, 2, 0})) - assert.True(t, IsInferior(CephVersion{14, 2, 2, 0}, CephVersion{15, 2, 2, 0})) - assert.True(t, IsInferior(CephVersion{15, 1, 3, 0}, CephVersion{15, 2, 2, 0})) - assert.True(t, IsInferior(CephVersion{15, 2, 1, 0}, CephVersion{15, 2, 2, 0})) - assert.True(t, IsInferior(CephVersion{15, 2, 1, 0}, CephVersion{15, 2, 2, 1})) + assert.False(t, IsInferior(CephVersion{14, 2, 2, 0, ""}, CephVersion{14, 2, 2, 0, ""})) + assert.False(t, IsInferior(CephVersion{15, 2, 2, 0, ""}, CephVersion{14, 2, 2, 0, ""})) + assert.True(t, IsInferior(CephVersion{14, 2, 2, 0, ""}, CephVersion{15, 2, 2, 0, ""})) + assert.True(t, IsInferior(CephVersion{15, 1, 3, 0, ""}, CephVersion{15, 2, 2, 0, ""})) + assert.True(t, IsInferior(CephVersion{15, 2, 1, 0, ""}, CephVersion{15, 2, 2, 0, ""})) + assert.True(t, IsInferior(CephVersion{15, 2, 1, 0, ""}, CephVersion{15, 2, 2, 1, ""})) } func TestValidateCephVersionsBetweenLocalAndExternalClusters(t *testing.T) {