Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update Uname parser to fix LooseVersion comparision error #3814

Merged
merged 4 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 48 additions & 3 deletions insights/parsers/uname.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,33 @@ def _best_version(self, other):
else:
return self._lv_version, other._lv_version

def _best_lv_release(self, other):
"""
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 = (
self._lv_release.vstring if not is_s_with_dist
else pad_release(".".join(s_release_parts[:-1]), len(s_release_parts))
)
o_release = (
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)

def __str__(self):
return "version: %s; release: %s; rel_maj: %s; lv_release: %s" % (
self.version, self.release, self._rel_maj, self._lv_release
Expand Down Expand Up @@ -519,9 +546,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._best_lv_release(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
Expand All @@ -530,6 +558,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
Expand All @@ -546,6 +575,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]

Expand Down Expand Up @@ -594,7 +624,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)

Expand All @@ -603,6 +639,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 find_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:
Expand All @@ -612,8 +654,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 = find_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)
32 changes: 29 additions & 3 deletions insights/tests/parsers/test_uname.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pytest
import warnings
from insights.parsers import uname
from insights.tests import context_wrap

Expand Down Expand Up @@ -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')

Expand All @@ -236,19 +239,42 @@ 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
xiangce marked this conversation as resolved.
Show resolved Hide resolved
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
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"),
Expand Down
Loading