Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RHELC-638] Splitting bootloader code #1080

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[RHELC-638] Splitting bootloader code #1080

wants to merge 1 commit into from

Conversation

pr-watson
Copy link
Contributor

This PR separates bootloader specific functions to a new bootloader.py file and removes them from their current location in grub.py. A new folder bootloader has been created to hold both the grub.py and bootloader.py files. A new unit test file for the bootloader code has been created as well and given it's own folder to contain the bootloader and grub unit tests.

Jira Issues: RHELC-638

Checklist

  • 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
  • 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

@pr-watson pr-watson added enhancement New feature or request tests-run-all Run the full test suite. Equivalent to `/packit test`. labels Feb 5, 2024
@has-bot
Copy link
Member

has-bot commented Feb 5, 2024

/packit test

Comment generated by an automation.

Venefilyn
Venefilyn approved these changes Feb 14, 2024
View reviewed changes
Copy link
Member

@Venefilyn Venefilyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Needs a rebase then integration test run

convert2rhel/bootloader/bootloader.py
# Copyright(C) 2021 Red Hat, Inc.
# Copyright(C) 2024 Red Hat, Inc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, not needed to be changed. Doesn't hurt but doesn't matter

convert2rhel/bootloader/bootloader.py Outdated
dev_number = _get_device_number(convert2rhel.bootloader.grub.get_efi_partition())
blk_dev = convert2rhel.bootloader.grub.get_grub_device()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could import grub and instead use this to make it simpler

Suggested change
dev_number = _get_device_number(convert2rhel.bootloader.grub.get_efi_partition())
blk_dev = convert2rhel.bootloader.grub.get_grub_device()
dev_number = _get_device_number(grub.get_efi_partition())
blk_dev = grub.get_grub_device()

github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 14, 2024
View reviewed changes
convert2rhel/unit_tests/bootloader/bootloader_test.py
import six

from convert2rhel import utils
from convert2rhel.bootloader import bootloader, grub

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'grub' is not used.
convert2rhel/unit_tests/bootloader/grub_test.py

__metaclass__ = type

import copy

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'copy' is not used.
convert2rhel/unit_tests/bootloader/grub_test.py
import six

from convert2rhel import utils
from convert2rhel.bootloader import bootloader, grub

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'bootloader' is not used.
convert2rhel/unit_tests/bootloader/bootloader_test.py


# TODO(pstodulk): put here a real examples of an output..
_SEC_STDOUT_ENABLED = "secure boot enabled"

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable '_SEC_STDOUT_ENABLED' is not used.
convert2rhel/unit_tests/bootloader/bootloader_test.py

# TODO(pstodulk): put here a real examples of an output..
_SEC_STDOUT_ENABLED = "secure boot enabled"
_SEC_STDOUT_DISABLED = "e.g. nothing..."

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable '_SEC_STDOUT_DISABLED' is not used.
convert2rhel/unit_tests/bootloader/grub_test.py


# TODO(pstodulk): put here a real examples of an output..
_SEC_STDOUT_ENABLED = "secure boot enabled"

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable '_SEC_STDOUT_ENABLED' is not used.
convert2rhel/unit_tests/bootloader/grub_test.py

# TODO(pstodulk): put here a real examples of an output..
_SEC_STDOUT_ENABLED = "secure boot enabled"
_SEC_STDOUT_DISABLED = "e.g. nothing..."

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable '_SEC_STDOUT_DISABLED' is not used.
@Venefilyn
Copy link
Member

We do have unit tests failing here too

@pr-watson pr-watson marked this pull request as draft February 19, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Venefilyn Venefilyn Venefilyn approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request tests-run-all Run the full test suite. Equivalent to `/packit test`.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pr-watson @has-bot @Venefilyn