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

Add support for POWEROFF_WAIT parameter #384

Merged
merged 1 commit into from
Mar 13, 2017

Conversation

bigon
Copy link
Contributor

@bigon bigon commented Jan 28, 2017

Use "systemctl reboot --force --force" to reboot the machine has not
been shutdown after the POWEROFF_WAIT delay.

Closes: #382

@SBINDIR@/upsmon -K >/dev/null 2>&1 && @SBINDIR@/upsdrvctl shutdown

if @SBINDIR@/upsmon -K >/dev/null 2>&1; then
wait_delay=`/bin/sed -ne 's#^ *POWEROFF_WAIT= *\(.*\)$#\1#p' /etc/nut/nut.conf`
Copy link
Member

Choose a reason for hiding this comment

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

As an .in template this should not refer to specific pathnames (etc => @sysconfdir@)...

And I'm not fully sure about /bin/sed ;)

Copy link
Member

@jimklimov jimklimov Jan 28, 2017

Choose a reason for hiding this comment

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

Also, perhaps sed should look for numbers spelled like \([0-9][0-9]*\)[^0-9]*$ for those dummiest implementations?


@SBINDIR@/upsdrvctl shutdown

if [ -n "$wait_delay" ] ; then
Copy link
Member

Choose a reason for hiding this comment

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

Should the check be for -n or for actually being a non-negative number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's also possible to pass strings here, like "15m" so I don't think it's possible

Copy link
Member

Choose a reason for hiding this comment

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

Oh, right. Well then, if "standard linux" sleep accepts whatever is coming, ok. If not, the sleep will be quite short ;)


if [ -n "$wait_delay" ] ; then
sleep $wait_delay
/bin/systemctl reboot --force --force
Copy link
Member

Choose a reason for hiding this comment

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

For posterity (so someone does not remove a "duplicate" --force, a comment would be welcome.

fi
fi

exit 0
Copy link
Member

Choose a reason for hiding this comment

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

As an added bonus, this should solve that annoying message just before OS goes down along the lines that "systemd failed to stop nutshutdown service" on non-upsmon-master systems... LGTM just for that ;)

Choose a reason for hiding this comment

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

Is there any reason /bin/systemctl reboot --force --force isn't being run when POWEROFF_WAIT is not set or am I misunderstanding something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peterhoeg upsdrvctl shutdown returns almost immediately, the UPS continues to give power for a few seconds after it receive the request to shutdown to let the machine shutdown cleanly.

@bigon bigon force-pushed the bug-382 branch 2 times, most recently from ea61fd0 to eb17566 Compare January 28, 2017 23:19
Use "systemctl reboot --force --force" to reboot the machine has not
been shutdown after the POWEROFF_WAIT delay.

Closes: networkupstools#382
@jimklimov
Copy link
Member

👍

@jimklimov
Copy link
Member

Anyone else wanna chime in? @peterhoeg?

@bigon
Copy link
Contributor Author

bigon commented Jan 28, 2017

I definitely would like to see this tested a bit more before it's merged :p

@SBINDIR@/upsmon -K >/dev/null 2>&1 && @SBINDIR@/upsdrvctl shutdown

if @SBINDIR@/upsmon -K >/dev/null 2>&1; then
wait_delay=`/bin/sed -ne 's#^ *POWEROFF_WAIT= *\(.*\)$#\1#p' @sysconfdir@/nut.conf`
Copy link
Member

Choose a reason for hiding this comment

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

nitpicking, but prefer @CONFPATH@ over @sysconfdir@, otherwise you can end up with something not fully expanded like "${prefix}/etc/nut.conf" when not specifying --sysconfdir and using default flags.
I'll do some more testing over the afternoon if possible... and I'll take care of this CONFPATH when merging too ;)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, those ${prefix} verbatims are evil and hard to avoid, struck a few more of those in the system/SMF instance wrapping PR ;)

@aquette aquette merged commit 49cd54a into networkupstools:master Mar 13, 2017
aquette pushed a commit that referenced this pull request Mar 13, 2017
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