Skip to content

Conversation

@GAURAVRAMRAKHYANI
Copy link
Contributor

Issue:
As per the function do_processes_require_restart in YumPackageManager.py, the output of "sudo yum ps" is parsed to check if there are any processes and if there is any then True is returned in the function do_processes_require_restart
There is issue in parsing logic.

Example logs when true is returned when it should return false:

DEBUG: - Output from package manager:
| Loaded plugins: enabled_repos_upload, package_upload, product-id, ps, search-
| : disabled-repos, subscription-manager
|
| This system is not registered with an entitlement server. You can use subscription-manager to register.
|
| pid proc CPU RSS State uptime
| ps
| Uploading Enabled Repositories Report
| Cannot upload enabled repos report, is this client registered?
VERBOSE: ==========================================================================
DEBUG: - Inapplicable line: Loaded plugins: enabled_repos_upload, package_upload, product-id, ps, search-
DEBUG: - Inapplicable line: : disabled-repos, subscription-manager
DEBUG: - Inapplicable line:
DEBUG: - Inapplicable line: This system is not registered with an entitlement server. You can use subscription-manager to register.
DEBUG: - Inapplicable line:
DEBUG: - Process list started: pid proc CPU RSS State uptime
DEBUG: - Inapplicable line: ps
DEBUG: - Inapplicable line: Uploading Enabled Repositories Report
DEBUG: - Applicable line: Cannot upload enabled repos report, is this client registered?
2023-06-24T12:09:52Z> - Processes requiring restart (1): [upload (Cannot), ]
DEBUG: - Reboot required debug flags (yum): False, True.
DEBUG: Setting reboot pending status. [RebootPendingStatus=True]

Following line is marked applicable but actually it is not for any process, it is error message line:
Cannot upload enabled repos report, is this client registered?

So, in this case reboot required is returned by the function but actually reboot is not required because above line is not for any process, it is error message.

Root Cause:
In parsing, it is checked if there are at least 7 strings in the line. If there are at least 7 strings then it is assumed as a process detail. But this line could be error line as per above example.

Fix:
Adding one more check that first string should be integer because first string should be Process ID and hence it should be integer.

Testing
(a) Testing done for case when first string of the line is integer because that is process detail.
(b) Testing done for scenario when there is no process line. Hence no reboot required.

Added sample output in the test type "SadPath" so that this scenario is covered in test case test_do_processes_require_restart in the file Test_YumPackageManager.py.

…pdates job even though reboot is not required
@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #210 (ff78413) into master (4fa74fa) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
+ Coverage   90.14%   90.19%   +0.05%     
==========================================
  Files          90       90              
  Lines       14475    14483       +8     
==========================================
+ Hits        13048    13063      +15     
+ Misses       1427     1420       -7     
Flag Coverage Δ
python27 90.19% <100.00%> (+0.05%) ⬆️
python39 90.19% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/core/src/package_managers/YumPackageManager.py 84.61% <100.00%> (+1.28%) ⬆️
src/core/tests/library/LegacyEnvLayerExtensions.py 94.74% <100.00%> (+0.03%) ⬆️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants