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.

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

Simplify grub_efi_path on redhat family #804

Merged
merged 1 commit into from May 2, 2023

Conversation

eb4x
Copy link
Contributor

@eb4x eb4x commented May 2, 2023

Remains backwards-compatible with supported distros, and we don't have to look for openeuler or oraclelinux or any other EL variant.

Remains backwards-compatible with supported distros, and we don't
have to look for openeuler or oraclelinux or any other EL variant.
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I recently dove into the (patched) source and confirmed it the lower case ID from /etc/os-release. This is a close approximation.

@ekohl ekohl merged commit 71e2e30 into theforeman:master May 2, 2023
6 of 10 checks passed
@m-bucher
Copy link

m-bucher commented Feb 7, 2024

@ekohl @eb4x I must object at least in part to this PR, because I actually install foreman on OracleLinux from time to time.

I am not sure if there is a better way to fix this than just reverting this commit (which is what I currently do).

@ekohl
Copy link
Member

ekohl commented Feb 7, 2024

@m-bucher I think this shouldn't break Oracle, unless the name in /etc/os-release differs from the Puppet fact name. Because that's how grub2 on Red Hat is patched: use the name from os-release (downcased) in the file path

@eb4x
Copy link
Contributor Author

eb4x commented Feb 8, 2024

Unfortunately, he's right. I installed OracleLinux 8.9 on an efi vm. It's /boot/efi/EFI/redhat, because why the 🤬 wouldn't it be.

Would this be an ok way to write an exception for this exceptional os?

$grub_efi_path = unless $facts['os']['name'] == 'OracleLinux' {
    downcase($facts['os']['name'])
} else {
  'redhat'
}

@ekohl
Copy link
Member

ekohl commented Feb 8, 2024

I would use a regular if instead of unless, but yes. This is (sadly) going to be needed

@eb4x
Copy link
Contributor Author

eb4x commented Feb 8, 2024

I "reverted" back to the previous way with the case statement, with the logic swapped. #828

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

Successfully merging this pull request may close these issues.

None yet

4 participants