[RHELC-752] Verify kernel boot files after conversion #721

r0x0d · 2023-02-01T20:10:55Z

Add a later check to warn the user about kernel boot files

We added a few later checks to verify and warn the user if there is any
errors with both initramfs vmlinuz files on their system after the
conversion.

This check has the intention to at least inform the user what are the
steps that they need to run in order to fix that problem.

Signed-off-by: Rodolfo Olivieri [email protected]

Jira Issue: RHELC-752

Checklist

PR meets acceptance criteria specified in the Jira issue
PR has been tested manually in a VM (either author or reviewer)
Jira issue has been made public if possible
[RHELC-] is part of the PR title
Code and tests are documented properly
The commits are squashed to as few commits as possible (without losing data)
When merged: Jira issue has been updated to Release Pending

codecov · 2023-02-01T20:13:06Z

Codecov Report

Base: 92.71% // Head: 92.82% // Increases project coverage by +0.11% 🎉

Coverage data is based on head (821fd59) compared to base (b90f5bb).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 821fd59 differs from pull request most recent head db25585. Consider uploading reports for the commit db25585 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #721      +/-   ##
==========================================
+ Coverage   92.71%   92.82%   +0.11%     
==========================================
  Files          24       24              
  Lines        3282     3319      +37     
  Branches      576      581       +5     
==========================================
+ Hits         3043     3081      +38     
+ Misses        172      171       -1     
  Partials       67       67

Flag	Coverage Δ
centos-linux-7	`88.15% <100.00%> (+0.16%)`	⬆️
centos-linux-8	`89.15% <100.00%> (+0.15%)`	⬆️

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

Impacted Files	Coverage Δ
convert2rhel/checks.py	`91.78% <100.00%> (+0.79%)`	⬆️
convert2rhel/grub.py	`94.52% <100.00%> (+0.47%)`	⬆️
convert2rhel/main.py	`91.25% <100.00%> (+0.11%)`	⬆️

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

convert2rhel/utils.py

r0x0d · 2023-02-01T20:19:37Z

Steps to reproduce locally (and for testing)

Open two terminals, one you will run the conversion inside the vagrant box, the other one you will run a script that will cause the error.
Go to the c2r-vagrant project and get a CentOS 8 box. You can go for CentOS 7 as well because the issue happen on both, but the steps below are meant for python 3 (CentOS 8).
1.1. An easy way of doing it is by pulling the latest changes from that project, and then running make refresh
On a second terminal, download those scripts to the convert2rhel source folder
2.1. Cause scriptlet failure by filling up disk space in /boot
2.2. Corrupt the initramfs file by echoing a text to it
Now, if you're on the latest version of the c2r-vagrant project, set the RSYNC=1 environment variable and then, you can run make convert and a conversion should start.
When you see the conversion running, go to the second terminal and execute one of those commands
4.1. vagrant ssh -- -t 'python3 /usr/lib/python3.6/site-packages/convert2rhel/<name-of-script-that-causes-scriptlet-failure>.py'
4.2. vagrant ssh -- -t 'python3 /usr/lib/python3.6/site-packages/convert2rhel/<name-of-script-that-causes-corruption>.py'
4.2.1. This script should be run when you see the task for generating the grub config (right after the prepare kernel one)
Wait for the conversion to reach the transaction part, and then, you will that the custom script you ran might stop running, that's intended, and it tells you that it finished and either filled up the disk space or corrupted the file, depending on which script you run.

Notes

This reproducer probably works with Oracle Linux as well, but you will need to change the kernel version inside of the custom scripts. Keep an eye on that.

convert2rhel/utils.py

convert2rhel/unit_tests/utils_test.py

tests/integration/tier1/kernel-boot-files/test_handle_kernel_boot_files.py

r0x0d · 2023-02-07T18:21:35Z

/packit test

convert2rhel/checks.py

tests/integration/tier1/kernel-boot-files/test_handle_corrupted_files.py

plans/tier1.fmf

abadger

Main code looks good. I'm just including a suggestion for how to implement outputing stdout to /dev/null and some spelling and wording changes.

I have looked at but not gone deeply into the unittests yet. I'm going to approve this because I don't see anything wrong with those but I know you specifically asked if I could see a better way to do them so I will look some more.

(I have not tested in a vm)

convert2rhel/checks.py

r0x0d · 2023-02-10T12:36:16Z

/packit test

r0x0d · 2023-02-10T18:13:05Z

/packit test

bocekm · 2023-02-17T13:14:11Z

Tested in a VM and it works fine. I've created a followup issue https://issues.redhat.com/browse/RHELC-902 (requesting to print required actions at the end of conversion).

tests/integration/tier1/kernel-boot-files/test_handle_corrupted_files.py

Co-authored-by: Toshio Kuratomi <[email protected]> Apply suggestions from code review Co-authored-by: Michal Bocek <[email protected]>

Signed-off-by: Rodolfo Olivieri <[email protected]>

bocekm · 2023-02-21T17:29:03Z

@danmyway, FYI, I've re-run:

testing-farm:centos-7-x86_64:tier1
testing-farm:oraclelinux-7-x86_64:tier1

All issues were infrastructure related.

danmyway · 2023-02-21T17:44:27Z

@danmyway, FYI, I've re-run:
* testing-farm:centos-7-x86_64:tier1

* testing-farm:oraclelinux-7-x86_64:tier1
All issues were infrastructure related.

I'd say waive these and merge if that happens again, if you agree @bocekm . The code is proved working by the tests implemented by @r0x0d.

danmyway

Looking good.
Failing tests unrelated to the changes.
Great job, thank you @r0x0d!

On CentOS and Oracle Linux 7 the test was failing because we were renaming the vmlinuz and initramfs files in order to back them up, but we left them in the /boot/ folder. That caused the grub2-mkconfig to pick them up and generated a new grub configuration using these renamed boot files. This change does not back up the boot files but instead simply removes them, simulating better the core test scenario of this integration test which is the kernel rpm scriptlet failure to copy/generate these boot files. After verifying that convert2rhel detected this situation of missing boot files correctly, the test goes on to do what the user instructed to do in such a case - that is reinstalling the kernel and re-runing grub2-mkconfig. Fixes oamg#721.

bocekm · 2023-02-22T13:58:08Z

Failing tests unrelated to the changes.

We incorrectly identified the test failures as unrelated to this PR. They were related and here's a fix of the test: #748.

On CentOS and Oracle Linux 7 the test was failing because we were renaming the vmlinuz and initramfs files in order to back them up, but we left them in the /boot/ folder. That caused the grub2-mkconfig to pick them up and generated a new grub configuration using these renamed boot files. This change does not back up the boot files but instead simply removes them, simulating better the core test scenario of this integration test which is the kernel rpm scriptlet failure to copy/generate these boot files. After verifying that convert2rhel detected this situation of missing boot files correctly, the test goes on to do what the user instructed to do in such a case - that is reinstalling the kernel and re-runing grub2-mkconfig. Fixes #721.

* Add a later check to warn the user about kernel boot files We added a few later checks to verify and warn the user if there are any errors with both `initramfs` `vmlinuz` files on their system after the conversion. This check has the intention to at least inform the user what are the steps that they need to run in order to fix that problem. Added some unit tests to assert that the correct path is being executed while we are checking for the kernel boot files. The tests added here tries to assert that both individual functions (checking for initramfs and vmlinuz) are called correctly and going through the correct paths, as well, checking that the workflow for the main check function is also doing what is supposed to. Signed-off-by: Rodolfo Olivieri <[email protected]> * Add unit tests for later checks Signed-off-by: Rodolfo Olivieri <[email protected]> * Add integration tests for the kernel boot files Signed-off-by: Rodolfo Olivieri <[email protected]> * Applied comments from review and a small refactor Refactored the `check_kernel_boot_files()` function to be simpler and not rely on functions that only do mainly one line of code. Signed-off-by: Rodolfo Olivieri <[email protected]> * Make an after conversion check * instead of having a full conversion running just to check for initramfs and vmlinuz files presence, introduce a check after each conversion * scrap the individual test case * deselect said check from cases with tainted files Signed-off-by: Daniel Diblik <[email protected]> * Apply suggestions from code review Co-authored-by: Preston Watson <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Change integration test Signed-off-by: Rodolfo Olivieri <[email protected]> * Apply suggestions from code review Co-authored-by: Daniel Diblik <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Remove leftover comparison Signed-off-by: Rodolfo Olivieri <[email protected]> * Apply suggestions from code review Co-authored-by: Toshio Kuratomi <[email protected]> Apply suggestions from code review Co-authored-by: Michal Bocek <[email protected]> * Change integration tests Signed-off-by: Rodolfo Olivieri <[email protected]> --------- Signed-off-by: Rodolfo Olivieri <[email protected]> Signed-off-by: Daniel Diblik <[email protected]> Co-authored-by: Daniel Diblik <[email protected]> Co-authored-by: Preston Watson <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Diblik <[email protected]> Co-authored-by: Michal Bocek <[email protected]>

On CentOS and Oracle Linux 7 the test was failing because we were renaming the vmlinuz and initramfs files in order to back them up, but we left them in the /boot/ folder. That caused the grub2-mkconfig to pick them up and generated a new grub configuration using these renamed boot files. This change does not back up the boot files but instead simply removes them, simulating better the core test scenario of this integration test which is the kernel rpm scriptlet failure to copy/generate these boot files. After verifying that convert2rhel detected this situation of missing boot files correctly, the test goes on to do what the user instructed to do in such a case - that is reinstalling the kernel and re-runing grub2-mkconfig. Fixes #721.

github-advanced-security bot found potential problems Feb 1, 2023

View reviewed changes

convert2rhel/utils.py Fixed Show fixed Hide fixed

convert2rhel/utils.py Fixed Show fixed Hide fixed

r0x0d requested a review from abadger February 1, 2023 20:20

r0x0d commented Feb 1, 2023

View reviewed changes

convert2rhel/utils.py Outdated Show resolved Hide resolved

r0x0d commented Feb 1, 2023

View reviewed changes

convert2rhel/utils.py Outdated Show resolved Hide resolved

r0x0d requested a review from Venefilyn February 2, 2023 12:13

r0x0d commented Feb 2, 2023

View reviewed changes

convert2rhel/unit_tests/utils_test.py Outdated Show resolved Hide resolved

r0x0d self-assigned this Feb 6, 2023

r0x0d added the kind/bug-fix A bug has been fixed label Feb 6, 2023

r0x0d force-pushed the check-for-kernel-boot-files-after-conversion branch from 2d743cf to 7de2027 Compare February 6, 2023 16:11

github-advanced-security bot found potential problems Feb 7, 2023

View reviewed changes

tests/integration/tier1/kernel-boot-files/test_handle_kernel_boot_files.py Fixed Show fixed Hide fixed

r0x0d force-pushed the check-for-kernel-boot-files-after-conversion branch 3 times, most recently from 2be7052 to cc26f2e Compare February 7, 2023 18:03

abadger reviewed Feb 8, 2023

View reviewed changes

convert2rhel/checks.py Show resolved Hide resolved

r0x0d force-pushed the check-for-kernel-boot-files-after-conversion branch from cc26f2e to 4d2b3d6 Compare February 8, 2023 14:55

github-advanced-security bot found potential problems Feb 8, 2023

View reviewed changes

tests/integration/tier1/kernel-boot-files/test_handle_corrupted_files.py Fixed Show fixed Hide fixed

r0x0d force-pushed the check-for-kernel-boot-files-after-conversion branch from 4d2b3d6 to 8508a4a Compare February 8, 2023 17:16

github-advanced-security bot found potential problems Feb 8, 2023

View reviewed changes

tests/integration/tier1/kernel-boot-files/test_handle_corrupted_files.py Fixed Show fixed Hide fixed

r0x0d force-pushed the check-for-kernel-boot-files-after-conversion branch 3 times, most recently from e3f26c1 to 10b3e6c Compare February 9, 2023 13:21

danmyway reviewed Feb 9, 2023

View reviewed changes

plans/tier1.fmf Outdated Show resolved Hide resolved

abadger approved these changes Feb 9, 2023

View reviewed changes

convert2rhel/checks.py Outdated Show resolved Hide resolved

convert2rhel/checks.py Outdated Show resolved Hide resolved

convert2rhel/checks.py Outdated Show resolved Hide resolved

convert2rhel/checks.py Outdated Show resolved Hide resolved

Venefilyn force-pushed the check-for-kernel-boot-files-after-conversion branch from b28095f to 7b2237c Compare February 10, 2023 12:50

r0x0d requested a review from danmyway February 10, 2023 18:16

r0x0d force-pushed the check-for-kernel-boot-files-after-conversion branch 2 times, most recently from 527f6e6 to 448e61a Compare February 17, 2023 19:16

github-advanced-security bot found potential problems Feb 17, 2023

View reviewed changes

tests/integration/tier1/kernel-boot-files/test_handle_corrupted_files.py Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Feb 20, 2023

View reviewed changes

tests/integration/tier1/kernel-boot-files/test_handle_corrupted_files.py Fixed Show fixed Hide fixed

r0x0d force-pushed the check-for-kernel-boot-files-after-conversion branch 7 times, most recently from 43287ec to 821fd59 Compare February 20, 2023 18:26

r0x0d and others added 2 commits February 20, 2023 16:30

Apply suggestions from code review

2c4da0b

Co-authored-by: Toshio Kuratomi <[email protected]> Apply suggestions from code review Co-authored-by: Michal Bocek <[email protected]>

Change integration tests

db25585

Signed-off-by: Rodolfo Olivieri <[email protected]>

r0x0d force-pushed the check-for-kernel-boot-files-after-conversion branch from 821fd59 to db25585 Compare February 20, 2023 19:30

danmyway approved these changes Feb 21, 2023

View reviewed changes

bocekm merged commit 616b4bd into oamg:main Feb 21, 2023

r0x0d deleted the check-for-kernel-boot-files-after-conversion branch February 22, 2023 01:36

bocekm mentioned this pull request Feb 22, 2023

[RHELC-752] Fix "missing boot files" test #748

Merged

7 tasks

This was referenced Feb 22, 2023

[RHELC-849, RHELC-850] Allow opting out of telemetry #723

Merged

Bump version to 1.2 #742

Merged

Venefilyn changed the title ~~[RHELC-752] Check for kernel boot files after conversion~~ [RHELC-752] Verify kernel boot files after conversion Feb 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RHELC-752] Verify kernel boot files after conversion #721

[RHELC-752] Verify kernel boot files after conversion #721

r0x0d commented Feb 1, 2023 •

edited by bocekm

Loading

codecov bot commented Feb 1, 2023 •

edited

Loading

r0x0d commented Feb 1, 2023 •

edited by bocekm

Loading

r0x0d commented Feb 7, 2023

abadger left a comment

r0x0d commented Feb 10, 2023

r0x0d commented Feb 10, 2023

bocekm commented Feb 17, 2023 •

edited

Loading

bocekm commented Feb 21, 2023

danmyway commented Feb 21, 2023

danmyway left a comment

bocekm commented Feb 22, 2023

[RHELC-752] Verify kernel boot files after conversion #721

[RHELC-752] Verify kernel boot files after conversion #721

Conversation

r0x0d commented Feb 1, 2023 • edited by bocekm Loading

codecov bot commented Feb 1, 2023 • edited Loading

Codecov Report

r0x0d commented Feb 1, 2023 • edited by bocekm Loading

Steps to reproduce locally (and for testing)

Notes

r0x0d commented Feb 7, 2023

abadger left a comment

Choose a reason for hiding this comment

r0x0d commented Feb 10, 2023

r0x0d commented Feb 10, 2023

bocekm commented Feb 17, 2023 • edited Loading

bocekm commented Feb 21, 2023

danmyway commented Feb 21, 2023

danmyway left a comment

Choose a reason for hiding this comment

bocekm commented Feb 22, 2023

r0x0d commented Feb 1, 2023 •

edited by bocekm

Loading

codecov bot commented Feb 1, 2023 •

edited

Loading

r0x0d commented Feb 1, 2023 •

edited by bocekm

Loading

bocekm commented Feb 17, 2023 •

edited

Loading