Re: [Openvpn-devel] [PATCH 1/1] Use systemd service manager notification

[Christian H. <li...@ew...>]

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);}