[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-tech
Subject:    Re: nsd rc script check returns strange exit code when nsd not running.
From:       Alexander Hall <alexander () beard ! se>
Date:       2014-06-26 4:31:11
Message-ID: 53ABA20F.90407 () beard ! se
[Download RAW message or body]

On 06/26/14 06:29, Alexander Hall wrote:
> On 06/25/14 14:06, Stuart Henderson wrote:
>> On 2014/06/25 13:04, Stuart Henderson wrote:
>>> On 2014/06/24 20:56, ben@tilderoot.com wrote:
>>>> The nsd rc script returns an exit code other than 1 when nsd is
>>>> not running. A problem arises when using a configuration management
>>>> (CM) system such as ansible to control system services. With this
>>>> non-standard exit code, the CM system cannot determine if the service
>>>> is running or not. The nsd rc script runs 'nsd-control status', which
>>>> documentation states will exit 3 when nsd is not running. I'd argue
>>>> that
>>>> the rc script should catch that, and instead exit 1.
>>>
>>> If it was just for a CM system I'd argue that it should probably be
>>> more flexible, but it also goes against the documentation in rc.subr(8),
>>> so I think this should be fixed.
>>>
>>> I'll commit this soon unless there are any objections.
>>>
>>>> Index: nsd
>>>> ===================================================================
>>>> RCS file: /cvs/src/etc/rc.d/nsd,v
>>>> retrieving revision 1.7
>>>> diff -u -p -u -r1.7 nsd
>>>> --- nsd 7 May 2014 02:46:05 -0000       1.7
>>>> +++ nsd 25 Jun 2014 03:53:50 -0000
>>>> @ -25,6 +25,12 @@ rc_start() {
>>>>
>>>>   rc_check() {
>>>>          ${daemon} ${daemon_flags} status
>>>> +       ret=$?
>>>> +       if [[ $ret != 0 ]]; then
>>>> +               return 1
>>>> +       else
>>>> +               return 0
>>>> +       fi
>>>>   }
>>>>
>>>>   rc_reload() {
>>>>
>>>
>>
>> Actually, no need for the if; this is simpler:
>>
>> Index: nsd
>> ===================================================================
>> RCS file: /cvs/src/etc/rc.d/nsd,v
>> retrieving revision 1.7
>> diff -u -p -r1.7 nsd
>> --- nsd    7 May 2014 02:46:05 -0000    1.7
>> +++ nsd    25 Jun 2014 12:06:29 -0000
>> @@ -24,7 +24,7 @@ rc_start() {
>>   }
>>
>>   rc_check() {
>> -    ${daemon} ${daemon_flags} status
>> +    ${daemon} ${daemon_flags} status || exit 1

Also, here you exit, not return. While it might not matter here, there 
is a big difference.

Thus, if you go with this, please 'return 1' rather than 'exit 1'.

/Alexander

>>   }
>>
>>   rc_reload() {
>>
>>
>
> Or,
>
>      ! ! ${daemon} ${daemon_flags} status
>
> but it might be less obvious.
>
> /Alexander
>

[prev in list] [next in list] [prev in thread] [next in thread]