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

fix: remove bootupd #152

Merged
merged 1 commit into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
13 changes: 9 additions & 4 deletions greenboot.spec
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Name: greenboot
Version: 0.15.5
Release: 2%{?dist}
Release: 3%{?dist}
Summary: Generic Health Check Framework for systemd
License: LGPL-2.1-or-later

Expand All @@ -19,7 +19,6 @@ BuildRequires: systemd-rpm-macros
Requires: systemd >= 240
Requires: grub2-tools-minimal
Requires: rpm-ostree
Requires: bootupd
# PAM is required to programatically read motd messages from /etc/motd.d/*
# This causes issues with RHEL-8 as the fix isn't there an el8 is on pam-1.3.x
Requires: pam >= 1.4.0
Expand Down Expand Up @@ -66,7 +65,6 @@ mkdir -p %{buildroot}%{_prefix}/lib/%{name}/check/required.d
mkdir %{buildroot}%{_prefix}/lib/%{name}/check/wanted.d
mkdir %{buildroot}%{_prefix}/lib/%{name}/green.d
mkdir %{buildroot}%{_prefix}/lib/%{name}/red.d
install -D -t %{buildroot}%{_prefix}/lib/bootupd/grub2-static/configs.d grub2/greenboot.cfg
mkdir -p %{buildroot}%{_unitdir}
mkdir -p %{buildroot}%{_unitdir}/greenboot-healthcheck.service.d
mkdir -p %{buildroot}%{_tmpfilesdir}
Expand All @@ -79,6 +77,7 @@ install -DpZm 0644 usr/lib/tmpfiles.d/greenboot-status-motd.conf %{buildroot}%{_
install -DpZm 0755 usr/lib/greenboot/check/required.d/* %{buildroot}%{_prefix}/lib/%{name}/check/required.d
install -DpZm 0755 usr/lib/greenboot/check/wanted.d/* %{buildroot}%{_prefix}/lib/%{name}/check/wanted.d
install -DpZm 0644 etc/greenboot/greenboot.conf %{buildroot}%{_sysconfdir}/%{name}/greenboot.conf
install -DpZm 0644 etc/grub.d/greenboot.cfg %{buildroot}%{_sysconfdir}/grub.d/greenboot.cfg

%post
%systemd_post greenboot-healthcheck.service
Expand All @@ -91,6 +90,9 @@ install -DpZm 0644 etc/greenboot/greenboot.conf %{buildroot}%{_sysconfdir}/%{nam
%systemd_post greenboot-grub2-set-success.service
%systemd_post greenboot-rpm-ostree-grub2-check-fallback.service
%systemd_post redboot-auto-reboot.service
if [ -d /usr/lib/bootupd/grub2-static/configs.d ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Nope! This is not going to work.

Copy link
Member

Choose a reason for hiding this comment

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

you can't move package files around in post scripts, it means for example the rpm won't verify properly for one, and it's also extremely prone to errors and interruptions. Also if someone does custom image it may well break.

Copy link
Member

Choose a reason for hiding this comment

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

@nullr0ute do you have any suggestions for conditionalizing where the grub config lands as Sayan is trying to do?

Copy link
Member

Choose a reason for hiding this comment

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

The general rule of thumb is that you shouldn't do it. I think we need to step back and look at the actual problem.

Copy link
Member Author

@say-paul say-paul Sep 9, 2024

Choose a reason for hiding this comment

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

So I applied a hard link instead of mv also added in %postun to remove the ln post remove.

Copy link

Choose a reason for hiding this comment

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

Is there a chance that /etc would be mounted on a different flesystem?
Is symlink better for what we're trying to do?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats a good point, hardlink will have issue between different file systems, softlink didn't work so I plan on using cp as it may not have an integrity issue as pointed out by @nullr0ute

cp /etc/grub.d/greenboot.cfg /usr/lib/bootupd/grub2-static/configs.d
fi

%post default-health-checks
%systemd_post greenboot-loading-message.service
Expand Down Expand Up @@ -119,6 +121,9 @@ install -DpZm 0644 etc/greenboot/greenboot.conf %{buildroot}%{_sysconfdir}/%{nam
%systemd_postun greenboot-grub2-set-counter.service
%systemd_postun greenboot-grub2-set-success.service
%systemd_postun greenboot-rpm-ostree-grub2-check-fallback.service
if [ -f /usr/lib/bootupd/grub2-static/configs.d/greenboot.cfg ]; then
rm -f /usr/lib/bootupd/grub2-static/configs.d/greenboot.cfg
fi

%postun default-health-checks
%systemd_postun greenboot-loading-message.service
Expand All @@ -143,7 +148,7 @@ install -DpZm 0644 etc/greenboot/greenboot.conf %{buildroot}%{_sysconfdir}/%{nam
%dir %{_prefix}/lib/%{name}/red.d
%{_exec_prefix}/lib/motd.d/boot-status
%{_tmpfilesdir}/greenboot-status-motd.conf
%{_prefix}/lib/bootupd/grub2-static/configs.d/*.cfg
%{_sysconfdir}/grub.d/*.cfg
Copy link
Member

Choose a reason for hiding this comment

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

This is an unrelated fix, it should be in a different commit/PR.

Copy link
Member Author

@say-paul say-paul Sep 9, 2024

Choose a reason for hiding this comment

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

I think you saw comparison between two commits, which made it seem unrelated.
It changes the path from bootupd/static-config to grub.d:

https://github.com/fedora-iot/greenboot/pull/152/files#r1747377644

%dir %{_libexecdir}/%{name}
%{_libexecdir}/%{name}/%{name}
%{_libexecdir}/%{name}/greenboot-grub2-set-success
Expand Down
Loading