From 74ea46840cecad6c2560ce85adb79ac0f35d3b61 Mon Sep 17 00:00:00 2001 From: Redouane Kachach Date: Wed, 13 Apr 2022 16:22:53 +0200 Subject: [PATCH] mgr/cephadm: fixing upgrade candidate verions listing Fixes: https://tracker.ceph.com/issues/53527 Signed-off-by: Redouane Kachach (cherry picked from commit 671442c4b445c4a0547978a6298645a5c2f5725f) --- .../orch/cephadm/upgrade/5-upgrade-ls.yaml | 2 +- src/pybind/mgr/cephadm/module.py | 4 +- src/pybind/mgr/cephadm/tests/test_upgrade.py | 85 +++++++++++++++++++ src/pybind/mgr/cephadm/upgrade.py | 39 ++++++--- src/pybind/mgr/orchestrator/_interface.py | 2 +- src/pybind/mgr/orchestrator/module.py | 6 +- 6 files changed, 118 insertions(+), 20 deletions(-) diff --git a/qa/suites/orch/cephadm/upgrade/5-upgrade-ls.yaml b/qa/suites/orch/cephadm/upgrade/5-upgrade-ls.yaml index 236ea6c6bae2d..799458bc5aeab 100644 --- a/qa/suites/orch/cephadm/upgrade/5-upgrade-ls.yaml +++ b/qa/suites/orch/cephadm/upgrade/5-upgrade-ls.yaml @@ -2,5 +2,5 @@ tasks: - cephadm.shell: mon.a: - ceph orch upgrade ls - - ceph orch upgrade ls --image quay.io/ceph/ceph | grep 16.2.0 + - ceph orch upgrade ls --image quay.io/ceph/ceph --show-all-versions | grep 16.2.0 - ceph orch upgrade ls --image quay.io/ceph/ceph --tags | grep v16.2.2 diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index 5ae3deda07f38..d5777a6876da5 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -2642,8 +2642,8 @@ Then run the following: return self.upgrade.upgrade_status() @handle_orch_error - def upgrade_ls(self, image: Optional[str], tags: bool) -> Dict[Any, Any]: - return self.upgrade.upgrade_ls(image, tags) + def upgrade_ls(self, image: Optional[str], tags: bool, show_all_versions: Optional[bool]) -> Dict[Any, Any]: + return self.upgrade.upgrade_ls(image, tags, show_all_versions) @handle_orch_error def upgrade_start(self, image: str, version: str) -> str: diff --git a/src/pybind/mgr/cephadm/tests/test_upgrade.py b/src/pybind/mgr/cephadm/tests/test_upgrade.py index 32aacf263f39f..522518c741aa1 100644 --- a/src/pybind/mgr/cephadm/tests/test_upgrade.py +++ b/src/pybind/mgr/cephadm/tests/test_upgrade.py @@ -164,3 +164,88 @@ def test_enough_mds_for_ok_to_stop(get, get_daemons_by_service, cephadm_module: get_daemons_by_service.side_effect = [[DaemonDescription(), DaemonDescription()]] assert cephadm_module.upgrade._enough_mds_for_ok_to_stop( DaemonDescription(daemon_type='mds', daemon_id='myfs.test.host1.gfknd', service_name='mds.myfs.test')) + + +@pytest.mark.parametrize("current_version, use_tags, show_all_versions, tags, result", + [ + # several candidate versions (from different major versions) + ( + (16, 1, '16.1.0'), + False, # use_tags + False, # show_all_versions + [ + 'v17.1.0', + 'v16.2.7', + 'v16.2.6', + 'v16.2.5', + 'v16.1.4', + 'v16.1.3', + 'v15.2.0', + ], + ['17.1.0', '16.2.7', '16.2.6', '16.2.5', '16.1.4', '16.1.3'] + ), + # candidate minor versions are available + ( + (16, 1, '16.1.0'), + False, # use_tags + False, # show_all_versions + [ + 'v16.2.2', + 'v16.2.1', + 'v16.1.6', + ], + ['16.2.2', '16.2.1', '16.1.6'] + ), + # all versions are less than the current version + ( + (17, 2, '17.2.0'), + False, # use_tags + False, # show_all_versions + [ + 'v17.1.0', + 'v16.2.7', + 'v16.2.6', + ], + [] + ), + # show all versions (regardless of the current version) + ( + (16, 1, '16.1.0'), + False, # use_tags + True, # show_all_versions + [ + 'v17.1.0', + 'v16.2.7', + 'v16.2.6', + 'v15.1.0', + 'v14.2.0', + ], + ['17.1.0', '16.2.7', '16.2.6', '15.1.0', '14.2.0'] + ), + # show all tags (regardless of the current version and show_all_versions flag) + ( + (16, 1, '16.1.0'), + True, # use_tags + False, # show_all_versions + [ + 'v17.1.0', + 'v16.2.7', + 'v16.2.6', + 'v16.2.5', + 'v16.1.4', + 'v16.1.3', + 'v15.2.0', + ], + ['v15.2.0', 'v16.1.3', 'v16.1.4', 'v16.2.5', + 'v16.2.6', 'v16.2.7', 'v17.1.0'] + ), + ]) +@mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}')) +def test_upgrade_ls(current_version, use_tags, show_all_versions, tags, result, cephadm_module: CephadmOrchestrator): + with mock.patch('cephadm.upgrade.Registry.get_tags', return_value=tags): + with mock.patch('cephadm.upgrade.CephadmUpgrade._get_current_version', return_value=current_version): + out = cephadm_module.upgrade.upgrade_ls(None, use_tags, show_all_versions) + if use_tags: + assert out['tags'] == result + else: + assert out['versions'] == result diff --git a/src/pybind/mgr/cephadm/upgrade.py b/src/pybind/mgr/cephadm/upgrade.py index f59752db9b2fd..bb157e7f4d760 100644 --- a/src/pybind/mgr/cephadm/upgrade.py +++ b/src/pybind/mgr/cephadm/upgrade.py @@ -161,47 +161,53 @@ class CephadmUpgrade: return '%s/%s daemons upgraded' % (done, len(daemons)), completed_types + def _get_current_version(self) -> Tuple[int, int, str]: + current_version = self.mgr.version.split('ceph version ')[1] + (current_major, current_minor, _) = current_version.split('-')[0].split('.', 2) + return (int(current_major), int(current_minor), current_version) + def _check_target_version(self, version: str) -> Optional[str]: try: - (major, minor, _) = version.split('.', 2) - assert int(minor) >= 0 + v = version.split('.', 2) + (major, minor) = (int(v[0]), int(v[1])) + assert minor >= 0 # patch might be a number or {number}-g{sha1} except ValueError: return 'version must be in the form X.Y.Z (e.g., 15.2.3)' - if int(major) < 15 or (int(major) == 15 and int(minor) < 2): + if major < 15 or (major == 15 and minor < 2): return 'cephadm only supports octopus (15.2.0) or later' # to far a jump? - current_version = self.mgr.version.split('ceph version ')[1] - (current_major, current_minor, _) = current_version.split('-')[0].split('.', 2) - if int(current_major) < int(major) - 2: + (current_major, current_minor, current_version) = self._get_current_version() + if current_major < major - 2: return f'ceph can only upgrade 1 or 2 major versions at a time; {current_version} -> {version} is too big a jump' - if int(current_major) > int(major): + if current_major > major: return f'ceph cannot downgrade major versions (from {current_version} to {version})' - if int(current_major) == int(major): - if int(current_minor) > int(minor): - return f'ceph cannot downgrade to a {"rc" if minor == "1" else "dev"} release' + if current_major == major: + if current_minor > minor: + return f'ceph cannot downgrade to a {"rc" if minor == 1 else "dev"} release' # check mon min monmap = self.mgr.get("mon_map") mon_min = monmap.get("min_mon_release", 0) - if mon_min < int(major) - 2: + if mon_min < major - 2: return f'min_mon_release ({mon_min}) < target {major} - 2; first complete an upgrade to an earlier release' # check osd min osdmap = self.mgr.get("osd_map") osd_min_name = osdmap.get("require_osd_release", "argonaut") osd_min = ceph_release_to_major(osd_min_name) - if osd_min < int(major) - 2: + if osd_min < major - 2: return f'require_osd_release ({osd_min_name} or {osd_min}) < target {major} - 2; first complete an upgrade to an earlier release' return None - def upgrade_ls(self, image: Optional[str], tags: bool) -> Dict: + def upgrade_ls(self, image: Optional[str], tags: bool, show_all_versions: Optional[bool]) -> Dict: if not image: image = self.mgr.container_image_base reg_name, bare_image = image.split('/', 1) reg = Registry(reg_name) + (current_major, current_minor, _) = self._get_current_version() versions = [] r: Dict[Any, Any] = { "image": image, @@ -218,7 +224,12 @@ class CephadmUpgrade: continue if '-' in v[2]: continue - versions.append('.'.join(v)) + v_major = int(v[0]) + v_minor = int(v[1]) + candidate_version = (v_major > current_major + or (v_major == current_major and v_minor >= current_minor)) + if show_all_versions or candidate_version: + versions.append('.'.join(v)) r["versions"] = sorted( versions, key=lambda k: list(map(int, k.split('.'))), diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py index 1d7341c070165..912040f9e16cf 100644 --- a/src/pybind/mgr/orchestrator/_interface.py +++ b/src/pybind/mgr/orchestrator/_interface.py @@ -661,7 +661,7 @@ class Orchestrator(object): def upgrade_check(self, image: Optional[str], version: Optional[str]) -> OrchResult[str]: raise NotImplementedError() - def upgrade_ls(self, image: Optional[str], tags: bool) -> OrchResult[Dict[Any, Any]]: + def upgrade_ls(self, image: Optional[str], tags: bool, show_all_versions: Optional[bool] = False) -> OrchResult[Dict[Any, Any]]: raise NotImplementedError() def upgrade_start(self, image: Optional[str], version: Optional[str]) -> OrchResult[str]: diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py index 232d883625efd..c7631df749297 100644 --- a/src/pybind/mgr/orchestrator/module.py +++ b/src/pybind/mgr/orchestrator/module.py @@ -1398,9 +1398,11 @@ Usage: @_cli_read_command('orch upgrade ls') def _upgrade_ls(self, image: Optional[str] = None, - tags: bool = False) -> HandleCommandResult: + tags: bool = False, + show_all_versions: Optional[bool] = False + ) -> HandleCommandResult: """Check for available versions (or tags) we can upgrade to""" - completion = self.upgrade_ls(image, tags) + completion = self.upgrade_ls(image, tags, show_all_versions) r = raise_if_exception(completion) out = json.dumps(r, indent=4) return HandleCommandResult(stdout=out) -- 2.39.5