[RHELC-664] Collect breadcrumbs through RHSM custom facts #612
Conversation
Thank you for contributing to the Convert2RHEL project!👋 Hello @r0x0d, thank you for submitting a PR 🚀!
To execute integration tests, members of the oamg organization don't need any extra step. The automation kicks that off for you! If you're an external collaborator (meaning that you're not part of the oamg organization), wait for somebody from the Note: In case there are problems with tests not being triggered automatically for a new PR/commit or pending for a long time, ping |
r0x0d
requested review from
abadger and
bocekm
and removed request for
abadger
|
This pull request introduces 2 alerts when merging 01c5262 into 7512287 - view on LGTM.com new alerts:
|
r0x0d
force-pushed
the
rhsm-breadcrumbs-collection
branch
from
01c5262 to
7aefee1
Compare
Codecov ReportBase: 91.59% // Head: 92.95% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #612 +/- ##
==========================================
+ Coverage 91.59% 92.95% +1.36%
==========================================
Files 18 18
Lines 2723 2769 +46
Branches 492 498 +6
==========================================
+ Hits 2494 2574 +80
+ Misses 171 131 -40
- Partials 58 64 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
This pull request introduces 2 alerts when merging 7aefee1 into 7512287 - view on LGTM.com new alerts:
|
|
/packit test |
r0x0d
force-pushed
the
rhsm-breadcrumbs-collection
branch
from
7aefee1 to
4ee923c
Compare
|
This pull request introduces 2 alerts when merging 4ee923c into 26cc4fe - view on LGTM.com new alerts:
|
|
/packit test |
|
This pull request introduces 2 alerts when merging 29aff19 into a625460 - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging 71b2a79 into ba02486 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 823dcac into ba02486 - view on LGTM.com new alerts:
|
r0x0d
force-pushed
the
rhsm-breadcrumbs-collection
branch
from
823dcac to
3dffb38
Compare
|
This pull request introduces 3 alerts when merging 3dffb38 into ba02486 - view on LGTM.com new alerts:
|
r0x0d
force-pushed
the
rhsm-breadcrumbs-collection
branch
from
8ff9e8c to
24ebcc0
Compare
|
@abadger since I'm using a couple of internal functions of SystemInfo class, should we make it public? |
add anyOF directive and cover null values source_os is set to "null" when we fail before early_collectio() has been run. We should talk about whether to run "early_collection" in rollback if it hasn't been done already but that's a separate issue to be determined some other time. Signed-off-by: Daniel Diblik <[email protected]>
When the log files on the converted system fail to validate, print the log files in full so that we know what the data was.
abadger
force-pushed
the
rhsm-breadcrumbs-collection
branch
from
4717c09 to
0833fa8
Compare
|
Figured out Two: source_os is being set to null but the fix was allowing source_os:id to be set to null. I've updated the schema so that should pass too. |
|
Okay, most integration tests run via pakit have now completed successfully. The yum issue is still there but on this retry, it was on centos-8.4 and oraclelinux-8.6. (oraclelinux-7 has not completed although it was started 5 hours ago... I think the yum_distro_sync test there has gotten stuck in RUNNING state. Every other test has passed.). Since the problem is occurring on both centos-8.4 and oraclelinux-8.6, I don't think it's a problem with the distribution but I don't think it's being caused by this code either.... Maybe it's a problem that snuck into the codebase when we were having so many false positives in integration testing for a while. I think this is dev complete, but I'll wait for @danmyway decide if we should merge or if he thinks the yum problems are caused by this. |
|
Hmm.. Update: I looked closer and there are two different errors: Centos 84:network error when subscription-manager tries to attach a subscription to the machine:
Leads to test_yum_distrosync failing. Seems unrelated to breadcrumbs. Oraclelinux 8.6This is the yum error I mentioned above. Since it is only happening on oraclelinux 8.6, it could be a problem at the distro packaging level... Have to test to find out. Causes test_run_conversion_using_custom_repos to fail Also seems unrelated to breadcrumbs changes. |
This code was merged without anyone realizing it was present in this unit test. Signed-off-by: Rodolfo Olivieri <[email protected]>
|
@r0x0d, one suggestion for a subsequent improvement - the output related to breadcrumbs:
You can either remove the migration logs as they have little value to the user, or leave there just one (the "updated" one) but pretty-print it.
|
|
Also, the name and version is printed three times when gathering system info: |
| utils.write_json_object_to_file(path=RHSM_CUSTOM_FACTS_FILE, data=data) | ||
| except (IOError, OSError): | ||
| rhsm_facts_path = os.path.dirname(RHSM_CUSTOM_FACTS_FILE) | ||
| loggerinst.warning("Unable to find RHSM facts folder at '%s'.", rhsm_facts_path) |
There was a problem hiding this comment.
I think that if the folder /etc/rhsm/facts/ does not exist, we should create it before trying to write the facts file to it.
The way it is now, we will have the breadcrumbs file available only after we install the subscription-manager package. But we'd like to know even about the failures that happen before we install the subscription-manager package. As a matter of fact, the subscription-manager pkg is not even being installed when the user runs the tool with the --norhsm option.
The facts are supposed to be collected by sos. We should check whether the sos plugin collects them even without the subscription-manager installed.
Example of where the breadcrumbs should have been written
[10/27/2022 22:39:25] TASK - [Prepare: Checking if the loaded kernel version is the most recent]
[10/27/2022 22:39:25] DEBUG - Calling command 'repoquery --quiet --qf "%{BUILDTIME}\t%{VERSION}-%{RELEASE}\t%{REPOID}" kernel'
[10/27/2022 22:39:25] DEBUG - Traceback (most recent call last):
File "/usr/lib/python2.6/site-packages/convert2rhel/main.py", line 94, in main
checks.perform_pre_checks()
File "/usr/lib/python2.6/site-packages/convert2rhel/checks.py", line 68, in perform_pre_checks
is_loaded_kernel_latest()
File "/usr/lib/python2.6/site-packages/convert2rhel/checks.py", line 672, in is_loaded_kernel_latest
_, latest_kernel, repoid = packages[0]
ValueError: need more than 1 value to unpack
Writing breadcrumbs to '/etc/migration-results'.
[10/27/2022 22:39:25] DEBUG - Prior migration log: {"activities": [{"executed": "/usr/bin/convert2rhel -y --debug --no-rpm-va --serverurl subscription.rhsm.stage.redhat.com --username c2r_main --password ***** --pool 2c94a05683f64f030184162087bf18cb", "success": false, "run_id": "null", "activity_ended": "2022-10-27T20:36:11.097896Z", "activity_started": "2022-10-27T20:36:01.609912Z", "version": "1", "env": {}, "activity": "conversion", "source_os": {"version": "6.10", "id": "Final", "name": "CentOS"}, "packages": [{"nevra": "convert2rhel-1.0-1.20221027201547923673.pr587.61.gda871a6.el6.noarch", "signature": "RSA/8, Thu Oct 27 22:17:20 2022, Key ID 175e6797ecf2dde9"}], "target_os": "null"}]}
[10/27/2022 22:39:25] DEBUG - Updated migration log: {"activities": [{"executed": "/usr/bin/convert2rhel -y --debug --no-rpm-va --serverurl subscription.rhsm.stage.redhat.com --username c2r_main --password ***** --pool 2c94a05683f64f030184162087bf18cb", "success": false, "run_id": "null", "activity_ended": "2022-10-27T20:36:11.097896Z", "activity_started": "2022-10-27T20:36:01.609912Z", "version": "1", "env": {}, "activity": "conversion", "source_os": {"version": "6.10", "id": "Final", "name": "CentOS"}, "packages": [{"nevra": "convert2rhel-1.0-1.20221027201547923673.pr587.61.gda871a6.el6.noarch", "signature": "RSA/8, Thu Oct 27 22:17:20 2022, Key ID 175e6797ecf2dde9"}], "target_os": "null"}, {"executed": "/usr/bin/convert2rhel -y --debug --no-rpm-va --serverurl subscription.rhsm.stage.redhat.com --username c2r_main --password ***** --pool 2c94a05683f64f030184162087bf18cb", "run_id": "null", "packages": [{"nevra": "convert2rhel-1.0-1.20221027201547923673.pr587.61.gda871a6.el6.noarch", "signature": "RSA/8, Thu Oct 27 22:17:20 2022, Key ID 175e6797ecf2dde9"}], "target_os": "null", "success": false, "activity_ended": "2022-10-27T20:39:25.381003Z", "version": "1", "env": {}, "activity": "conversion", "source_os": {"version": "6.10", "id": "Final", "name": "CentOS"}, "activity_started": "2022-10-27T20:39:19.906445Z"}]}
Writing RHSM custom facts to '/etc/rhsm/facts/convert2rhel.facts'.
WARNING - Unable to find RHSM facts folder at '/etc/rhsm/facts'.
No changes were made to the system.
After we introduced the collection of breadcrumbs (oamg#612) as a RHSM facts, we started to use some internal (now public) functions from the systeminfo.py module, which at first, those functions were responsible for logging out some system information, such as: Name, Version, Config file used, and so on... That change caused some duplicated lines in the Convert2RHEL log output. This commit fixes this by moving the logs to the `resolve_system_info` function instead of placing the logs in each of the facts collection. Signed-off-by: Rodolfo Olivieri <[email protected]>
After we introduced the collection of breadcrumbs (oamg#612) as a RHSM facts, we started to use some internal (now public) functions from the systeminfo.py module, which at first, those functions were responsible for logging out some system information, such as: Name, Version, Config file used, and so on... That change caused some duplicated lines in the Convert2RHEL log output. This commit fixes this by moving the logs to the `resolve_system_info` function instead of placing the logs in each of the facts collection. Signed-off-by: Rodolfo Olivieri <[email protected]>
* Move system info logs to happen after the data collection After we introduced the collection of breadcrumbs (#612) as a RHSM facts, we started to use some internal (now public) functions from the systeminfo.py module, which at first, those functions were responsible for logging out some system information, such as: Name, Version, Config file used, and so on... That change caused some duplicated lines in the Convert2RHEL log output. This commit fixes this by moving the logs to the `resolve_system_info` function instead of placing the logs in each of the facts collection. Signed-off-by: Rodolfo Olivieri <[email protected]> * Separate system info collection from logs Signed-off-by: Rodolfo Olivieri <[email protected]> Signed-off-by: Rodolfo Olivieri <[email protected]>
After merging the oamg#612, we left behind a few comments to be addressed in a future PR of some things that we could improve. Signed-off-by: Rodolfo Olivieri <[email protected]>
* Move system info logs to happen after the data collection After we introduced the collection of breadcrumbs (oamg#612) as a RHSM facts, we started to use some internal (now public) functions from the systeminfo.py module, which at first, those functions were responsible for logging out some system information, such as: Name, Version, Config file used, and so on... That change caused some duplicated lines in the Convert2RHEL log output. This commit fixes this by moving the logs to the `resolve_system_info` function instead of placing the logs in each of the facts collection. Signed-off-by: Rodolfo Olivieri <[email protected]> * Separate system info collection from logs Signed-off-by: Rodolfo Olivieri <[email protected]> Signed-off-by: Rodolfo Olivieri <[email protected]>
After merging the oamg#612, we left behind a few comments to be addressed in a future PR of some things that we could improve. Signed-off-by: Rodolfo Olivieri <[email protected]>
After merging the oamg#612, we left behind a few comments to be addressed in a future PR of some things that we could improve. Signed-off-by: Rodolfo Olivieri <[email protected]>
* Add functions to the utils module We have moved the `hide_secrets()` function form subscription module to the utils module and added a new function to flatten dictionaries to a single level. Signed-off-by: Rodolfo Olivieri <[email protected]> * Dump the breadcrumbs into the RHSM facts format Added a new function to the breadcrumbs class to dump the current view of the breadcrmubs data into the RHSM custom facts format (single level json). First, we refactored the way we sanitaze the CLI parameters in the breadcrumbs by just using the `hide_secrets()` function (that is now in the `utils.py` module) to be our main source of sanitization instead of the old function that was way more complex and did the same thing as the `hide_secrets()`. We also introduced a few changes to the breadcrumbs class to make it simpler and have only one method to call to determine if the data collection is finished or not. Signed-off-by: Rodolfo Olivieri <[email protected]> * Add unit_tests for all the changes Signed-off-by: Rodolfo Olivieri <[email protected]> * Assert that the custom facts are generated accordingly Signed-off-by: Rodolfo Olivieri <[email protected]> * Add unit_tests for main rollback process Signed-off-by: Rodolfo Olivieri <[email protected]> * Rework the way we gather the system information for source_os and target_os Signed-off-by: Rodolfo Olivieri <[email protected]> * Add unit_tests for system_info distribution id Signed-off-by: Rodolfo Olivieri <[email protected]> * Make internal function public from systeminfo class Signed-off-by: Rodolfo Olivieri <[email protected]> * Fix integration tests failures There was a problem with how we were checking that the source_os was correct set in the integration tests, so that needed to be changed to adapt to the new schema. Signed-off-by: Rodolfo Olivieri <[email protected]> Co-authored-by: Toshio Kuratomi <[email protected]> * Refactor correct_distro test. Updated the logic so that we declare the destination_distro string and source_distro data in a distionary and then use the dictionary to check whether we converted to the correct distro. * Skip rhsm custom facts check when convert2rhel is invoked with --no-rhsm or --disable-submgr the rhsm custom facts are not created, therefore we need to skip the check for those kind of tests Signed-off-by: Daniel Diblik <[email protected]> * Fix failing schema validation add anyOF directive and cover null values source_os is set to "null" when we fail before early_collectio() has been run. We should talk about whether to run "early_collection" in rollback if it hasn't been done already but that's a separate issue to be determined some other time. Signed-off-by: Daniel Diblik <[email protected]> * Add some logging to narrow down when the source_os data is being set to null. * Enhance log from test_flag_system_as_converted.py test failures. When the log files on the converted system fail to validate, print the log files in full so that we know what the data was. Signed-off-by: Rodolfo Olivieri <[email protected]> Signed-off-by: Daniel Diblik <[email protected]> Co-authored-by: Toshio Kuratomi <[email protected]> Co-authored-by: Toshio Kuratomi <[email protected]> Co-authored-by: Daniel Diblik <[email protected]>
* Move system info logs to happen after the data collection After we introduced the collection of breadcrumbs (#612) as a RHSM facts, we started to use some internal (now public) functions from the systeminfo.py module, which at first, those functions were responsible for logging out some system information, such as: Name, Version, Config file used, and so on... That change caused some duplicated lines in the Convert2RHEL log output. This commit fixes this by moving the logs to the `resolve_system_info` function instead of placing the logs in each of the facts collection. Signed-off-by: Rodolfo Olivieri <[email protected]> * Separate system info collection from logs Signed-off-by: Rodolfo Olivieri <[email protected]> Signed-off-by: Rodolfo Olivieri <[email protected]>
This PR introduces the collection of breadcrumbs through subscription-manager, by actually converting the current state of our breadcrumbs into RHSM custom facts and uploading it to the candlepin server tied to the registration.
Currently, we are introducing this as an MVP to the general idea. In the future, we will stop outputting the file at
/etc/migration-resultsand stick only with the one that is located at/etc/rhsm/facts/*.facts, but for now, we have both of them placed on the system in case of something going wrong and not having the facts synced with subscription-manager.TODO
Jira Issue: RHELC-664
Checklist
[RHELC-]is part of the PR titleRelease Pending