xorp service file

[Dhionel Díaz]

El 09/09/15 a las 21:26, Felipe Sateler escribió:
> On 9 September 2015 at 20:46, Dhionel Díaz <ddiaz at cenditel.gob.ve> wrote:
>> El 09/09/15 a las 16:34, Felipe Sateler escribió:
>>> Hi Dhionel,
>>>
>>> On 9 September 2015 at 17:38, Dhionel Díaz <ddiaz at cenditel.gob.ve> wrote:
>>>> Hi everyone,
>>>>
>>>> Following the directions in https://wiki.debian.org/systemd/HowToHelp
>>>> I've attached a service file I've written for the xorp daemon, it has
>>>> been tested in one of our production routers.
>>>
>>> Great!
>>>
>>>>
>>>> I'll be awaiting your review.
>>>>
>>>> [Unit]
>>>> Description=eXtensible Open Router Platform
>>>> After=network.target remote-fs.target
>>>
>>> I don't think remote-fs.target is required here, as having
>>> DefaultDependencies=yes (the default) ensures all the basic
>>> filesystems are mounted.
>>
>> Ok, I see. I'll remove it.
>>
>>>
>>>>
>>>> [Service]
>>>> EnvironmentFile=-/etc/default/xorp
>>>> ExecStart=/usr/sbin/xorp_rtrmgr $DAEMON_OPTS
>>>
>>> It would be better to pass the option to daemonize and make the
>>> service Type=forking, otherwise systemd does not know when the service
>>> is successfully started.
>>>
>>> Maybe even pass -P and specify PIDFile= so that systemd knows for sure
>>> what the main process is.
>>
>> I'm confused. I thought the recommended approach was to make the service
>> run in foreground, in order to optimize the startup and monitoring.
>> Also, as far as I've verified no package depends on xorp; maybe that's
>> normal for a routing daemon.
>>
>> Another reason to implement the foreground behavior was to deactivate
>> the weekly restart that the logrotate script executes when a pid file is
>> found; that restart is not longer needed because with the proposed
>> configuration the log messages are sent to the journal via stderr.
> 
> Yes, rotating the log would be irrelevant if the logging is to the journal.
> 
>>
>> As an alternative, ¿do you think it would be appropriate to include a
>> sd_notify call in xorp_rtrmgr main process and then use Type=notify?
> 
> That would be even better. The problem with Type=simple (the default
> if no type is specified) is that systemd automatically assumes the
> service started up correctly. Which can be confusing if eg, the config
> is wrong: systemd would start the service, then the service would
> immediately fail. Much better is for systemd to know the service never
> started up correctly and let you know that.


OK, I'll prepare a patch. At first sight, it seems it would be a small one.


> 
> I suggested using Type=forking because it would be less work. The
> PIDFile is optional, and if xorp only spawns a single process, then it
> is not necessary at all.
> 

A reasonable simplification. Now I see the point.

>>
>>>
>>>> TimeoutStopSec=233
>>>
>>> Why this weird value?
>>
>> It's just a nice number to specify an almost four minute timeout. The
>> init script implements a two minute timeout, but in my experience xorp
>> is rather slow to stop and when the router is highly loaded that might
>> not be enough time. So I've incremented it using, given the oportunity,
>> a somewhat special number.
> 
> OK, this can be overriden by a user anyway if needed.
> 
>>
>>>
>>>> Restart=always
>>>
>>> I don't know if xorp can be told to exit, but this would cause it to
>>> be restarted in such a case.
>>>
>>
>> As far as I've verified, xorp cannot be told to exit via its command
>> interface (xorpsh); maybe that's also normal in a routing daemon.
>> Therefore, the rationale was to indicate that the service should always
>> be available after it's enabled and started.
> 
> Makes sense.
> 
>>
>>>>
>>>> [Install]
>>>> WantedBy=multi-user.target
>>>
>>> Looks good otherwise to me!
>>>
>>>
>>
>> Thanks for your time.
> 
> Thank you for your contribution.
> 
> 

Glad to help.

Regards,

-- 
Dhionel Díaz
Centro Nacional de Desarrollo e Investigación en Tecnologías Libres
Ministerio del Poder Popular para
Educación Universitaria, Ciencia y Tecnología

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://alioth-lists.debian.net/pipermail/pkg-systemd-maintainers/attachments/20150910/2f2ce1c8/attachment-0002.sig>