From b3920dd305656dba02cae6f71ee2088fa609bfb2 Mon Sep 17 00:00:00 2001 From: Ponnuvel Palaniyappan Date: Tue, 5 Mar 2024 21:03:40 +0000 Subject: [PATCH] [pacemaker] Use pep440 formatted version on comparison There are couple of instances (both on pacemaker) of `parse_version` being used to compare the package versions. In cases, notably on Ubuntu, where the version comform to PEP440, this fails. So we now convert those to PEP440 format before comparing. Fixes #3548. Signed-off-by: Ponnuvel Palaniyappan --- sos/collector/clusters/juju.py | 7 ++++--- sos/collector/clusters/pacemaker.py | 4 ++-- sos/collector/sosnode.py | 28 +++------------------------- sos/report/plugins/pacemaker.py | 4 ++-- sos/utilities.py | 26 ++++++++++++++++++++++++++ 5 files changed, 37 insertions(+), 32 deletions(-) diff --git a/sos/collector/clusters/juju.py b/sos/collector/clusters/juju.py index be69759a27..a8ef68fb3d 100644 --- a/sos/collector/clusters/juju.py +++ b/sos/collector/clusters/juju.py @@ -13,7 +13,7 @@ import re from sos.collector.clusters import Cluster -from sos.utilities import parse_version +from sos.utilities import sos_parse_version from sos.utilities import sos_get_command_output @@ -161,12 +161,13 @@ def _get_model_info(self, model_name): def _get_juju_version(self): """Grab the version of juju""" res = sos_get_command_output("juju version") - return res['output'].split("-")[0] + return res['output'] def _execute_juju_status(self, model_name): model_option = f"-m {model_name}" if model_name else "" format_option = "--format json" - if parse_version(self._get_juju_version()) > parse_version("3"): + juju_version = self._get_juju_version() + if sos_parse_version(juju_version) > sos_parse_version("3"): format_option += " --no-color" status_cmd = f"{self.cmd} status {model_option} {format_option}" res = self.exec_primary_cmd(status_cmd) diff --git a/sos/collector/clusters/pacemaker.py b/sos/collector/clusters/pacemaker.py index c83f8c3c0b..bd3a832b8a 100644 --- a/sos/collector/clusters/pacemaker.py +++ b/sos/collector/clusters/pacemaker.py @@ -11,7 +11,7 @@ import re from sos.collector.clusters import Cluster -from sos.utilities import parse_version +from sos.utilities import sos_parse_version from xml.etree import ElementTree @@ -63,7 +63,7 @@ def get_nodes_from_crm(self): _ver = self.exec_primary_cmd('crm_mon --version') if _ver['status'] == 0: cver = _ver['output'].split()[1].split('-')[0] - if not parse_version(cver) > parse_version('2.0.3'): + if not sos_parse_version(cver) > sos_parse_version('2.0.3'): xmlopt = '--as-xml' else: return diff --git a/sos/collector/sosnode.py b/sos/collector/sosnode.py index 07a7186517..f315c58a67 100644 --- a/sos/collector/sosnode.py +++ b/sos/collector/sosnode.py @@ -26,7 +26,7 @@ ConnectionException, UnsupportedHostException, InvalidTransportException) -from sos.utilities import parse_version +from sos.utilities import sos_parse_version TRANSPORTS = { 'local': LocalTransport, @@ -412,31 +412,9 @@ def check_sos_version(self, ver): :returns: True if installed version is at least ``ver``, else False :rtype: ``bool`` """ - def _format_version_to_pep440(ver): - """ Convert the version into a PEP440 compliant version scheme.""" - public_version_re = re.compile( - r"^([0-9][0-9.]*(?:(?:a|b|rc|.post|.dev)[0-9]+)*)\+?" - ) - try: - _, public, local = public_version_re.split(ver, maxsplit=1) - if not local: - return ver - sanitized_local = re.sub("[+~]+", ".", local).strip("-") - pep440_version = f"{public}+{sanitized_local}" - return pep440_version - except Exception as err: - self.log_debug(f"Unable to format {ver} to pep440 format: " - f"{err}") - return ver - - _ver = _format_version_to_pep440(ver) - _node_formatted_version = _format_version_to_pep440( - self.sos_info['version']) - try: - _node_ver = parse_version(_node_formatted_version) - _test_ver = parse_version(_ver) - return _node_ver >= _test_ver + _node_ver = self.sos_info['version'] + return sos_parse_version(_node_ver) >= sos_parse_version(ver) except Exception as err: self.log_error("Error checking sos version: %s" % err) return False diff --git a/sos/report/plugins/pacemaker.py b/sos/report/plugins/pacemaker.py index cf11e2197b..113691e108 100644 --- a/sos/report/plugins/pacemaker.py +++ b/sos/report/plugins/pacemaker.py @@ -8,7 +8,7 @@ from sos.report.plugins import (Plugin, RedHatPlugin, DebianPlugin, UbuntuPlugin, PluginOpt) -from sos.utilities import parse_version +from sos.utilities import sos_parse_version from datetime import datetime, timedelta import re @@ -55,7 +55,7 @@ def setup_pcs(self): ]) pcs_version = '.'.join(pcs_pkg['version']) - if parse_version(pcs_version) > parse_version('0.10.8'): + if sos_parse_version(pcs_version) > sos_parse_version('0.10.8'): self.add_cmd_output("pcs property config --all") else: self.add_cmd_output("pcs property list --all") diff --git a/sos/utilities.py b/sos/utilities.py index ce371b0a03..c8f1199335 100644 --- a/sos/utilities.py +++ b/sos/utilities.py @@ -73,6 +73,32 @@ ] +def format_version_to_pep440(ver): + """ Convert the version into a PEP440 compliant version scheme.""" + public_version_re = re.compile( + r"^([0-9][0-9.]*(?:(?:a|b|rc|.post|.dev)[0-9]+)*)\+?" + ) + try: + _, public, local = public_version_re.split(ver, maxsplit=1) + if not local: + return ver + sanitized_local = re.sub("[+~]+", ".", local).strip("-") + pep440_version = f"{public}+{sanitized_local}" + return pep440_version + except Exception as err: + log.debug(f"Unable to format {ver} to pep440 format: {err}") + return ver + + +def sos_parse_version(ver, pep440=True): + """ Converts the version to PEP440 format before parsing """ + if pep440: + ver_pep440 = format_version_to_pep440(ver) + return parse_version(ver_pep440) + + return parse_version(ver) + + def tail(filename, number_of_bytes): """Returns the last number_of_bytes of filename""" with open(filename, "rb") as f: