-
Notifications
You must be signed in to change notification settings - Fork 29
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
Version 0.1 #5
Version 0.1 #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some notes/questions
Description=greenboot Success Scripts Runner | ||
BindsTo=greenboot.target | ||
After=greenboot.target | ||
OnFailure=redboot.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jlebon: This can be moved into the target. Where do you prefer it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This belongs in the target anyway, no? We should run redboot.service
if we fail to run check
scripts, not if we fail to run green
scripts, right?
|
||
[Unit] | ||
Description=greenboot Success Scripts Runner | ||
BindsTo=greenboot.target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works with Requires
too. Or RequiredBy
on the target. BindsTo is a stronger binding. I was thinking it might make sense because once we have Watchdog check services and they make the target fail, this should be propagated to greenboot.service, too. BindsTo does that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh gotcha, I was wondering about that. Hmm, so it's possible to mark a target as failed after the fact?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, we'll use Requires
, as discussed on IRC
#!/bin/bash | ||
set -euo pipefail | ||
|
||
echo "Health Check SUCCESS! Boot status is GREEN" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add command to update motd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @dustymabe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
#!/bin/bash | ||
set -euo pipefail | ||
|
||
echo "Health Check FAILURE! Boot status is RED" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add command to update motd
mkdir %{buildroot}%{_sysconfdir}/%{name}.d/red | ||
install -Dpm 0755 etc/greenboot.d/red/00_redboot_notification.sh %{buildroot}%{_sysconfdir}/%{name}.d/red/00_redboot_notification.sh | ||
install -Dpm 0755 etc/greenboot.d/red/98_ostree_rollback.sh %{buildroot}%{_sysconfdir}/%{name}.d/red/98_ostree_rollback.sh | ||
install -Dpm 0755 etc/greenboot.d/red/99_reboot.sh %{buildroot}%{_sysconfdir}/%{name}.d/red/99_reboot.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
split this out into its own package, so we don't force everyone to reboot on a red boot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's done
|
||
echo "Running greenboot Wanted Scripts" | ||
|
||
exit 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the trailing exit 0
s here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably not
# vi: set ft=ruby : | ||
|
||
Vagrant.configure("2") do |config| | ||
config.vm.box = "fedora/28-cloud-base" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to use the Fedora Atomic Host box here and use rpm-ostree install
, but I'm cool leaving that for later!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do I enable copr repo with that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't but you can curl the repo file into /etc/yum.repos.d/. It's fine to leave it like it is for now.
|
||
[Unit] | ||
Description=greenboot Success Scripts Runner | ||
BindsTo=greenboot.target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh gotcha, I was wondering about that. Hmm, so it's possible to mark a target as failed after the fact?
|
||
[Service] | ||
Type=oneshot | ||
ExecStart=/usr/libexec/greenboot/greenboot.sh green |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this service need something like WantedBy=multi-user.target
? What activates it otherwise? You could also add to greenboot.target
:
Wants=greenboot.service
Before=greenboot.service
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. have been doing it manually right now.
ExecStart=/usr/libexec/greenboot/greenboot.sh check | ||
|
||
[Install] | ||
RequiredBy=greenboot.target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this feels a bit "indirect". I would expect this dependency to happen directly in greenboot.target
instead, e.g. something like:
Requires=greenboot-healthcheck.service
After=greenboot-healthcheck.service
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would certainly work. Reasoning behind it was to keep the structure of this service just as any other, custom check service
Description=greenboot Success Scripts Runner | ||
BindsTo=greenboot.target | ||
After=greenboot.target | ||
OnFailure=redboot.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This belongs in the target anyway, no? We should run redboot.service
if we fail to run check
scripts, not if we fail to run green
scripts, right?
echo -e "\e[1;31mRed Script '$(basename $script)' FAILURE (exit code '$rc'). Continuing...\e[0m" >&2 | ||
fi | ||
done | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: we can probably dedup all this even more if we have a function that takes the path to scan, whether it's fatal, and whatever identifier we want to use when printing errors to the journal. But we can do that later!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had this in mind for when converting to C :) 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The color codes aren't picked up by systemd-cat :(
73a301d
to
48fc3ac
Compare
|
||
[Unit] | ||
Description=greenboot Success Target | ||
Wants=greenboot.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jlebon re: https://github.com/LorbusChris/greenboot/pull/4#discussion_r195443658
change this to Requires
to also fail the target on green script failures, not just health check failures, creating a circular dependency between both the service and the target?
No description provided.