Menu
▾
▴
openvpn-devel
|
From: Christian H. <li...@ew...> - 2016-11-29 15:27:57
|
From: Christian Hesse <ma...@ew...> Notify systemd service manager when our initialization sequence completed. This helps ordering services as dependencies can rely on vpn being available. Signed-off-by: Christian Hesse <ma...@ew...> --- distro/systemd/openvpn-client@.service | 1 + distro/systemd/openvpn-server@.service | 1 + src/openvpn/init.c | 6 ++++++ src/openvpn/init.h | 4 ++++ 4 files changed, 12 insertions(+) diff --git a/distro/systemd/openvpn-client@.service b/distro/systemd/openvpn-client@.service index 18b84dd..f64a239 100644 --- a/distro/systemd/openvpn-client@.service +++ b/distro/systemd/openvpn-client@.service @@ -7,6 +7,7 @@ Documentation=https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] +Type=notify PrivateTmp=true RuntimeDirectory=openvpn-client RuntimeDirectoryMode=0710 diff --git a/distro/systemd/openvpn-server@.service b/distro/systemd/openvpn-server@.service index a2b7b52..890e6a9 100644 --- a/distro/systemd/openvpn-server@.service +++ b/distro/systemd/openvpn-server@.service @@ -7,6 +7,7 @@ Documentation=https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] +Type=notify PrivateTmp=true RuntimeDirectory=openvpn-server RuntimeDirectoryMode=0710 diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 2ccbab2..551e579 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1251,10 +1251,16 @@ initialization_sequence_completed (struct context *c, const unsigned int flags) show_adapters (M_INFO|M_NOPREFIX); msg (M_INFO, "%s With Errors ( see http://openvpn.net/faq.html#dhcpclientserv )", message); #else +#ifdef ENABLE_SYSTEMD + sd_notifyf(0, "STATUS=Failed to start up: %s With Errors\nERRNO=1", message); +#endif /* HAVE_SYSTEMD_SD_DAEMON_H */ msg (M_INFO, "%s With Errors", message); #endif } else +#ifdef ENABLE_SYSTEMD + sd_notifyf(0, "READY=1\nSTATUS=%s\nMAINPID=%lu", message, (unsigned long) getpid()); +#endif msg (M_INFO, "%s", message); /* Flag that we initialized */ diff --git a/src/openvpn/init.h b/src/openvpn/init.h index 524bc64..0518b06 100644 --- a/src/openvpn/init.h +++ b/src/openvpn/init.h @@ -27,6 +27,10 @@ #include "openvpn.h" +#ifdef ENABLE_SYSTEMD +#include <systemd/sd-daemon.h> +#endif + /* * Baseline maximum number of events * to wait for. -- 2.10.2 |
|
From: David S. <op...@sf...> - 2016-11-30 01:45:38
Attachments:
signature.asc
|
On 29/11/16 16:27, Christian Hesse wrote: > From: Christian Hesse <ma...@ew...> > > Notify systemd service manager when our initialization sequence > completed. This helps ordering services as dependencies can rely on vpn > being available. Funny detail is that I have a somewhat similar patch in a local git tree, awaiting proper testing ... I postponed it as this is not something we will pull into v2.4. We're going to release 2.4_rc1 this week, and that is too late for more intrusive changes (even though the changeset itself is small, the code changes makes OpenVPN behave somewhat different when managed by systemd). Just a question, as it is good to see more people looking into these code paths ... I was considering to extend my approach to update STATUS= a bit more frequently. On the client side, I thought it would be good if the status line had "Resolving %s", "Connecting to %s", "Successful connection to %s" or "Failed to connect to %s". On the server side I was pondering on a "Successfully started, %i clients connected". What do you think about that? Does the sd_notify() API support more frequent updates? Also when using Type=notify ... does systemd expect the OpenVPN process to fork into the background or run in the foreground as now? -- kind regards, David Sommerseth OpenVPN Technologies, Inc > --- > distro/systemd/openvpn-client@.service | 1 + > distro/systemd/openvpn-server@.service | 1 + > src/openvpn/init.c | 6 ++++++ > src/openvpn/init.h | 4 ++++ > 4 files changed, 12 insertions(+) > > diff --git a/distro/systemd/openvpn-client@.service b/distro/systemd/openvpn-client@.service > index 18b84dd..f64a239 100644 > --- a/distro/systemd/openvpn-client@.service > +++ b/distro/systemd/openvpn-client@.service > @@ -7,6 +7,7 @@ Documentation=https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage > Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO > > [Service] > +Type=notify > PrivateTmp=true > RuntimeDirectory=openvpn-client > RuntimeDirectoryMode=0710 > diff --git a/distro/systemd/openvpn-server@.service b/distro/systemd/openvpn-server@.service > index a2b7b52..890e6a9 100644 > --- a/distro/systemd/openvpn-server@.service > +++ b/distro/systemd/openvpn-server@.service > @@ -7,6 +7,7 @@ Documentation=https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage > Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO > > [Service] > +Type=notify > PrivateTmp=true > RuntimeDirectory=openvpn-server > RuntimeDirectoryMode=0710 > diff --git a/src/openvpn/init.c b/src/openvpn/init.c > index 2ccbab2..551e579 100644 > --- a/src/openvpn/init.c > +++ b/src/openvpn/init.c > @@ -1251,10 +1251,16 @@ initialization_sequence_completed (struct context *c, const unsigned int flags) > show_adapters (M_INFO|M_NOPREFIX); > msg (M_INFO, "%s With Errors ( see http://openvpn.net/faq.html#dhcpclientserv )", message); > #else > +#ifdef ENABLE_SYSTEMD > + sd_notifyf(0, "STATUS=Failed to start up: %s With Errors\nERRNO=1", message); > +#endif /* HAVE_SYSTEMD_SD_DAEMON_H */ > msg (M_INFO, "%s With Errors", message); > #endif > } > else > +#ifdef ENABLE_SYSTEMD > + sd_notifyf(0, "READY=1\nSTATUS=%s\nMAINPID=%lu", message, (unsigned long) getpid()); > +#endif > msg (M_INFO, "%s", message); > > /* Flag that we initialized */ > diff --git a/src/openvpn/init.h b/src/openvpn/init.h > index 524bc64..0518b06 100644 > --- a/src/openvpn/init.h > +++ b/src/openvpn/init.h > @@ -27,6 +27,10 @@ > > #include "openvpn.h" > > +#ifdef ENABLE_SYSTEMD > +#include <systemd/sd-daemon.h> > +#endif > + > /* > * Baseline maximum number of events > * to wait for. > |
|
From: Christian H. <li...@ew...> - 2016-11-30 08:12:37
|
CCing Elias Probst as he is listed as contributor for last commit changing
systemd units (8b42c197626430118ed126c1b8256ba5ae1f699a, "systemd: Improve
the systemd unit files").
Anybody else involved with systemd units?
David Sommerseth <op...@sf...> on Wed, 2016/11/30 02:45:
> On 29/11/16 16:27, Christian Hesse wrote:
> > From: Christian Hesse <ma...@ew...>
> >
> > Notify systemd service manager when our initialization sequence
> > completed. This helps ordering services as dependencies can rely on vpn
> > being available.
>
> Funny detail is that I have a somewhat similar patch in a local git
> tree, awaiting proper testing ... I postponed it as this is not
> something we will pull into v2.4. We're going to release 2.4_rc1 this
> week, and that is too late for more intrusive changes (even though the
> changeset itself is small, the code changes makes OpenVPN behave
> somewhat different when managed by systemd).
We should rethink this... I am pretty sure I will not ship the code as-is
with Arch Linux. More below.
Tested by me, works pretty well. ;)
> Just a question, as it is good to see more people looking into these
> code paths ... I was considering to extend my approach to update STATUS=
> a bit more frequently. On the client side, I thought it would be good
> if the status line had "Resolving %s", "Connecting to %s", "Successful
> connection to %s" or "Failed to connect to %s". On the server side I
> was pondering on a "Successfully started, %i clients connected". What
> do you think about that? Does the sd_notify() API support more frequent
> updates?
Interesting idea... I will have a look.
> Also when using Type=notify ... does systemd expect the OpenVPN process
> to fork into the background or run in the foreground as now?
Ok, lets go into detail. We can use three different settings: Type=simple,
Type=forking and Type=notify.
* We used Type=forking for a long time. That is fine: systemd reports success
when the process forks off first time. That is when openvpn successfully
completed initialization sequence.
* The current systemd unit use Type=simple (which is implicit). systemd
reports success as soon as the process is executed, it does not wait for
anything. So startup can look like that: systemd starts openvpn process ->
unit is in state 'started' -> openvpn bails out with an error
before the initialization sequence completed -> systemd unit is in state
'failed' now. The problem is that it was in state 'started' intermittently:
Manual systemctl (starting service from command line) reports success, other
services depending on openvpn are started while dependency failed
later, ... This is just broken.
* My patch introduces Type=notify. The (main) process must not fork, so most
things work like simple, except that systemd does not report success on
process execution, but waits for the sd_notify() call. We do not have
intermittent state 'success' and everything works as expected.
I will not package the code as-is with our Arch Linux package. Either I
revert back to Type=forking or apply the patch for Type=notify.
So I still vote to apply this as soon as possible.
--
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
|
|
From: Christian H. <li...@ew...> - 2016-12-01 12:48:20
|
Christian Hesse <li...@ew...> on Wed, 2016/11/30 09:12:
> Ok, lets go into detail. We can use three different settings: Type=simple,
> Type=forking and Type=notify.
>
> * We used Type=forking for a long time. That is fine: systemd reports
> success when the process forks off first time. That is when openvpn
> successfully completed initialization sequence.
>
> * The current systemd unit use Type=simple (which is implicit). systemd
> reports success as soon as the process is executed, it does not wait for
> anything. So startup can look like that: systemd starts openvpn process ->
> unit is in state 'started' -> openvpn bails out with an error
> before the initialization sequence completed -> systemd unit is in state
> 'failed' now. The problem is that it was in state 'started'
> intermittently: Manual systemctl (starting service from command line)
> reports success, other services depending on openvpn are started while
> dependency failed later, ... This is just broken.
>
> * My patch introduces Type=notify. The (main) process must not fork, so most
> things work like simple, except that systemd does not report success on
> process execution, but waits for the sd_notify() call. We do not have
> intermittent state 'success' and everything works as expected.
>
> I will not package the code as-is with our Arch Linux package. Either I
> revert back to Type=forking or apply the patch for Type=notify.
>
> So I still vote to apply this as soon as possible.
I prepared an example:
root@leda ~ # systemctl start openvpn-client@lugor
root@leda ~ # systemctl status openvpn-client@lugor
● ope...@lu... - OpenVPN tunnel for lugor
Loaded: loaded (/usr/lib/systemd/system/openvpn-client@.service; enabled; vendor preset: disabled)
Active: active (running) since Thu 2016-12-01 13:35:12 CET; 8s ago
Docs: man:openvpn(8)
https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage
https://community.openvpn.net/openvpn/wiki/HOWTO
Process: 11700 ExecStartPre=/bin/sh -c grep -q -E ^daemon %i.conf || exit 0 && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when being managed by systemd" ; exit 1 (code=exited, status=0/SUCCESS)
Main PID: 11703 (openvpn)
Tasks: 1 (limit: 4915)
CGroup: /system.slice/system-openvpn\x2dclient.slice/ope...@lu...
└─11703 /usr/sbin/openvpn --suppress-timestamps --nobind --config lugor.conf
Dec 01 13:35:13 leda openvpn[11703]: GID set to nobody
Dec 01 13:35:13 leda openvpn[11703]: UID set to nobody
Dec 01 13:35:13 leda openvpn[11703]: Initialization Sequence Completed
root@leda ~ # # looks good...
root@leda ~ # echo "bad-option" >> /etc/openvpn/client/lugor.conf
root@leda ~ # systemctl restart openvpn-client@lugor
root@leda ~ # # succeeds, no?
root@leda ~ # systemctl status openvpn-client@lugor
● ope...@lu... - OpenVPN tunnel for lugor
Loaded: loaded (/usr/lib/systemd/system/openvpn-client@.service; enabled; vendor preset: disabled)
Active: failed (Result: exit-code) since Thu 2016-12-01 13:36:14 CET; 15s ago
Docs: man:openvpn(8)
https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage
https://community.openvpn.net/openvpn/wiki/HOWTO
Process: 11911 ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf (code=exited, status=1/FAILURE)
Process: 11908 ExecStartPre=/bin/sh -c grep -q -E ^daemon %i.conf || exit 0 && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when being managed by systemd" ; exit 1 (code=exited, status=0/SUCCESS)
Main PID: 11911 (code=exited, status=1/FAILURE)
Dec 01 13:36:14 leda systemd[1]: Starting OpenVPN tunnel for lugor...
Dec 01 13:36:14 leda systemd[1]: Started OpenVPN tunnel for lugor.
Dec 01 13:36:14 leda openvpn[11911]: Options error: Unrecognized option or missing or extra parameter(s) in lugor.conf:32: bad-option (2.4_beta2)
Dec 01 13:36:14 leda openvpn[11911]: Use --help for more information.
Dec 01 13:36:14 leda systemd[1]: ope...@lu...: Main process exited, code=exited, status=1/FAILURE
Dec 01 13:36:14 leda systemd[1]: ope...@lu...: Unit entered failed state.
Dec 01 13:36:14 leda systemd[1]: ope...@lu...: Failed with result 'exit-code'.
3 root@leda ~ # # Oops...
3 root@leda ~ # # now install openvpn with my systemd patches
3 root@leda ~ # systemctl restart openvpn-client@lugor
Job for ope...@lu... failed because the control process exited with error code.
See "systemctl status ope...@lu..." and "journalctl -xe" for details.
1 root@leda ~ # systemctl status openvpn-client@lugor
● ope...@lu... - OpenVPN tunnel for lugor
Loaded: loaded (/usr/lib/systemd/system/openvpn-client@.service; enabled; vendor preset: disabled)
Active: failed (Result: exit-code) since Thu 2016-12-01 13:37:10 CET; 50s ago
Docs: man:openvpn(8)
https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage
https://community.openvpn.net/openvpn/wiki/HOWTO
Process: 12227 ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf (code=exited, status=1/FAILURE)
Main PID: 12227 (code=exited, status=1/FAILURE)
Dec 01 13:37:10 leda systemd[1]: Starting OpenVPN tunnel for lugor...
Dec 01 13:37:10 leda openvpn[12227]: Options error: Unrecognized option or missing or extra parameter(s) in lugor.conf:32: bad-option (2.4_beta2)
Dec 01 13:37:10 leda openvpn[12227]: Use --help for more information.
Dec 01 13:37:10 leda systemd[1]: ope...@lu...: Main process exited, code=exited, status=1/FAILURE
Dec 01 13:37:10 leda systemd[1]: Failed to start OpenVPN tunnel for lugor.
Dec 01 13:37:10 leda systemd[1]: ope...@lu...: Unit entered failed state.
Dec 01 13:37:10 leda systemd[1]: ope...@lu...: Failed with result 'exit-code'.
With current code the unit reports success and fails later in background,
with my code it fails immediately and reports the error to the user. Later
behavior lets things work for enabled unit with dependencies.
--
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
|
|
From: Christian H. <li...@ew...> - 2016-11-30 08:59:29
|
From: Christian Hesse <ma...@ew...>
We start with systemd Type=notify, so refuse to daemonize.
Signed-off-by: Christian Hesse <ma...@ew...>
---
distro/systemd/openvpn-client@.service | 1 -
distro/systemd/openvpn-server@.service | 1 -
src/openvpn/init.c | 7 +++++++
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/distro/systemd/openvpn-client@.service b/distro/systemd/openvpn-client@.service
index f64a239..5618af3 100644
--- a/distro/systemd/openvpn-client@.service
+++ b/distro/systemd/openvpn-client@.service
@@ -12,7 +12,6 @@ PrivateTmp=true
RuntimeDirectory=openvpn-client
RuntimeDirectoryMode=0710
WorkingDirectory=/etc/openvpn/client
-ExecStartPre=/bin/sh -c 'grep -q -E ^daemon %i.conf || exit 0 && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when being managed by systemd" ; exit 1'
ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf
CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
LimitNPROC=10
diff --git a/distro/systemd/openvpn-server@.service b/distro/systemd/openvpn-server@.service
index 890e6a9..b9b4dba 100644
--- a/distro/systemd/openvpn-server@.service
+++ b/distro/systemd/openvpn-server@.service
@@ -12,7 +12,6 @@ PrivateTmp=true
RuntimeDirectory=openvpn-server
RuntimeDirectoryMode=0710
WorkingDirectory=/etc/openvpn/server
-ExecStartPre=/bin/sh -c 'grep -q -E ^daemon %i.conf || exit 0 && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when being managed by systemd" ; exit 1'
ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.conf
CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
LimitNPROC=10
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 551e579..7ab5c52 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -926,6 +926,13 @@ bool
possibly_become_daemon (const struct options *options)
{
bool ret = false;
+
+#ifdef ENABLE_SYSTEMD
+ /* return without forking if we are running from systemd */
+ if (sd_notify(0, "READY=0") > 0)
+ return ret;
+#endif
+
if (options->daemon)
{
ASSERT (!options->inetd);
--
2.10.2
|
|
From: Steffan K. <ste...@fo...> - 2016-11-30 09:06:41
|
Hi,
On 30-11-16 09:59, Christian Hesse wrote:
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -926,6 +926,13 @@ bool
> possibly_become_daemon (const struct options *options)
> {
> bool ret = false;
> +
> +#ifdef ENABLE_SYSTEMD
> + /* return without forking if we are running from systemd */
> + if (sd_notify(0, "READY=0") > 0)
> + return ret;
> +#endif
> +
> if (options->daemon)
> {
> ASSERT (!options->inetd);
Does this mean I cannot run openvpn --config bla.conf --daemon from the
command line any more on a systemd system? This would be a deal-breaker
for me.
-Steffan
|
|
From: Christian H. <li...@ew...> - 2016-11-30 09:15:33
|
Steffan Karger <ste...@fo...> on Wed, 2016/11/30 10:06:
> Hi,
>
> On 30-11-16 09:59, Christian Hesse wrote:
> > --- a/src/openvpn/init.c
> > +++ b/src/openvpn/init.c
> > @@ -926,6 +926,13 @@ bool
> > possibly_become_daemon (const struct options *options)
> > {
> > bool ret = false;
> > +
> > +#ifdef ENABLE_SYSTEMD
> > + /* return without forking if we are running from systemd */
> > + if (sd_notify(0, "READY=0") > 0)
> > + return ret;
> > +#endif
> > +
> > if (options->daemon)
> > {
> > ASSERT (!options->inetd);
>
> Does this mean I cannot run openvpn --config bla.conf --daemon from the
> command line any more on a systemd system? This would be a deal-breaker
> for me.
No. That means openvpn knows when it is run from within a system service.
Daemonization is refused there as we we start with "Type=notify".
sd_notify() is a no-op when run from command line. So everything works as
usual.
--
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
|
|
From: Steffan K. <ste...@fo...> - 2016-11-30 09:19:21
Attachments:
signature.asc
|
On 30-11-16 10:15, Christian Hesse wrote:
> Steffan Karger <ste...@fo...> on Wed, 2016/11/30 10:06:
>> Hi,
>>
>> On 30-11-16 09:59, Christian Hesse wrote:
>>> --- a/src/openvpn/init.c
>>> +++ b/src/openvpn/init.c
>>> @@ -926,6 +926,13 @@ bool
>>> possibly_become_daemon (const struct options *options)
>>> {
>>> bool ret = false;
>>> +
>>> +#ifdef ENABLE_SYSTEMD
>>> + /* return without forking if we are running from systemd */
>>> + if (sd_notify(0, "READY=0") > 0)
>>> + return ret;
>>> +#endif
>>> +
>>> if (options->daemon)
>>> {
>>> ASSERT (!options->inetd);
>>
>> Does this mean I cannot run openvpn --config bla.conf --daemon from the
>> command line any more on a systemd system? This would be a deal-breaker
>> for me.
>
> No. That means openvpn knows when it is run from within a system service.
> Daemonization is refused there as we we start with "Type=notify".
>
> sd_notify() is a no-op when run from command line. So everything works as
> usual.
Perfect. Thanks for the swift response. (Confusing API though...)
-Steffan
|
|
From: David S. <op...@sf...> - 2016-11-30 11:55:01
Attachments:
signature.asc
|
On 30/11/16 10:15, Christian Hesse wrote:
> Steffan Karger <ste...@fo...> on Wed, 2016/11/30 10:06:
>> Hi,
>>
>> On 30-11-16 09:59, Christian Hesse wrote:
>>> --- a/src/openvpn/init.c
>>> +++ b/src/openvpn/init.c
>>> @@ -926,6 +926,13 @@ bool
>>> possibly_become_daemon (const struct options *options)
>>> {
>>> bool ret = false;
>>> +
>>> +#ifdef ENABLE_SYSTEMD
>>> + /* return without forking if we are running from systemd */
>>> + if (sd_notify(0, "READY=0") > 0)
>>> + return ret;
>>> +#endif
>>> +
>>> if (options->daemon)
>>> {
>>> ASSERT (!options->inetd);
>>
>> Does this mean I cannot run openvpn --config bla.conf --daemon from the
>> command line any more on a systemd system? This would be a deal-breaker
>> for me.
>
> No. That means openvpn knows when it is run from within a system service.
> Daemonization is refused there as we we start with "Type=notify".
>
> sd_notify() is a no-op when run from command line. So everything works as
> usual.
Ahh ... okay, I'll test the patch more thoroughly and I revoke my NAK if
everything works as expected.
--
kind regards,
David Sommerseth
OpenVPN Technologies, Inc
|
|
From: David S. <op...@sf...> - 2016-11-30 11:52:43
Attachments:
signature.asc
|
On 30/11/16 09:59, Christian Hesse wrote:
> From: Christian Hesse <ma...@ew...>
>
> We start with systemd Type=notify, so refuse to daemonize.
>
> Signed-off-by: Christian Hesse <ma...@ew...>
> ---
> distro/systemd/openvpn-client@.service | 1 -
> distro/systemd/openvpn-server@.service | 1 -
> src/openvpn/init.c | 7 +++++++
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/distro/systemd/openvpn-client@.service b/distro/systemd/openvpn-client@.service
> index f64a239..5618af3 100644
> --- a/distro/systemd/openvpn-client@.service
> +++ b/distro/systemd/openvpn-client@.service
> @@ -12,7 +12,6 @@ PrivateTmp=true
> RuntimeDirectory=openvpn-client
> RuntimeDirectoryMode=0710
> WorkingDirectory=/etc/openvpn/client
> -ExecStartPre=/bin/sh -c 'grep -q -E ^daemon %i.conf || exit 0 && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when being managed by systemd" ; exit 1'
> ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf
> CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
> LimitNPROC=10
> diff --git a/distro/systemd/openvpn-server@.service b/distro/systemd/openvpn-server@.service
> index 890e6a9..b9b4dba 100644
> --- a/distro/systemd/openvpn-server@.service
> +++ b/distro/systemd/openvpn-server@.service
> @@ -12,7 +12,6 @@ PrivateTmp=true
> RuntimeDirectory=openvpn-server
> RuntimeDirectoryMode=0710
> WorkingDirectory=/etc/openvpn/server
> -ExecStartPre=/bin/sh -c 'grep -q -E ^daemon %i.conf || exit 0 && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when being managed by systemd" ; exit 1'
> ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.conf
> CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
> LimitNPROC=10
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 551e579..7ab5c52 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -926,6 +926,13 @@ bool
> possibly_become_daemon (const struct options *options)
> {
> bool ret = false;
> +
> +#ifdef ENABLE_SYSTEMD
> + /* return without forking if we are running from systemd */
> + if (sd_notify(0, "READY=0") > 0)
> + return ret;
> +#endif
> +
> if (options->daemon)
> {
> ASSERT (!options->inetd);
>
NAK on this approach. We cannot dictate that users _must_ start OpenVPN
as a daemon via systemd if it has been built with systemd support.
I understand the sentiment for this change, but we need to ensure users
may use their own scripts and hand-crafted configs to start OpenVPN,
also if systemd is present.
--
kind regards,
David Sommerseth
OpenVPN Technologies, Inc
|
|
From: Christian H. <li...@ew...> - 2016-11-30 12:22:35
|
David Sommerseth <op...@sf...> on Wed, 2016/11/30 12:52:
> On 30/11/16 09:59, Christian Hesse wrote:
> > From: Christian Hesse <ma...@ew...>
> >
> > We start with systemd Type=notify, so refuse to daemonize.
> >
> > Signed-off-by: Christian Hesse <ma...@ew...>
> > ---
> > distro/systemd/openvpn-client@.service | 1 -
> > distro/systemd/openvpn-server@.service | 1 -
> > src/openvpn/init.c | 7 +++++++
> > 3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/distro/systemd/openvpn-client@.service
> > b/distro/systemd/openvpn-client@.service index f64a239..5618af3 100644
> > --- a/distro/systemd/openvpn-client@.service
> > +++ b/distro/systemd/openvpn-client@.service
> > @@ -12,7 +12,6 @@ PrivateTmp=true
> > RuntimeDirectory=openvpn-client
> > RuntimeDirectoryMode=0710
> > WorkingDirectory=/etc/openvpn/client
> > -ExecStartPre=/bin/sh -c 'grep -q -E ^daemon %i.conf || exit 0
> > && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when
> > being managed by systemd" ; exit 1' ExecStart=/usr/sbin/openvpn
> > --suppress-timestamps --nobind --config %i.conf
> > CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID
> > CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE LimitNPROC=10 diff --git
> > a/distro/systemd/openvpn-server@.service
> > b/distro/systemd/openvpn-server@.service index 890e6a9..b9b4dba 100644
> > --- a/distro/systemd/openvpn-server@.service +++
> > b/distro/systemd/openvpn-server@.service @@ -12,7 +12,6 @@
> > PrivateTmp=true RuntimeDirectory=openvpn-server RuntimeDirectoryMode=0710
> > WorkingDirectory=/etc/openvpn/server
> > -ExecStartPre=/bin/sh -c 'grep -q -E ^daemon %i.conf || exit 0
> > && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when
> > being managed by systemd" ; exit 1' ExecStart=/usr/sbin/openvpn --status
> > %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps
> > --config %i.conf CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN
> > CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT
> > CAP_DAC_OVERRIDE LimitNPROC=10 diff --git a/src/openvpn/init.c
> > b/src/openvpn/init.c index 551e579..7ab5c52 100644 ---
> > a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -926,6 +926,13 @@ bool
> > possibly_become_daemon (const struct options *options)
> > {
> > bool ret = false;
> > +
> > +#ifdef ENABLE_SYSTEMD
> > + /* return without forking if we are running from systemd */
> > + if (sd_notify(0, "READY=0") > 0)
> > + return ret;
> > +#endif
> > +
> > if (options->daemon)
> > {
> > ASSERT (!options->inetd);
> >
>
> NAK on this approach. We cannot dictate that users _must_ start OpenVPN
> as a daemon via systemd if it has been built with systemd support.
>
> I understand the sentiment for this change, but we need to ensure users
> may use their own scripts and hand-crafted configs to start OpenVPN,
> also if systemd is present.
We do not dictate anything. Starting openvpn from scripts or command line
this is a no-op. From man sd_notify(3):
> On failure, these calls return a negative errno-style error code. If
> $NOTIFY_SOCKET was not set and hence no status data could be sent, 0 is
> returned. If the status was sent, these functions return with a positive
> return value. In order to support both, init systems that implement this
> scheme and those which do not, it is generally recommended to ignore the
> return value of this call.
That is what we check for here: If (and only if) openvpn is started from
systemd (read: from a systemd unit/service) it denies to fork. The openvpn
service starts properly even with "daemon" in config file.
This is unrelated to systemd being installed or the system being booted with
systemd.
--
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
|
Thanks for helping keep SourceForge clean.
X