From de0712a8ce8862e312fdb3bc7ad5246dbc3ae081 Mon Sep 17 00:00:00 2001 From: Qin Ping Date: Wed, 28 Jun 2023 15:09:48 +0800 Subject: [PATCH 1/4] fix: enhance Uname.fixed_by() Exception: TypeError: '<' not supported between instances of 'str' and 'int' is returned when the fixed_by or introduced_in doesn't have distribution part in release and version and other release parts are the same as current kernel. The detailed update is not doing comparation the distribution part in release. Signed-off-by: Ping Qin --- insights/parsers/uname.py | 42 ++++++++++++++++++++++++++-- insights/tests/parsers/test_uname.py | 19 +++++++++++-- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/insights/parsers/uname.py b/insights/parsers/uname.py index f96b744de0..72961703e0 100644 --- a/insights/parsers/uname.py +++ b/insights/parsers/uname.py @@ -397,6 +397,24 @@ def _best_version(self, other): else: return self._lv_version, other._lv_version + def _lv_release_without_dist(self, other): + """ + Get the LooseVersion releases without distribution part. + """ + s_release_parts = self._lv_release.vstring.split(".") + o_release_parts = other._lv_release.vstring.split(".") + s_release = ( + pad_release(".".join(s_release_parts[:-1]), len(s_release_parts)) + if s_release_parts[-1].startswith("el") + else ".".join(s_release_parts) + ) + o_release = ( + pad_release(".".join(o_release_parts[:-1]), len(o_release_parts)) + if o_release_parts[-1].startswith("el") + else ".".join(o_release_parts) + ) + return LooseVersion(s_release), LooseVersion(o_release) + def __str__(self): return "version: %s; release: %s; rel_maj: %s; lv_release: %s" % ( self.version, self.release, self._rel_maj, self._lv_release @@ -519,9 +537,10 @@ def _less_than(self, other): internal to the class. """ s_version, o_version = self._best_version(other) + s_lv_release, o_lv_release = self._lv_release_without_dist(other) if s_version == o_version: - ret = self._lv_release < other._lv_release + ret = s_lv_release < o_lv_release else: ret = s_version < o_version return ret @@ -530,6 +549,7 @@ def fixed_by(self, *fixes, **kwargs): """ Determine whether the Uname object is fixed by a range of releases or by a specific release. + Only RHEL Uname objects and RHEL real time Uname objects are supported. :Parameters: - `fixes`: List of one or more Uname objects to compare to the @@ -546,6 +566,7 @@ def fixed_by(self, *fixes, **kwargs): introduced_in = kwargs.get("introduced_in") if introduced_in and self._less_than(self.from_kernel(introduced_in)): return [] + fix_unames = sorted((self.from_kernel(f) for f in fixes)) fix_kernels = [f.kernel for f in fix_unames] @@ -594,7 +615,13 @@ def pad_release(release_to_pad, num_sections=4): pad_release("390.11.el6", 4) - will return ``390.11.0.el6``. Finally, if no "el" is specified: + will return ``390.11.0.el6``. + + pad_release("390.11.rt6.8.el6", 7) + + will return ``390.11.0.0.rt6.8.el6``. + + Finally, if no "el" is specified: pad_release("390.11", 4) @@ -603,6 +630,12 @@ def pad_release(release_to_pad, num_sections=4): If the number of sections of the release to be padded is greater than num_sections, a ``ValueError`` will be raised. ''' + def is_rt_release(items): + for i, s in enumerate(items): + if s.startswith("rt"): + return i + return -1 + parts = release_to_pad.split('.') if len(parts) > num_sections: @@ -612,8 +645,11 @@ def pad_release(release_to_pad, num_sections=4): pad_count = num_sections - len(parts) is_el_release = any(letter.isalpha() for letter in parts[-1]) + rt_release_idx = is_rt_release(parts) - if len(parts) > 1 and is_el_release: + if len(parts) > 1 and rt_release_idx >= 0: + return ".".join(parts[:rt_release_idx] + ['0'] * pad_count + parts[rt_release_idx:]) + elif len(parts) > 1 and is_el_release: return ".".join(parts[:-1] + ['0'] * pad_count + parts[-1:]) else: return ".".join(parts + ['0'] * pad_count) diff --git a/insights/tests/parsers/test_uname.py b/insights/tests/parsers/test_uname.py index ddf8d0025b..e9b7701045 100644 --- a/insights/tests/parsers/test_uname.py +++ b/insights/tests/parsers/test_uname.py @@ -236,19 +236,32 @@ def test_fixed_by(): assert ['2.6.33-100.el6'] == u.fixed_by('2.6.33-100.el6') assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-220.1.el6', '2.6.32-600.el6') + # fixes without distribution part + assert [] == u.fixed_by('2.6.32-504') + # fixes only has kernel version part + assert [] == u.fixed_by('2.6.32-') + # introduce without distribution part + assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-600.el6', introduced_in='2.6.32-504') + # introduce only has kernel version part + assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-600.el6', introduced_in='2.6.32-') + + rt_u = uname.Uname.from_uname_str("Linux qqhrycsq2 4.18.0-305.rt7.72.el8.x86_64 #1 SMP Wed Jun 13 18:24:36 EDT 2012 x86_64 x86_64 x86_64 GNU/Linux") + # fixes without distribution part + assert [] == rt_u.fixed_by('4.18.0-305.rt7.72') + # introduce without distribution part + assert ['4.18.0-307.rt7.72.el8'] == rt_u.fixed_by('4.18.0-307.rt7.72.el8', introduced_in='4.18.0-305.rt7.72') + def test_unknown_release(): u = uname.Uname.from_kernel("2.6.23-504.23.3.el6.revertBZ1169225") assert "504.23.3.0.el6" == u._lv_release - fixed_by = u.fixed_by("2.6.18-128.39.1.el5", "2.6.18-238.40.1.el5", "2.6.18-308.13.1.el5", "2.6.18-348.el5") - assert [] == fixed_by def test_fixed_by_rhel5(): test_kernels = [ (uname.Uname.from_uname_str("Linux oprddb1r5.example.com 2.6.18-348.el5 #1 SMP Wed Nov 28 21:22:00 EST 2012 x86_64 x86_64 x86_64 GNU/Linux"), []), (uname.Uname.from_uname_str("Linux srspidr1-3.example2.com 2.6.18-402.el5 #1 SMP Thu Jan 8 06:22:34 EST 2015 x86_64 x86_64 x86_64 GNU/Linux"), []), - (uname.Uname.from_uname_str("Linux PVT-Dev1.pvtsolar.local 2.6.18-398.el5xen #1 SMP Tue Aug 12 06:30:31 EDT 2014 x86_64 x86_64 x86_64 GNU/Linux"), []), + (uname.Uname.from_uname_str("Linux PVT-Dev1.pvtsolar.local 2.6.18-398.el5 #1 SMP Tue Aug 12 06:30:31 EDT 2014 x86_64 x86_64 x86_64 GNU/Linux"), []), (uname.Uname.from_kernel("2.6.18-194.el5"), ["2.6.18-238.40.1.el5", "2.6.18-308.13.1.el5", "2.6.18-348.el5"]), (uname.Uname.from_kernel("2.6.18-128.el5"), From 041e8c8ecf2addc1304ad1e05e464df75299ec74 Mon Sep 17 00:00:00 2001 From: Ping Qin Date: Tue, 8 Aug 2023 14:20:22 +0900 Subject: [PATCH 2/4] fix: Update Uname.fixed_by() When _lv_release in self and other both or neither have the distribution part, compare them directly. Otherwise, removing the distribution part first, then doing the comparation without the distribution part and raising a warning. Signed-off-by: Ping Qin --- insights/parsers/uname.py | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/insights/parsers/uname.py b/insights/parsers/uname.py index 72961703e0..fd06f5602c 100644 --- a/insights/parsers/uname.py +++ b/insights/parsers/uname.py @@ -397,21 +397,30 @@ def _best_version(self, other): else: return self._lv_version, other._lv_version - def _lv_release_without_dist(self, other): + def _best_lv_release(self, other): """ - Get the LooseVersion releases without distribution part. + When the _lv_release in self and other both or neither have + the distribution part, return then directly. Otherwise, + removing the distribution part, and raising a warning. + + Notes:: Now, only `.el*` distribution is supported. """ + dist_opts = ('el',) s_release_parts = self._lv_release.vstring.split(".") o_release_parts = other._lv_release.vstring.split(".") + is_s_with_dist = s_release_parts[-1].startswith(dist_opts) + is_o_with_dist = o_release_parts[-1].startswith(dist_opts) + if not (is_s_with_dist ^ is_o_with_dist): + return self._lv_release, other._lv_release + import warnings + warnings.warn('Comparison of distribution part will be ingored.') s_release = ( - pad_release(".".join(s_release_parts[:-1]), len(s_release_parts)) - if s_release_parts[-1].startswith("el") - else ".".join(s_release_parts) + self._lv_release.vstring if not is_s_with_dist + else pad_release(".".join(s_release_parts[:-1]), len(s_release_parts)) ) o_release = ( - pad_release(".".join(o_release_parts[:-1]), len(o_release_parts)) - if o_release_parts[-1].startswith("el") - else ".".join(o_release_parts) + other._lv_release.vstring if not is_o_with_dist + else pad_release(".".join(o_release_parts[:-1]), len(o_release_parts)) ) return LooseVersion(s_release), LooseVersion(o_release) @@ -537,7 +546,7 @@ def _less_than(self, other): internal to the class. """ s_version, o_version = self._best_version(other) - s_lv_release, o_lv_release = self._lv_release_without_dist(other) + s_lv_release, o_lv_release = self._best_lv_release(other) if s_version == o_version: ret = s_lv_release < o_lv_release @@ -630,7 +639,7 @@ def pad_release(release_to_pad, num_sections=4): If the number of sections of the release to be padded is greater than num_sections, a ``ValueError`` will be raised. ''' - def is_rt_release(items): + def find_rt_release(items): for i, s in enumerate(items): if s.startswith("rt"): return i @@ -645,7 +654,7 @@ def is_rt_release(items): pad_count = num_sections - len(parts) is_el_release = any(letter.isalpha() for letter in parts[-1]) - rt_release_idx = is_rt_release(parts) + rt_release_idx = find_rt_release(parts) if len(parts) > 1 and rt_release_idx >= 0: return ".".join(parts[:rt_release_idx] + ['0'] * pad_count + parts[rt_release_idx:]) From e768bc1a108167ac3437ca7a1584f53a666b2f78 Mon Sep 17 00:00:00 2001 From: Ping Qin Date: Wed, 9 Aug 2023 10:51:33 +0900 Subject: [PATCH 3/4] fix: Add test for pad_release() and warnings in fixed_by() Signed-off-by: Ping Qin --- insights/tests/parsers/test_uname.py | 29 ++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/insights/tests/parsers/test_uname.py b/insights/tests/parsers/test_uname.py index e9b7701045..a94514a25f 100644 --- a/insights/tests/parsers/test_uname.py +++ b/insights/tests/parsers/test_uname.py @@ -1,4 +1,5 @@ import pytest +import warnings from insights.parsers import uname from insights.tests import context_wrap @@ -223,6 +224,8 @@ def test_pad_release(): assert "390.0.0.0" == uname.pad_release("390") assert "390.12.0.0" == uname.pad_release("390.12") assert "1160.71.1.0" == uname.pad_release("1160.71.1") + assert "390.12.0.0.rt5.3.el6" == uname.pad_release("390.12.rt5.3.el6", 7) + assert "390.12.0.0.0.rt5.3" == uname.pad_release("390.12.rt5.3", 7) with pytest.raises(ValueError): uname.pad_release('390.11.12.13.el6') @@ -236,14 +239,24 @@ def test_fixed_by(): assert ['2.6.33-100.el6'] == u.fixed_by('2.6.33-100.el6') assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-220.1.el6', '2.6.32-600.el6') - # fixes without distribution part - assert [] == u.fixed_by('2.6.32-504') - # fixes only has kernel version part - assert [] == u.fixed_by('2.6.32-') - # introduce without distribution part - assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-600.el6', introduced_in='2.6.32-504') - # introduce only has kernel version part - assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-600.el6', introduced_in='2.6.32-') + # For all remaining tests, cause the warnings to always be caught + warnings.simplefilter("always") + warning_msg = "Comparison of distribution part will be ingored" + + with warnings.catch_warnings(record=True) as w: + # fixes without distribution part + assert [] == u.fixed_by('2.6.32-504') + + # fixes only has kernel version part + assert [] == u.fixed_by('2.6.32-') + + # introduce without distribution part + assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-600.el6', introduced_in='2.6.32-504') + + # introduce only has kernel version part + assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-600.el6', introduced_in='2.6.32-') + + assert any([warning_msg in str(ww.message) for ww in w]) rt_u = uname.Uname.from_uname_str("Linux qqhrycsq2 4.18.0-305.rt7.72.el8.x86_64 #1 SMP Wed Jun 13 18:24:36 EDT 2012 x86_64 x86_64 x86_64 GNU/Linux") # fixes without distribution part From 2216770969b3b99658d5ee9a6417bddc821574b2 Mon Sep 17 00:00:00 2001 From: Ping Qin Date: Wed, 9 Aug 2023 12:12:33 +0900 Subject: [PATCH 4/4] fix: seperate the test cases for warnings in a different method Signed-off-by: Ping Qin --- insights/tests/parsers/test_uname.py | 34 ++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/insights/tests/parsers/test_uname.py b/insights/tests/parsers/test_uname.py index a94514a25f..07f5c2870c 100644 --- a/insights/tests/parsers/test_uname.py +++ b/insights/tests/parsers/test_uname.py @@ -239,30 +239,46 @@ def test_fixed_by(): assert ['2.6.33-100.el6'] == u.fixed_by('2.6.33-100.el6') assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-220.1.el6', '2.6.32-600.el6') - # For all remaining tests, cause the warnings to always be caught - warnings.simplefilter("always") - warning_msg = "Comparison of distribution part will be ingored" +def test_fixed_by_warning(): + # For all remaining tests, cause the comparation warnings + # to always be caught, and ignore other warnings. + warning_msg = "Comparison of distribution part will be ingored" + warnings.simplefilter("ignore") + warnings.filterwarnings("always", message=warning_msg) with warnings.catch_warnings(record=True) as w: + + u = uname.Uname.from_uname_str("Linux qqhrycsq2 2.6.32-504.el6.x86_64 #1 SMP Wed Jun 13 18:24:36 EDT 2012 x86_64 x86_64 x86_64 GNU/Linux") + # should be no comparation warning reported + assert [] == u.fixed_by('2.6.32-220.1.el6', '2.6.32-504.el6') + assert len(w) == 0 + # fixes without distribution part assert [] == u.fixed_by('2.6.32-504') + assert len(w) == 1 # fixes only has kernel version part assert [] == u.fixed_by('2.6.32-') + assert len(w) == 2 # introduce without distribution part assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-600.el6', introduced_in='2.6.32-504') + assert len(w) == 3 # introduce only has kernel version part assert ['2.6.32-600.el6'] == u.fixed_by('2.6.32-600.el6', introduced_in='2.6.32-') + assert len(w) == 4 - assert any([warning_msg in str(ww.message) for ww in w]) + rt_u = uname.Uname.from_uname_str("Linux qqhrycsq2 4.18.0-305.rt7.72.el8.x86_64 #1 SMP Wed Jun 13 18:24:36 EDT 2012 x86_64 x86_64 x86_64 GNU/Linux") + # fixes without distribution part + assert [] == rt_u.fixed_by('4.18.0-305.rt7.72') + assert len(w) == 5 + + # introduce without distribution part + assert ['4.18.0-307.rt7.72.el8'] == rt_u.fixed_by('4.18.0-307.rt7.72.el8', introduced_in='4.18.0-305.rt7.72') + assert len(w) == 6 - rt_u = uname.Uname.from_uname_str("Linux qqhrycsq2 4.18.0-305.rt7.72.el8.x86_64 #1 SMP Wed Jun 13 18:24:36 EDT 2012 x86_64 x86_64 x86_64 GNU/Linux") - # fixes without distribution part - assert [] == rt_u.fixed_by('4.18.0-305.rt7.72') - # introduce without distribution part - assert ['4.18.0-307.rt7.72.el8'] == rt_u.fixed_by('4.18.0-307.rt7.72.el8', introduced_in='4.18.0-305.rt7.72') + warnings.resetwarnings() def test_unknown_release():