Fix yum output parsing to avoid false reboot detection #336
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1062,7 +1062,7 @@ def do_processes_require_restart(self): | |
| continue | ||
|
|
||
| process_details = re.split(r'\s+', line.strip()) | ||
| if len(process_details) < 7 or line.find("packages excluded") >= 0: | ||
|
There was a problem hiding this comment. Add/adjust unit coverage to lock in this behavior: extend the YumPackageManager reboot/process parsing tests (and the LegacyEnvLayerExtensions "sudo yum ps" fixture) with a sample line like " packages excluded …" appearing in the yum-ps output, and assert that it does not increment process_count / trigger reboot pending.
Contributor
There was a problem hiding this comment. Please share an example of why this change is needed. In the sense, what was the current behavior seen on a VM, with the help of logs and how does this change correct it, again with the help of in VM logs
Author
There was a problem hiding this comment. Before:
Contributor
There was a problem hiding this comment. I'm asking for logs from a VM for the before and after scenarios. Create a VM, reproduce the before scenario which show the false reboot detection and then another set of logs that show this issue fixed with the help of your code change
Author
There was a problem hiding this comment. Before (VM logs): After (VM logs):
Contributor
There was a problem hiding this comment. Your response is just a statement on what the before and after should look like, this is not however an actual log fetched from a VM. Repeating my ask from before: @sadiksubhani9-sudo, you need to create a VM, run this code in that VM, fetch logs from the VM that demonstrates the before scenario. Then run your changed code in the VM, fetch logs that demonstrate the after scenario
Author
There was a problem hiding this comment. @rane-rajasi sending a reminder for the PR review...
Author
There was a problem hiding this comment. @kjohn-msft, Please take a look when you get a chance. Thanks!
Collaborator
There was a problem hiding this comment. @rane-rajasi requested the contents of a specific folder (ideally zipped) from the test machine - prior to the change and after the change. I don't see that in the internal folder. The goal is to understand the dev-test validation flow associated with any change (this change implicitly being simple). Which means:
Contributor
There was a problem hiding this comment. @sadiksubhani9-sudo please share the contents of the extension logs folder, path shared here: 
Koshy is referring the same in this feedback
Author
There was a problem hiding this comment. |
||
| self.composite_logger.log_verbose("[YPM] > Inapplicable line: " + str(line)) | ||
| continue | ||
| else: | ||
|
|
||
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.
The parser still accepts any line with >=7 whitespace-separated tokens whose first token is an int as a “process” line. The new special-case filter for "packages excluded" fixes one known message, but the overall parsing remains overly permissive and can misclassify other numeric-prefixed informational lines. Consider switching to a stricter format validation for yum-ps rows (e.g., regex/column validation for CPU/RSS/State/uptime) so future yum output variations don’t reintroduce false reboot detection.