Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 348724

Summary: sys-apps/openrc does not respect custom restart function
Product: Gentoo Hosted Projects Reporter: William Hubbs <williamh>
Component: OpenRCAssignee: OpenRC Team <openrc>
Status: RESOLVED FIXED    
Severity: normal CC: esigra, idl0r, roy
Priority: High    
Version: unspecified   
Hardware: All   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---
Bug Depends on:    
Bug Blocks: 213988    
Attachments: emerge-info.txt
/etc/init.d/junk
Patch for hb-working-rcscripts (handbook chapter on init scripts)

Description William Hubbs gentoo-dev 2010-12-14 16:24:35 UTC
If a service script defines a custom restart function, this function is
not executed.

Steps to reproduce:

- choose a service with a custom restart function (I will attach a
  "junk" service that can be used for testing this).
- run "/etc/init.d/[service] restart" or "rc-service [service] restart".
- you will see that the custom restart function is not executed.
Comment 1 William Hubbs gentoo-dev 2010-12-14 16:25:59 UTC
Created attachment 257150 [details]
emerge-info.txt


my emerge --info.
Comment 2 William Hubbs gentoo-dev 2010-12-14 16:34:48 UTC
Created attachment 257152 [details]
/etc/init.d/junk

This junk service can be used for testing.

Note that if you try to restart this service the custom restart function
is not executed.
Comment 3 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2011-01-06 06:35:32 UTC
IIRC this was a design choice made by Roy, but I cannot remember why.

WilliamH: can you ask him about it?
Comment 4 Roy Marples 2011-01-07 00:50:06 UTC
Robin is correct. It *was* a design choice which fixed a problem. What the problem was I don't recall though :S

Anyway, a restart is basically stop then start.

If you want any more functionality then it's not a restart is it? Maybe reload would be better in many instances. That's my view anyway :)
Comment 5 Christian Ruppert (idl0r) gentoo-dev 2011-01-07 10:26:44 UTC
Well.. I just did:
# Workaround for now, until openrc's restart has been fixed.
# openrc doesn't care about a restart() function in init scripts.
if [ "${RC_CMD}" = "restart" ]; then
        checkconfig || { eend 1; return 1; }
fi

That allows me to check the config first and in case of an error it won't restart until all problems has been fixed. That's why I wanted to use a init script internal restart() *if* available.
Comment 6 William Hubbs gentoo-dev 2011-01-07 15:43:07 UTC
(In reply to comment #5)
> Well.. I just did:
> # Workaround for now, until openrc's restart has been fixed.
> # openrc doesn't care about a restart() function in init scripts.
> if [ "${RC_CMD}" = "restart" ]; then
>         checkconfig || { eend 1; return 1; }
> fi
> That allows me to check the config first and in case of an error it won't
> restart until all problems has been fixed. That's why I wanted to use a init
> script internal restart() *if* available.

Shouldn't this go in the start() function and without checking rc_cmd so that your service doesn't start if there are problems?
Comment 7 Christian Ruppert (idl0r) gentoo-dev 2011-01-07 15:55:19 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Well.. I just did:
> > # Workaround for now, until openrc's restart has been fixed.
> > # openrc doesn't care about a restart() function in init scripts.
> > if [ "${RC_CMD}" = "restart" ]; then
> >         checkconfig || { eend 1; return 1; }
> > fi
> > That allows me to check the config first and in case of an error it won't
> > restart until all problems has been fixed. That's why I wanted to use a init
> > script internal restart() *if* available.
> 
> Shouldn't this go in the start() function and without checking rc_cmd so that
> your service doesn't start if there are problems?
> 

No.
E.g. you're running bind/named and have several domains, you also use cfengine or anything else which helps to maintain your server and you do a mistake, typo or so but cfengine would restart named anyway then it would stop named but not start due to a syntax error or so - so no named running anymore.
That's why it does the configcheck only *if* it's a restart.
Comment 8 SpanKY gentoo-dev 2011-01-07 17:56:27 UTC
so put the checkconfig call into stop

considering so many init.d scripts have adopted the chkconfig/checkconfig convention, i guess we should look into integrating that automatically
Comment 9 Christian Ruppert (idl0r) gentoo-dev 2011-01-07 18:07:17 UTC
(In reply to comment #8)
> so put the checkconfig call into stop
> 
> considering so many init.d scripts have adopted the chkconfig/checkconfig
> convention, i guess we should look into integrating that automatically
> 

It's in stop but with the RC_CMD check to ensure checkconfig will not be called when doing "stop" only, it's not necessary there.

http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/net-dns/bind/files/named.init-r9?view=markup
Comment 10 William Hubbs gentoo-dev 2011-01-07 19:35:07 UTC
(In reply to comment #8)
> considering so many init.d scripts have adopted the chkconfig/checkconfig
> convention, i guess we should look into integrating that automatically

Doesn't openrc support start/stop_pre functions? If those are supported and used both for stop/start and restarting services, couldn't script writers use them to handle any pre stop/start checks?
Comment 11 William Hubbs gentoo-dev 2011-01-07 20:18:28 UTC
I just verified on my system that openrc supports start_pre() and stop_pre() functions. If these return any value other than 0, the start or stop, respectively, is considered to fail, and the start() or stop() function is not called.

There are also start/stop_post() functions which are run after start/stop() if they are successfully run. The same thing applies here. If the post functions return anything other than 0, the stop/start are considered to be a failure.

I do not know if baselayout-1 supports this. If it doesn't, you may need to wait for openrc to go stable before you can use it.
Comment 12 Christian Ruppert (idl0r) gentoo-dev 2011-01-13 16:57:22 UTC
<snip>
05:32:40          @idl0r | WilliamH: what if you do stop a service? the sequence would be stop_pre, stop. stop_post too right?
05:33:14          @idl0r | WilliamH: i need a restart function because i must do some stuff *only* if it is a restart, not on stop
05:33:24          @idl0r | WilliamH: that's why i added the RC_CMD hack
</snip>
Comment 13 William Hubbs gentoo-dev 2011-01-25 21:48:06 UTC
Hi all,

The openrc code does have a way to handle this; however, we do not have it documented anywhere. I found the following comment in openrc source code:

        /* Export the command we're running.
           This is important as we stamp on the restart function now but
           some start/stop routines still need to behave differently if
           restarting. */

The whole reason the RC_CMD environment variable is exported is so that stop/start routines can behave differently when they are restarting, so Christian's test in comment #5 is considered the official way to handle this situation.
Comment 14 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2011-04-08 19:39:41 UTC
Moving to openrc documentation tracker bug
Comment 15 Sven Vermeulen 2011-07-03 14:19:02 UTC
Created attachment 278931 [details, diff]
Patch for hb-working-rcscripts (handbook chapter on init scripts)

This patch removes the script-defined "restart" functionality and describes that a restart is done by openrc through a regular "stop, start". If the user needs specific restart functionality, he needs to check the RC_CMD variable content instead.

In the section on defining user-specific functions, an <impo> paragraph is added to inform the user that "restart" cannot be overridden.
Comment 16 Sven Vermeulen (RETIRED) gentoo-dev 2011-08-12 19:35:47 UTC
Guys, the documentation has been updated to reflect this (cfr patch). If that is all that is necessary, then this bug can be marked as FIXED.
Comment 17 William Hubbs gentoo-dev 2011-08-12 20:46:12 UTC
I am fine closing this since the handbook has been updated.