-
Notifications
You must be signed in to change notification settings - Fork 116
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
[1.28] New dmidecode-based DMI facts collector; use it on aarch64 #3216
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Coverage (computed on Fedora latest) •
|
The python-dmidecode module has currently not received work upstream in the last 7 years, and it starts to fails in some situations (e.g. Kernel Lockdown, when /dev/mem cannot be read). As way around that, switch the MiniHostCollector facts collector to invoke the dmidecode(1) tool instead to get some facts from known DMI strings. The general result of the detection should still be the same, as the fetched strings are used (or potentially used) during the cloud detection by the providers. Also, this fact detector is used only when subscription-manager is not installed, so the impact should be minimal. Adapt the buildsystem & packaging bits to require dmidecode for cloud-what rather than the Python module dmidecode. (cherry picked from commit 9422d24)
Create a new parser for parsing the output of the dmidecode(1) tool, collecting all the data from it. Add a test for it with existing outputs to check, as dmidecode information can be read only as root, and they would change on each system. The data-based tests check that the parser works fine, and extracts at least the system UUID. The test data have sensible details (UUIDs, serial numbers, product names, and part numbers) replaced with placeholders; this also helps testing that the parsed output contains the values we expect. In addition to that, add a facts collector that uses this new parser; to make sure the facts resemble somehow what the python-dmidecode-based one returns, do few quirks to adapt the data read from dmidecode(1). Add a test for this new collector, using the aforementioned test data, and checking that at least some subtags of "dmi" facts are collected. (cherry picked from commit b100b50)
Use the new DmidecodeFactCollector to collect the DMI facts, rather than DmiFirmwareInfoCollector (that uses python-dmidecode). Switches the requirement of subscription-manager from python-dmidecode (not available on SUSE distros) to dmidecode unconditionally (available on any modern distro). (cherry picked from commit 9253695)
Drop the facts collector based on python-dmidecode, as it is no more used now. (cherry picked from commit e969e2f)
subscription-manager itself does not use python-dmidecode anymore now. Also, python-dmidecode is not available in SUSE distros, exactly where this plugin is supposed to run. Hence, drop the dmidecode warnings cleaning, as it is effectively dead code. (cherry picked from commit 617048e)
The function was not using the parameter it was being passed, but some other local variable at its scope. Changing the name to ensure the right string is compared. (cherry picked from commit 2ef4d87)
* Card ID: ENT-5112 * The dmidecode is available on ARM64. Thus it make sense to use same approach, which is used on x86_64 platform and parse output of dmidecode * Old code and old tests were dropped * subscription-manager.spec file was modified (cherry picked from commit ddf4d6f)
ptoscano
force-pushed
the
ptoscano/2173583-1.28
branch
from
March 1, 2023 10:37
9d9db69
to
1169ff9
Compare
jirihnidek
approved these changes
Mar 1, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM: I can confirm that it works on x86_64
and aarch64
.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The python-dmidecode module has currently not received work upstream in the last 7 years, and it starts to fails in some situations (e.g. Kernel Lockdown, when /dev/mem cannot be read). Hence, we cannot keep using python-dmidecode anymore.
As way forward, create a parser to parse the output of
dmidecode(1)
, and a fact collector that uses this parser to provide"dmi.*"
facts.The parser and the facts collector are tested using collected outputs (and anonymized) of
dmidecode(1)
, so it is easy to check whether changes will break existing outputs.Cloud-what has its own facts collector using python-dmidecode in case subscription-manager is not installed. In it, manually query
dmidecode(1)
for known DMI strings used by the cloud detection, so it should work in that case too.The dmidecode is available on ARM64. Thus it make sense to use same approach, which is used on x86_64 platform and
parse output of
dmidecode(1)
.Please refer to the individual commits for few more explanations.
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2173583
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2109365 (fixed by dropping the old code)
Backport of: