[RHELC-1331] Port post_ponr_set_efi_configuration() to Action framework #1256

pr-watson · 2024-06-05T17:57:56Z

This PR ports the post_ponr_set_efi_configuration function to the action framework. Changing any relevant logger crictical, warning and info messages to Error results and Warning and Info messages respectively.

Jira Issues:

RHELC-1331

Checklist

PR has been tested manually in a VM (either author or reviewer)
Jira issue has been made public if possible
[RHELC-] or [HMS-] is part of the PR title
GitHub label has been added to help with Release notes
PR title explains the change from the user's point of view
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 if relevant

has-bot · 2024-06-05T17:58:38Z

/packit test --labels sanity

Comment generated by an automation.

Log | Bot Usage

convert2rhel/actions/conversion/set_efi_config.py

convert2rhel/unit_tests/actions/conversion/set_efi_config_test.py

codecov · 2024-06-25T22:55:33Z

Codecov Report

Attention: Patch coverage is 98.98990% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.30%. Comparing base (9f2e407) to head (6fe3946).
Report is 10 commits behind head on main.

Files	Patch %	Lines
convert2rhel/actions/conversion/set_efi_config.py	98.97%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1256      +/-   ##
==========================================
+ Coverage   96.19%   96.30%   +0.11%     
==========================================
  Files          58       59       +1     
  Lines        4831     4872      +41     
  Branches      848      858      +10     
==========================================
+ Hits         4647     4692      +45     
+ Misses        106      103       -3     
+ Partials       78       77       -1

Flag	Coverage Δ
centos-linux-7	`91.61% <98.98%> (+0.17%)`	⬆️
centos-linux-8	`92.52% <98.98%> (+0.14%)`	⬆️
centos-linux-9	`92.56% <98.98%> (+0.14%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

convert2rhel/unit_tests/actions/conversion/set_efi_config_test.py

r0x0d

Some comments, otherwise, looks good.

convert2rhel/unit_tests/actions/conversion/set_efi_config_test.py

convert2rhel/actions/conversion/set_efi_config.py

convert2rhel/unit_tests/actions/conversion/set_efi_config_test.py

r0x0d · 2024-08-06T18:24:54Z

/packit test --labels sanity

r0x0d · 2024-08-06T18:25:03Z

@pr-watson, please squash the commits

convert2rhel/actions/conversion/set_efi_config.py

Venefilyn

With the Message ID guidelines I setup in the docs, can you come up with something else for the message IDs? If it's difficult or you feel like there is a better way to do message IDs just suggest it in the chat or update the wiki (and let us know)
https://github.com/oamg/convert2rhel/wiki/Action-Framework#message-ids

I also didn't go through all the set_message() actions as I'm just wondering now what the audience for it is.

@oamg/convert2rhel-developers is the intention for the action messages that are displayed in Insights and others to be for readability where novice developer or sysadmins can easily know what to do without prior knowledge or something else? If that is the case we should really be improving the titles, messages, diagnosis, and remediations to be more readable

convert2rhel/actions/conversion/set_efi_config.py

@hosekadam

Without this dependency, the action can run before the package conversion in the system. Ideally, we want this to run after the `KERNEL_PACKAGES_INSTALLATION`, as it is no use to have this run before the package replacement. Pointed out by @hosekadam in oamg#1256 (comment)

convert2rhel/actions/conversion/set_efi_config.py

r0x0d · 2024-08-15T13:54:47Z

/packit build

pr-watson · 2024-08-15T14:08:46Z

/packit build

pr-watson · 2024-08-15T14:13:37Z

/packit build

danmyway

Tests look good from QE POV

hosekadam · 2024-08-15T14:33:24Z

convert2rhel/actions/conversion/set_efi_config.py

+            logger.info("Moving '%s' to '%s'" % (src_file, dst_file))
+
+            try:
+                shutil.move(src_file, dst_file)


I'm thinking if we shouldn't go back to the copy2, since it doesn't make a big difference (the /boot/efi/EFI/centos won't be removed) and as we discussed here #1256 (comment) we can refactor this later and for now we should be closer to the original code.

If we decide to move, sorry Preston for an extra work you had with my ideas

I will move tihs back to copy2 and rebaes the PR. Don't worry @pr-watson

Yes we're sticking with move, it's a good addition

pr-watson · 2024-08-15T14:38:12Z

/packit build

r0x0d

Let's merge after the required actions passes

all addressed

r0x0d · 2024-08-15T15:46:48Z

rhel 7 rpm not being built is not a PR problem. It is happening in other PRs as well.

@hosekadam

Without this dependency, the action can run before the package conversion in the system. Ideally, we want this to run after the `KERNEL_PACKAGES_INSTALLATION`, as it is no use to have this run before the package replacement. Pointed out by @hosekadam in #1256 (comment)

pr-watson added kind/feature New feature or request tests/sanity PR ready to run the sanity test suit. Equivalent to `/packit test --labels sanity`. labels Jun 5, 2024

github-advanced-security bot found potential problems Jun 5, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Fixed Show resolved Hide resolved

r0x0d reviewed Jun 18, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Outdated Show resolved Hide resolved

r0x0d reviewed Jun 18, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Show resolved Hide resolved

r0x0d reviewed Jun 18, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Jun 18, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Jun 24, 2024

View reviewed changes

pr-watson force-pushed the migrate-post-conversion-set-efi-config branch 2 times, most recently from eb501b7 to 0a90d52 Compare June 25, 2024 22:54

github-advanced-security bot found potential problems Jun 25, 2024

View reviewed changes

convert2rhel/unit_tests/actions/conversion/set_efi_config_test.py Fixed Show fixed Hide fixed

r0x0d requested changes Jun 26, 2024

View reviewed changes

r0x0d force-pushed the migrate-post-conversion-set-efi-config branch from 216487d to 773197a Compare July 16, 2024 18:58

pr-watson force-pushed the migrate-post-conversion-set-efi-config branch from 84b3f58 to 3599a04 Compare August 5, 2024 18:42

Venefilyn assigned r0x0d Aug 6, 2024

pr-watson mentioned this pull request Aug 6, 2024

Post Conversion Action Framework merge order #1331

Closed

13 tasks

r0x0d reviewed Aug 6, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Outdated Show resolved Hide resolved

r0x0d reviewed Aug 6, 2024

View reviewed changes

convert2rhel/unit_tests/actions/conversion/set_efi_config_test.py Outdated Show resolved Hide resolved

pr-watson requested a review from a team as a code owner August 7, 2024 13:59

r0x0d force-pushed the migrate-post-conversion-set-efi-config branch from 8ad4537 to d0bd3df Compare August 7, 2024 16:23

pr-watson force-pushed the migrate-post-conversion-set-efi-config branch from d0bd3df to 1b10818 Compare August 7, 2024 17:55

r0x0d reviewed Aug 8, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Outdated Show resolved Hide resolved

r0x0d reviewed Aug 8, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Show resolved Hide resolved

r0x0d reviewed Aug 8, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Outdated Show resolved Hide resolved

Venefilyn requested changes Aug 8, 2024

View reviewed changes

pr-watson force-pushed the migrate-post-conversion-set-efi-config branch from 974cf4b to d3a4d17 Compare August 8, 2024 20:30

hosekadam reviewed Aug 15, 2024

View reviewed changes

convert2rhel/actions/conversion/set_efi_config.py Outdated Show resolved Hide resolved

pr-watson force-pushed the migrate-post-conversion-set-efi-config branch from 2eb0b7d to a6a001e Compare August 15, 2024 14:01

pr-watson and others added 3 commits August 15, 2024 10:14

[RHELC-1331] Port post_ponr_set_efi_configuration() to Action framework

1119727

Apply refactor to CopyGrubFiles

5444544

Update action results and messages

fd27d13

pr-watson force-pushed the migrate-post-conversion-set-efi-config branch from a6a001e to 6b7b62c Compare August 15, 2024 14:14

danmyway approved these changes Aug 15, 2024

View reviewed changes

hosekadam reviewed Aug 15, 2024

View reviewed changes

pr-watson added 6 commits August 15, 2024 11:41

Updated action IDs to new format

c5c2fb5

Add Convert task for bootloader

b1426b3

Improve unable to find grub config message

58ef1bb

Add condition to skip copy of non-existing files

ba8efa6

Add condition to check if system is UEFI in ReplaceEfiBootEntry action

d09cd88

Add unit test for replace_efi_boot_entry_non_uefi

af13d12

r0x0d force-pushed the migrate-post-conversion-set-efi-config branch 2 times, most recently from 32faa2d to ea8cd1e Compare August 15, 2024 14:44

r0x0d approved these changes Aug 15, 2024

View reviewed changes

Update actions to skip when system is non uefi

6fe3946

r0x0d force-pushed the migrate-post-conversion-set-efi-config branch from ea8cd1e to 6fe3946 Compare August 15, 2024 15:41

hosekadam merged commit 80333d9 into oamg:main Aug 15, 2024
17 of 18 checks passed

This was referenced Aug 21, 2024

Bump to version 2.1.0 #1352

Merged

Bump to version 2.1.0 #1356

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RHELC-1331] Port post_ponr_set_efi_configuration() to Action framework #1256

[RHELC-1331] Port post_ponr_set_efi_configuration() to Action framework #1256

pr-watson commented Jun 5, 2024 •

edited

Loading

has-bot commented Jun 5, 2024

codecov bot commented Jun 25, 2024 •

edited

Loading

r0x0d left a comment

r0x0d commented Aug 6, 2024

r0x0d commented Aug 6, 2024

Venefilyn left a comment

r0x0d commented Aug 15, 2024

pr-watson commented Aug 15, 2024

pr-watson commented Aug 15, 2024

danmyway left a comment

hosekadam Aug 15, 2024

r0x0d Aug 15, 2024

pr-watson Aug 15, 2024

pr-watson commented Aug 15, 2024

r0x0d left a comment

r0x0d commented Aug 15, 2024

[RHELC-1331] Port post_ponr_set_efi_configuration() to Action framework #1256

[RHELC-1331] Port post_ponr_set_efi_configuration() to Action framework #1256

Conversation

pr-watson commented Jun 5, 2024 • edited Loading

has-bot commented Jun 5, 2024

codecov bot commented Jun 25, 2024 • edited Loading

Codecov Report

r0x0d left a comment

Choose a reason for hiding this comment

r0x0d commented Aug 6, 2024

r0x0d commented Aug 6, 2024

Venefilyn left a comment

Choose a reason for hiding this comment

r0x0d commented Aug 15, 2024

pr-watson commented Aug 15, 2024

pr-watson commented Aug 15, 2024

danmyway left a comment

Choose a reason for hiding this comment

hosekadam Aug 15, 2024

Choose a reason for hiding this comment

r0x0d Aug 15, 2024

Choose a reason for hiding this comment

pr-watson Aug 15, 2024

Choose a reason for hiding this comment

pr-watson commented Aug 15, 2024

r0x0d left a comment

Choose a reason for hiding this comment

r0x0d commented Aug 15, 2024

pr-watson commented Jun 5, 2024 •

edited

Loading

codecov bot commented Jun 25, 2024 •

edited

Loading