Skip to content

wip: Support 'reload' action for supervise-daemon #1091

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

Open
meoovv wants to merge 6 commits into
base: master
Choose a base branch
from
+17 −1
Open

wip: Support 'reload' action for supervise-daemon #1091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5429717
Support 'reload' action for services with supervisor=supervise-daemon
Sep 27, 2026
a8b883a
Document 'reload' action for s6 and supervise-daemon services
Sep 27, 2026
44c5983
Document 'reload' action for s6 and supervise-daemon services [2]
Sep 27, 2026
5430474
Remove useless code from supervise-daemon's reload function
Sep 27, 2026
c30a1d6
Support reloadsig= option for supervise-daemon
Sep 27, 2026
874e8de
Support reloadsig= option for supervise-daemon [2]
Sep 27, 2026
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
3 changes: 3 additions & 0 deletions man/openrc-run.8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ Stops and starts the service, including dependencies. This cannot be
overridden. See the description of the RC_CMD variable below for the
method to make your service behave differently when restart is being
executed.
.It Ar reload
Sends reload (SIGHUP) signal to the service, only supported when supervisor is supervise-daemon
or s6. Note that this may kill daemons which do not process SIGHUP, supervise-daemon will restart them.
Comment on lines +55 to +56

@navi-desu navi-desu Sep 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

start-stop-daemon also has --signal, so there's no reason it couldn't also implement reload()

while we do want to move towards defaulting to supervise-daemon, plenty of services ship a reload function for s-s-d, so having a way for those services to drop their custom functions would help with that goal

this leaves only runit, which does support sending a signal but honestly i have no idea how is our state of support for runit at this point that maybe we leave it missing for now

@meoovv meoovv Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

well, theres some activity around adding reload support to s-s-d in #1032, but author doesnt seem to be addressing concerns. i could create a similar pr for s-s-d, then we could wait for author of #1052 and once everything has been merged, list reload in openrc-run.sh

@navi-desu navi-desu Sep 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

f_ is very much active on the openrc irc channel and ska is also around there. i mostly asked both of them to wait with this features until 0.64 was out (plus some time on top to chase regressions), there were too many features stacked on master

@meoovv meoovv Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so, its probably time to call them back now 🧐

@thesamesam thesamesam Sep 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@meoovv I appreciate you're enthusiastic, but you're being rather pushy. Please try to dial it back.

@thesamesam thesamesam Sep 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@meoovv Yes, but projects don't generally need someone to come in and start delegating tasks. People work on FOSS projects when they feel like it. You don't need to tell navi to ping them, she can do that herself if or when she wants to.

But also, 0.64 and 0.64.1 only just came out. It's not like it's been ages.

discussions should be focused and task-oriented

It's not the same as running a meeting in a company.

@meoovv meoovv Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

disagree, but i can clarify things more gently if people do feel uncomfortable.

@thesamesam thesamesam Sep 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm asking you to not try direct people.

@funderscore1 funderscore1 Oct 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

but author doesnt seem to be addressing concerns.

FWIW my PR does both s-s-d and s-d. I am pretty sure I addressed everything, what are you thinking of in particular that needs addressing?

both #1032 and #1052 use name reloadsig for the reload-signal option and provide no boolean option for enabling or disabling reloads. so to mark a service as non-reloadable, user needs to set reloadsig="".

There is no need for a boolean here, IMO. If reloadsig is not set, then service is not reloadable. There shouldn't be a need to manually set reloadsig="" in service scripts for that.

@funderscore1 funderscore1 Oct 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

And between the time I opened the PR and now I also took a break from userspace work for reasons. Go easy on me

.It Ar status
Shows the status of the service. The return code matches the status, with the
exception of "started" returning 0 to match standard command behaviour.
Expand Down
1 change: 1 addition & 0 deletions sh/openrc-run.sh.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ default_reload()
{
case "$supervisor" in
s6) s6_reload ;;
supervise-daemon) supervise_reload ;;
?*)
eerror "$RC_SVCNAME: undefined function 'reload'"
exit 1
Expand Down
14 changes: 13 additions & 1 deletion sh/supervise-daemon.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# This file may not be copied, modified, propagated, or distributed
# except according to the terms contained in the LICENSE file.

extra_commands="healthcheck unhealthy ${extra_commands}"
extra_commands="healthcheck unhealthy reload ${extra_commands}"

@navi-desu navi-desu Sep 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if it's becoming a default, might as well add to openrc-run directly like start and stop


supervise_start()
{
Expand Down Expand Up @@ -83,6 +83,18 @@ supervise_stop()
eend $? "Failed to stop ${name:-$RC_SVCNAME}"
}

supervise_reload()
{
case "${reloadsig}" in
"") eerror "Reloading is disabled for ${name:-$RC_SVCNAME}" ;;
*)
ebegin "Reloading ${name:-$RC_SVCNAME}"
supervise-daemon "${RC_SVCNAME}" --signal "${reloadsig:-"HUP"}"
eend $? "Failed to reload ${name:-$RC_SVCNAME}"
;;
esac
}

_check_supervised()
{
local child_pid start_time
Expand Down
Loading