| Message ID | 1467533939-26606-1-git-send-email-nroach44@gmail.com |
|---|---|
| State | Accepted |
| Commit | 649cf99821d4418319aaf0c897ab43cab4dc902f |
| Headers | show |
Hi Nathaniel, all On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> wrote: > Use a template service file as all of the daemons use almost > identical arguments and generally appear the same to the init > system. > > We "Wants=" zebra as that's the daemon for interfacing to the > kernel, and it's not required for the other daemons to work > but it's probably going to be used in nearly all setups. > > /usr/bin/env is needed as systemd doesn't allow the instance > variable (%i) in the executable path. > > We don't enable these services by default as this would require > creating configuration and /etc/default files. (And is easily > achieved with an FS overlay) > > Signed-off-by: Nathaniel Roach <nroach44@gmail.com> > --- > Changes v2 -> v3: > - Remove invalid references to quagga.service (Arnout) > - Check if the binary is executable before trying to start it > (Arnout) > - Remove PID file arguments and options (Arnout) > - Add reload capability as the daemons do support it > > Changes v1 -> v2: > (As suggested by Arnout Vandecappelle) > - Completely remove shim and use /usr/bin/env instead > - Don't tell quagga to fork as systemd prefers it > - Add comment to .service file about /usr/bin/env > - Explain not enabling the service on build in patch > --- > package/quagga/quagga.mk | 2 ++ > package/quagga/quagga@.service | 20 ++++++++++++++++++++ > 2 files changed, 22 insertions(+) > create mode 100644 package/quagga/quagga@.service > > diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk > index 22e90ad..1bbc72d 100644 > --- a/package/quagga/quagga.mk > +++ b/package/quagga/quagga.mk > @@ -75,6 +75,8 @@ endif > define QUAGGA_INSTALL_INIT_SYSTEMD > $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ > $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf > + $(INSTALL) -D -m 644 package/quagga/quagga@.service \ > + $(TARGET_DIR)/usr/lib/systemd/system/quagga@.service > endef > > $(eval $(autotools-package)) > diff --git a/package/quagga/quagga@.service b/package/quagga/quagga@.service > new file mode 100644 > index 0000000..16acc30 > --- /dev/null > +++ b/package/quagga/quagga@.service > @@ -0,0 +1,20 @@ > +[Unit] > +Description=Quagga %i routing daemon > +ConditionFileIsExecutable=/usr/sbin/%i > +Wants=quagga@zebra.service Just a though, I read about why you added the Wants, maybe a comment would be nice here ? > + > +[Service] > +Type=simple Since you have removed the '--daemon' option, maybe you should revert this to "forking" ? > +EnvironmentFile=/etc/default/quagga-%i > +PrivateTmp=true > +# Systemd doesn't like having %i in the executable path. > +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf > +ExecReload=/bin/kill -HUP $MAINPID > +KillMode=mixed > +KillSignal=SIGINT If I understand correctly, what you want to do is send a SIGINT to the main processes and SIGKILL to the other ? From the services file I can find in major distro, they don't specify it so it's left to the default SIGTERM, why did you need to change that ? (Maybe you already explained in your previous mail and I missed, sorry if that's the case). > +Restart=on-failure > +RestartSec=1 Why ? > + > +[Install] > +WantedBy=multi-user.target > + > -- > 2.8.1 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hi Maxime, On 03/07/16 18:42, Maxime Hadjinlian wrote: > Hi Nathaniel, all > > On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> wrote: >> Use a template service file as all of the daemons use almost >> identical arguments and generally appear the same to the init >> system. >> >> We "Wants=" zebra as that's the daemon for interfacing to the >> kernel, and it's not required for the other daemons to work >> but it's probably going to be used in nearly all setups. >> >> /usr/bin/env is needed as systemd doesn't allow the instance >> variable (%i) in the executable path. >> >> We don't enable these services by default as this would require >> creating configuration and /etc/default files. (And is easily >> achieved with an FS overlay) >> >> Signed-off-by: Nathaniel Roach <nroach44@gmail.com> >> --- >> Changes v2 -> v3: >> - Remove invalid references to quagga.service (Arnout) >> - Check if the binary is executable before trying to start it >> (Arnout) >> - Remove PID file arguments and options (Arnout) >> - Add reload capability as the daemons do support it >> >> Changes v1 -> v2: >> (As suggested by Arnout Vandecappelle) >> - Completely remove shim and use /usr/bin/env instead >> - Don't tell quagga to fork as systemd prefers it >> - Add comment to .service file about /usr/bin/env >> - Explain not enabling the service on build in patch >> --- >> package/quagga/quagga.mk | 2 ++ >> package/quagga/quagga@.service | 20 ++++++++++++++++++++ >> 2 files changed, 22 insertions(+) >> create mode 100644 package/quagga/quagga@.service >> >> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >> index 22e90ad..1bbc72d 100644 >> --- a/package/quagga/quagga.mk >> +++ b/package/quagga/quagga.mk >> @@ -75,6 +75,8 @@ endif >> define QUAGGA_INSTALL_INIT_SYSTEMD >> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >> + $(INSTALL) -D -m 644 package/quagga/quagga@.service \ >> + $(TARGET_DIR)/usr/lib/systemd/system/quagga@.service >> endef >> >> $(eval $(autotools-package)) >> diff --git a/package/quagga/quagga@.service b/package/quagga/quagga@.service >> new file mode 100644 >> index 0000000..16acc30 >> --- /dev/null >> +++ b/package/quagga/quagga@.service >> @@ -0,0 +1,20 @@ >> +[Unit] >> +Description=Quagga %i routing daemon >> +ConditionFileIsExecutable=/usr/sbin/%i >> +Wants=quagga@zebra.service > Just a though, I read about why you added the Wants, maybe a comment > would be nice here ? >> + >> +[Service] >> +Type=simple > Since you have removed the '--daemon' option, maybe you should revert > this to "forking" ? I might have it backwards, but it would need "forking" if --daemon was specified, otherwise "systemctl start quagga@ospfd" would hang. >> +EnvironmentFile=/etc/default/quagga-%i >> +PrivateTmp=true >> +# Systemd doesn't like having %i in the executable path. >> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >> +ExecReload=/bin/kill -HUP $MAINPID >> +KillMode=mixed >> +KillSignal=SIGINT > If I understand correctly, what you want to do is send a SIGINT to the > main processes and SIGKILL to the other ? From the services file I can > find in major distro, they don't specify it so it's left to the > default SIGTERM, why did you need to change that ? (Maybe you already > explained in your previous mail and I missed, sorry if that's the > case). KillMode=Mixed is an artefact left over from the OpenVPN .service file I used as a reference. I (again) misread the systemd docs and left it in as it seemed more appropriate than the default if any child processes are started. This can be removed on commit, or I'll fix it if there's any other things that need to be changed. >> +Restart=on-failure >> +RestartSec=1 > Why ? I've had ospfd crash during configuration and general usage, this is undesirable as it's responsible for keeping a large amount of routes in my router's tables. >> + >> +[Install] >> +WantedBy=multi-user.target >> + >> -- >> 2.8.1 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@busybox.net >> http://lists.busybox.net/mailman/listinfo/buildroot
Hi Nathaniel, On Sun, Jul 3, 2016 at 12:54 PM, Nathaniel Roach <nroach44@gmail.com> wrote: > Hi Maxime, > > > > On 03/07/16 18:42, Maxime Hadjinlian wrote: >> >> Hi Nathaniel, all >> >> On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> >> wrote: >>> >>> Use a template service file as all of the daemons use almost >>> identical arguments and generally appear the same to the init >>> system. >>> >>> We "Wants=" zebra as that's the daemon for interfacing to the >>> kernel, and it's not required for the other daemons to work >>> but it's probably going to be used in nearly all setups. >>> >>> /usr/bin/env is needed as systemd doesn't allow the instance >>> variable (%i) in the executable path. >>> >>> We don't enable these services by default as this would require >>> creating configuration and /etc/default files. (And is easily >>> achieved with an FS overlay) >>> >>> Signed-off-by: Nathaniel Roach <nroach44@gmail.com> >>> --- >>> Changes v2 -> v3: >>> - Remove invalid references to quagga.service (Arnout) >>> - Check if the binary is executable before trying to start it >>> (Arnout) >>> - Remove PID file arguments and options (Arnout) >>> - Add reload capability as the daemons do support it >>> >>> Changes v1 -> v2: >>> (As suggested by Arnout Vandecappelle) >>> - Completely remove shim and use /usr/bin/env instead >>> - Don't tell quagga to fork as systemd prefers it >>> - Add comment to .service file about /usr/bin/env >>> - Explain not enabling the service on build in patch >>> --- >>> package/quagga/quagga.mk | 2 ++ >>> package/quagga/quagga@.service | 20 ++++++++++++++++++++ >>> 2 files changed, 22 insertions(+) >>> create mode 100644 package/quagga/quagga@.service >>> >>> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >>> index 22e90ad..1bbc72d 100644 >>> --- a/package/quagga/quagga.mk >>> +++ b/package/quagga/quagga.mk >>> @@ -75,6 +75,8 @@ endif >>> define QUAGGA_INSTALL_INIT_SYSTEMD >>> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >>> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >>> + $(INSTALL) -D -m 644 package/quagga/quagga@.service \ >>> + $(TARGET_DIR)/usr/lib/systemd/system/quagga@.service >>> endef >>> >>> $(eval $(autotools-package)) >>> diff --git a/package/quagga/quagga@.service >>> b/package/quagga/quagga@.service >>> new file mode 100644 >>> index 0000000..16acc30 >>> --- /dev/null >>> +++ b/package/quagga/quagga@.service >>> @@ -0,0 +1,20 @@ >>> +[Unit] >>> +Description=Quagga %i routing daemon >>> +ConditionFileIsExecutable=/usr/sbin/%i >>> +Wants=quagga@zebra.service >> >> Just a though, I read about why you added the Wants, maybe a comment >> would be nice here ? >>> >>> + >>> +[Service] >>> +Type=simple >> >> Since you have removed the '--daemon' option, maybe you should revert >> this to "forking" ? > > I might have it backwards, but it would need "forking" if --daemon was > specified, otherwise "systemctl start quagga@ospfd" would hang. You are absolutely right, I am the one that got confused. I missed the '-d' on the ExecStart line in the services in the quagga sources. >>> >>> +EnvironmentFile=/etc/default/quagga-%i >>> +PrivateTmp=true >>> +# Systemd doesn't like having %i in the executable path. >>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>> +ExecReload=/bin/kill -HUP $MAINPID >>> +KillMode=mixed >>> +KillSignal=SIGINT >> >> If I understand correctly, what you want to do is send a SIGINT to the >> main processes and SIGKILL to the other ? From the services file I can >> find in major distro, they don't specify it so it's left to the >> default SIGTERM, why did you need to change that ? (Maybe you already >> explained in your previous mail and I missed, sorry if that's the >> case). > > KillMode=Mixed is an artefact left over from the OpenVPN .service file I > used as a reference. I (again) misread the systemd docs and left it in as it > seemed more appropriate than the default if any child processes are started. > This can be removed on commit, or I'll fix it if there's any other things > that need to be changed. Thanks for the explanation. >>> >>> +Restart=on-failure >>> +RestartSec=1 >> >> Why ? > > I've had ospfd crash during configuration and general usage, this is > undesirable as it's responsible for keeping a large amount of routes in my > router's tables. I was thinking about the RestartSec delay, I agree on the "on-failure". > >>> + >>> +[Install] >>> +WantedBy=multi-user.target >>> + >>> -- >>> 2.8.1 >>> >>> _______________________________________________ >>> buildroot mailing list >>> buildroot@busybox.net >>> http://lists.busybox.net/mailman/listinfo/buildroot > >
On 03/07/16 20:59, Maxime Hadjinlian wrote: > Hi Nathaniel, > > On Sun, Jul 3, 2016 at 12:54 PM, Nathaniel Roach <nroach44@gmail.com> wrote: >> Hi Maxime, >> >> >> >> On 03/07/16 18:42, Maxime Hadjinlian wrote: >>> Hi Nathaniel, all >>> >>> On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> >>> wrote: >>>> Use a template service file as all of the daemons use almost >>>> identical arguments and generally appear the same to the init >>>> system. >>>> >>>> We "Wants=" zebra as that's the daemon for interfacing to the >>>> kernel, and it's not required for the other daemons to work >>>> but it's probably going to be used in nearly all setups. >>>> >>>> /usr/bin/env is needed as systemd doesn't allow the instance >>>> variable (%i) in the executable path. >>>> >>>> We don't enable these services by default as this would require >>>> creating configuration and /etc/default files. (And is easily >>>> achieved with an FS overlay) >>>> >>>> Signed-off-by: Nathaniel Roach <nroach44@gmail.com> >>>> --- >>>> Changes v2 -> v3: >>>> - Remove invalid references to quagga.service (Arnout) >>>> - Check if the binary is executable before trying to start it >>>> (Arnout) >>>> - Remove PID file arguments and options (Arnout) >>>> - Add reload capability as the daemons do support it >>>> >>>> Changes v1 -> v2: >>>> (As suggested by Arnout Vandecappelle) >>>> - Completely remove shim and use /usr/bin/env instead >>>> - Don't tell quagga to fork as systemd prefers it >>>> - Add comment to .service file about /usr/bin/env >>>> - Explain not enabling the service on build in patch >>>> --- >>>> package/quagga/quagga.mk | 2 ++ >>>> package/quagga/quagga@.service | 20 ++++++++++++++++++++ >>>> 2 files changed, 22 insertions(+) >>>> create mode 100644 package/quagga/quagga@.service >>>> >>>> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >>>> index 22e90ad..1bbc72d 100644 >>>> --- a/package/quagga/quagga.mk >>>> +++ b/package/quagga/quagga.mk >>>> @@ -75,6 +75,8 @@ endif >>>> define QUAGGA_INSTALL_INIT_SYSTEMD >>>> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >>>> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >>>> + $(INSTALL) -D -m 644 package/quagga/quagga@.service \ >>>> + $(TARGET_DIR)/usr/lib/systemd/system/quagga@.service >>>> endef >>>> >>>> $(eval $(autotools-package)) >>>> diff --git a/package/quagga/quagga@.service >>>> b/package/quagga/quagga@.service >>>> new file mode 100644 >>>> index 0000000..16acc30 >>>> --- /dev/null >>>> +++ b/package/quagga/quagga@.service >>>> @@ -0,0 +1,20 @@ >>>> +[Unit] >>>> +Description=Quagga %i routing daemon >>>> +ConditionFileIsExecutable=/usr/sbin/%i >>>> +Wants=quagga@zebra.service >>> Just a though, I read about why you added the Wants, maybe a comment >>> would be nice here ? >>>> + >>>> +[Service] >>>> +Type=simple >>> Since you have removed the '--daemon' option, maybe you should revert >>> this to "forking" ? >> I might have it backwards, but it would need "forking" if --daemon was >> specified, otherwise "systemctl start quagga@ospfd" would hang. > You are absolutely right, I am the one that got confused. I missed the > '-d' on the ExecStart line in the services in the quagga sources. >>>> +EnvironmentFile=/etc/default/quagga-%i >>>> +PrivateTmp=true >>>> +# Systemd doesn't like having %i in the executable path. >>>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>>> +ExecReload=/bin/kill -HUP $MAINPID >>>> +KillMode=mixed >>>> +KillSignal=SIGINT >>> If I understand correctly, what you want to do is send a SIGINT to the >>> main processes and SIGKILL to the other ? From the services file I can >>> find in major distro, they don't specify it so it's left to the >>> default SIGTERM, why did you need to change that ? (Maybe you already >>> explained in your previous mail and I missed, sorry if that's the >>> case). >> KillMode=Mixed is an artefact left over from the OpenVPN .service file I >> used as a reference. I (again) misread the systemd docs and left it in as it >> seemed more appropriate than the default if any child processes are started. >> This can be removed on commit, or I'll fix it if there's any other things >> that need to be changed. > Thanks for the explanation. >>>> +Restart=on-failure >>>> +RestartSec=1 >>> Why ? >> I've had ospfd crash during configuration and general usage, this is >> undesirable as it's responsible for keeping a large amount of routes in my >> router's tables. > I was thinking about the RestartSec delay, I agree on the "on-failure". Oh, derp. I didn't pick 1 second for any reason other than it's quickest. If there's a good argument for longer it can be set longer. >>>> + >>>> +[Install] >>>> +WantedBy=multi-user.target >>>> + >>>> -- >>>> 2.8.1 >>>> >>>> _______________________________________________ >>>> buildroot mailing list >>>> buildroot@busybox.net >>>> http://lists.busybox.net/mailman/listinfo/buildroot >>
Oh, and it's not 0 such that it won't restart and hog resources quite as badly if it's instantly dieing. On 03/07/16 20:59, Maxime Hadjinlian wrote: > Hi Nathaniel, > > On Sun, Jul 3, 2016 at 12:54 PM, Nathaniel Roach <nroach44@gmail.com> wrote: >> Hi Maxime, >> >> >> >> On 03/07/16 18:42, Maxime Hadjinlian wrote: >>> Hi Nathaniel, all >>> >>> On Sun, Jul 3, 2016 at 10:18 AM, Nathaniel Roach <nroach44@gmail.com> >>> wrote: >>>> Use a template service file as all of the daemons use almost >>>> identical arguments and generally appear the same to the init >>>> system. >>>> >>>> We "Wants=" zebra as that's the daemon for interfacing to the >>>> kernel, and it's not required for the other daemons to work >>>> but it's probably going to be used in nearly all setups. >>>> >>>> /usr/bin/env is needed as systemd doesn't allow the instance >>>> variable (%i) in the executable path. >>>> >>>> We don't enable these services by default as this would require >>>> creating configuration and /etc/default files. (And is easily >>>> achieved with an FS overlay) >>>> >>>> Signed-off-by: Nathaniel Roach <nroach44@gmail.com> >>>> --- >>>> Changes v2 -> v3: >>>> - Remove invalid references to quagga.service (Arnout) >>>> - Check if the binary is executable before trying to start it >>>> (Arnout) >>>> - Remove PID file arguments and options (Arnout) >>>> - Add reload capability as the daemons do support it >>>> >>>> Changes v1 -> v2: >>>> (As suggested by Arnout Vandecappelle) >>>> - Completely remove shim and use /usr/bin/env instead >>>> - Don't tell quagga to fork as systemd prefers it >>>> - Add comment to .service file about /usr/bin/env >>>> - Explain not enabling the service on build in patch >>>> --- >>>> package/quagga/quagga.mk | 2 ++ >>>> package/quagga/quagga@.service | 20 ++++++++++++++++++++ >>>> 2 files changed, 22 insertions(+) >>>> create mode 100644 package/quagga/quagga@.service >>>> >>>> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >>>> index 22e90ad..1bbc72d 100644 >>>> --- a/package/quagga/quagga.mk >>>> +++ b/package/quagga/quagga.mk >>>> @@ -75,6 +75,8 @@ endif >>>> define QUAGGA_INSTALL_INIT_SYSTEMD >>>> $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >>>> $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >>>> + $(INSTALL) -D -m 644 package/quagga/quagga@.service \ >>>> + $(TARGET_DIR)/usr/lib/systemd/system/quagga@.service >>>> endef >>>> >>>> $(eval $(autotools-package)) >>>> diff --git a/package/quagga/quagga@.service >>>> b/package/quagga/quagga@.service >>>> new file mode 100644 >>>> index 0000000..16acc30 >>>> --- /dev/null >>>> +++ b/package/quagga/quagga@.service >>>> @@ -0,0 +1,20 @@ >>>> +[Unit] >>>> +Description=Quagga %i routing daemon >>>> +ConditionFileIsExecutable=/usr/sbin/%i >>>> +Wants=quagga@zebra.service >>> Just a though, I read about why you added the Wants, maybe a comment >>> would be nice here ? >>>> + >>>> +[Service] >>>> +Type=simple >>> Since you have removed the '--daemon' option, maybe you should revert >>> this to "forking" ? >> I might have it backwards, but it would need "forking" if --daemon was >> specified, otherwise "systemctl start quagga@ospfd" would hang. > You are absolutely right, I am the one that got confused. I missed the > '-d' on the ExecStart line in the services in the quagga sources. >>>> +EnvironmentFile=/etc/default/quagga-%i >>>> +PrivateTmp=true >>>> +# Systemd doesn't like having %i in the executable path. >>>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>>> +ExecReload=/bin/kill -HUP $MAINPID >>>> +KillMode=mixed >>>> +KillSignal=SIGINT >>> If I understand correctly, what you want to do is send a SIGINT to the >>> main processes and SIGKILL to the other ? From the services file I can >>> find in major distro, they don't specify it so it's left to the >>> default SIGTERM, why did you need to change that ? (Maybe you already >>> explained in your previous mail and I missed, sorry if that's the >>> case). >> KillMode=Mixed is an artefact left over from the OpenVPN .service file I >> used as a reference. I (again) misread the systemd docs and left it in as it >> seemed more appropriate than the default if any child processes are started. >> This can be removed on commit, or I'll fix it if there's any other things >> that need to be changed. > Thanks for the explanation. >>>> +Restart=on-failure >>>> +RestartSec=1 >>> Why ? >> I've had ospfd crash during configuration and general usage, this is >> undesirable as it's responsible for keeping a large amount of routes in my >> router's tables. > I was thinking about the RestartSec delay, I agree on the "on-failure". >>>> + >>>> +[Install] >>>> +WantedBy=multi-user.target >>>> + >>>> -- >>>> 2.8.1 >>>> >>>> _______________________________________________ >>>> buildroot mailing list >>>> buildroot@busybox.net >>>> http://lists.busybox.net/mailman/listinfo/buildroot >>
On Sun, Jul 3, 2016 at 3:06 PM, Nathaniel Roach <nroach44@gmail.com> wrote: > Oh, and it's not 0 such that it won't restart and hog resources quite as > badly if it's instantly dieing. > The default is 100ms, do you think that's still too fast ? Usually it's enough but if you encounter problem and 1s is the shortest it can do, then 1s it is. Other than that: Reviewed-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> [...] >>>>> +EnvironmentFile=/etc/default/quagga-%i >>>>> +PrivateTmp=true >>>>> +# Systemd doesn't like having %i in the executable path. >>>>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>>>> +ExecReload=/bin/kill -HUP $MAINPID >>>>> +KillMode=mixed >>>>> +KillSignal=SIGINT >>>> >>>> If I understand correctly, what you want to do is send a SIGINT to the >>>> main processes and SIGKILL to the other ? From the services file I can >>>> find in major distro, they don't specify it so it's left to the >>>> default SIGTERM, why did you need to change that ? (Maybe you already >>>> explained in your previous mail and I missed, sorry if that's the >>>> case). >>> >>> KillMode=Mixed is an artefact left over from the OpenVPN .service file I >>> used as a reference. I (again) misread the systemd docs and left it in as >>> it >>> seemed more appropriate than the default if any child processes are >>> started. >>> This can be removed on commit, or I'll fix it if there's any other things >>> that need to be changed. [...]
Ah, another case of me misinterpreting defaults. This can also be removed if it's preferred to leave it as default. On 3 July 2016 9:09:39 PM AWST, Maxime Hadjinlian <maxime.hadjinlian@gmail.com> wrote: >On Sun, Jul 3, 2016 at 3:06 PM, Nathaniel Roach <nroach44@gmail.com> >wrote: >> Oh, and it's not 0 such that it won't restart and hog resources quite >as >> badly if it's instantly dieing. >> >The default is 100ms, do you think that's still too fast ? Usually >it's enough but if you encounter problem and 1s is the shortest it can >do, then 1s it is. > >Other than that: >Reviewed-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com> >[...] >>>>>> +EnvironmentFile=/etc/default/quagga-%i >>>>>> +PrivateTmp=true >>>>>> +# Systemd doesn't like having %i in the executable path. >>>>>> +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf >>>>>> +ExecReload=/bin/kill -HUP $MAINPID >>>>>> +KillMode=mixed >>>>>> +KillSignal=SIGINT >>>>> >>>>> If I understand correctly, what you want to do is send a SIGINT to >the >>>>> main processes and SIGKILL to the other ? From the services file I >can >>>>> find in major distro, they don't specify it so it's left to the >>>>> default SIGTERM, why did you need to change that ? (Maybe you >already >>>>> explained in your previous mail and I missed, sorry if that's the >>>>> case). >>>> >>>> KillMode=Mixed is an artefact left over from the OpenVPN .service >file I >>>> used as a reference. I (again) misread the systemd docs and left it >in as >>>> it >>>> seemed more appropriate than the default if any child processes are >>>> started. >>>> This can be removed on commit, or I'll fix it if there's any other >things >>>> that need to be changed. >[...]
>>>>> "Nathaniel" == Nathaniel Roach <nroach44@gmail.com> writes: > Use a template service file as all of the daemons use almost > identical arguments and generally appear the same to the init > system. > We "Wants=" zebra as that's the daemon for interfacing to the > kernel, and it's not required for the other daemons to work > but it's probably going to be used in nearly all setups. > /usr/bin/env is needed as systemd doesn't allow the instance > variable (%i) in the executable path. > We don't enable these services by default as this would require > creating configuration and /etc/default files. (And is easily > achieved with an FS overlay) > Signed-off-by: Nathaniel Roach <nroach44@gmail.com> > --- > Changes v2 -> v3: > - Remove invalid references to quagga.service (Arnout) > - Check if the binary is executable before trying to start it > (Arnout) > - Remove PID file arguments and options (Arnout) > - Add reload capability as the daemons do support it Committed after removing killmode/killsignal/restartsec as suggested by Maxime, thanks.
diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk index 22e90ad..1bbc72d 100644 --- a/package/quagga/quagga.mk +++ b/package/quagga/quagga.mk @@ -75,6 +75,8 @@ endif define QUAGGA_INSTALL_INIT_SYSTEMD $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf + $(INSTALL) -D -m 644 package/quagga/quagga@.service \ + $(TARGET_DIR)/usr/lib/systemd/system/quagga@.service endef $(eval $(autotools-package)) diff --git a/package/quagga/quagga@.service b/package/quagga/quagga@.service new file mode 100644 index 0000000..16acc30 --- /dev/null +++ b/package/quagga/quagga@.service @@ -0,0 +1,20 @@ +[Unit] +Description=Quagga %i routing daemon +ConditionFileIsExecutable=/usr/sbin/%i +Wants=quagga@zebra.service + +[Service] +Type=simple +EnvironmentFile=/etc/default/quagga-%i +PrivateTmp=true +# Systemd doesn't like having %i in the executable path. +ExecStart=/usr/bin/env /usr/sbin/%i $OPTS -f /etc/quagga/%i.conf +ExecReload=/bin/kill -HUP $MAINPID +KillMode=mixed +KillSignal=SIGINT +Restart=on-failure +RestartSec=1 + +[Install] +WantedBy=multi-user.target +
Use a template service file as all of the daemons use almost identical arguments and generally appear the same to the init system. We "Wants=" zebra as that's the daemon for interfacing to the kernel, and it's not required for the other daemons to work but it's probably going to be used in nearly all setups. /usr/bin/env is needed as systemd doesn't allow the instance variable (%i) in the executable path. We don't enable these services by default as this would require creating configuration and /etc/default files. (And is easily achieved with an FS overlay) Signed-off-by: Nathaniel Roach <nroach44@gmail.com> --- Changes v2 -> v3: - Remove invalid references to quagga.service (Arnout) - Check if the binary is executable before trying to start it (Arnout) - Remove PID file arguments and options (Arnout) - Add reload capability as the daemons do support it Changes v1 -> v2: (As suggested by Arnout Vandecappelle) - Completely remove shim and use /usr/bin/env instead - Don't tell quagga to fork as systemd prefers it - Add comment to .service file about /usr/bin/env - Explain not enabling the service on build in patch --- package/quagga/quagga.mk | 2 ++ package/quagga/quagga@.service | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 package/quagga/quagga@.service