[ovs-dev] [RFC 3/3] Red Hat Systemd Integration

Thu Jul 7 00:31:07 UTC 2016

Flavio Leitner <fbl at sysclose.org> writes:

> On Wed, Jul 06, 2016 at 04:28:26PM -0400, Aaron Conole wrote:
>> Flavio Leitner <fbl at sysclose.org> writes:
>> 
>> > On Fri, Jul 01, 2016 at 05:26:23PM -0400, Aaron Conole wrote:
>> >> +ConditionPathIsReadWrite=/var/run/openvswitch/db.sock
>> >
>> > Interesting, so we start ovs-vswitchd only when db.sock is ready.
>> >
>> > By default, the prefix is /usr/local and so the db.sock location
>> > would be /usr/local/var/run/openvswitch/db.sock.  That can be
>> > fixed moving the file to .in and add to the makefile as OVS does
>> > for spec files, for instance.
>> 
>> I can make it depend on rundir, but there were other hardcoded paths, so
>> I thought one more wouldn't make a difference.  I could fix all the
>> hardcoded paths, though.  Should I do that?
>
> You have a point.  The bug is already there and is unrelated to
> this patchset, so drop my comment.
>
>> >> +
>> >> +[Service]
>> >> +Type=forking
>> >
>> > I think systemd monitors the thread after the first fork as
>> > the main process which in this case it would be the execution
>> > of the daemon itself, which forks again.  Shouldn't we provide
>> > PIDFile then? I honestly don't know what systemd will do in
>> > this case.
>> 
>> That's what the --no-monitoring (well, renaming it to --no-monitor) call
>> does.  'Type=forking' is a hint to systemd to watch for the last process
>> remaining instead of reasoning that the first process started is the
>> process to track.  It appears (at least on fedora and RHEL7) to track
>> the ovs daemons properly.
>> 
>> systemd won't poll the PID at the PIDFile.  It's really just a hint to
>> systemd that the process now exists.  There is no detection of failure
>> or any kind of other fancy systemd integrations.  I tried using that
>> instead of writing the monitor patch.
>
> Yup, the key is 'last process'.  I just tried on my env and it is 
> beautiful!
>
> # systemctl status ovsdb-server 
> ● ovsdb-server.service - Open vSwitch Database Unit
>    Loaded: loaded (/usr/lib/systemd/system/ovsdb-server.service; static; vendor preset: disabled)
>    Active: active (running) since Wed 2016-07-06 19:05:15 BRT; 4s ago
>   Process: 9222 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd --no-monitoring start --system-id=random $OPTIONS (code=exited, status=0/SUCCESS)
>  Main PID: 9514 (ovsdb-server)
>    Memory: 1.0M
>       CPU: 182ms
>    CGroup: /system.slice/ovsdb-server.service
>            └─9514 ovsdb-server /etc/openvswitch/conf.db -vconsole:emer -vsysl...
>
>
> $ ps -e -T  | grep ovsdb-server
>  9514  9514 ?        00:00:00 ovsdb-server
>
> Looks correct.
> # kill -9 9514
> # systemctl status ovsdb-server 
> ● ovsdb-server.service - Open vSwitch Database Unit
>    Loaded: loaded (/usr/lib/systemd/system/ovsdb-server.service; static; vendor preset: disabled)
>    Active: failed (Result: signal) since Wed 2016-07-06 19:07:56 BRT; 2s ago
>   Process: 9706 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop (code=exited, status=0/SUCCESS)
>   Process: 9222 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd --no-monitoring start --system-id=random $OPTIONS (code=exited, status=0/SUCCESS)
>  Main PID: 9514 (code=killed, signal=KILL)
>
>
> Now ovs-vswitchd:
> # systemctl status ovs-vswitchd 
> ● ovs-vswitchd.service - Open vSwitch Forwarding Unit
>    Loaded: loaded (/usr/lib/systemd/system/ovs-vswitchd.service; static; vendor preset: disabled)
>    Active: active (running) since Wed 2016-07-06 19:05:16 BRT; 3min 12s ago
>   Process: 9217 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server --no-monitoring start --system-id=random $OPTIONS (code=exited, status=0/SUCCESS)
>  Main PID: 9482 (ovs-vswitchd)
>    Memory: 74.5M
>       CPU: 587ms
>    CGroup: /system.slice/ovs-vswitchd.service
>            └─9482 ovs-vswitchd unix:/var/run/openvswitch/db.sock -vconsole:em...
>
> $ ps -e -T | grep 9482
>  9482  9482 ?        00:00:00 ovs-vswitchd
>  9482  9592 ?        00:00:00 urcu5
>  9482  9602 ?        00:00:00 handler10
>  9482  9603 ?        00:00:00 handler11
>  9482  9604 ?        00:00:00 handler12
>  9482  9605 ?        00:00:00 handler13
>  9482  9606 ?        00:00:00 handler14
>  9482  9607 ?        00:00:00 revalidator15
>  9482  9608 ?        00:00:00 revalidator16
>  9482  9609 ?        00:00:00 revalidator17
>
> # kill -9 9607
> # systemctl status ovs-vswitchd 
> ● ovs-vswitchd.service - Open vSwitch Forwarding Unit
>    Loaded: loaded (/usr/lib/systemd/system/ovs-vswitchd.service; static; vendor preset: disabled)
>    Active: failed (Result: signal) since Wed 2016-07-06 19:08:56 BRT; 1s ago
>   Process: 9831 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop (code=exited, status=0/SUCCESS)
>   Process: 9217 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server --no-monitoring start --system-id=random $OPTIONS (code=exited, status=0/SUCCESS)
>  Main PID: 9482 (code=killed, signal=KILL)
>
>
> But there is a catch here.  When I kill any of the daemons, the
> openvswitch.service remains active which hides the actual state
> of OVS.  Have you considered doing this?
>
> --- a/rhel/usr_lib_systemd_system_openvswitch.service
> +++ b/rhel/usr_lib_systemd_system_openvswitch.service
> @@ -1,8 +1,8 @@
>  [Unit]
>  Description=Open vSwitch
>  After=syslog.target network.target ovsdb-server.service
> -Requires=ovsdb-server.service
> -Requires=ovs-vswitchd.service
> +BindsTo=ovsdb-server.service
> +BindsTo=ovs-vswitchd.service

No, but it seems to make a better guarantee, so I'll switch to it.

>  [Service]
>  Type=oneshot
>
>
>
>> 
>> >> +EnvironmentFile=-/etc/sysconfig/openvswitch
>> >> +ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>> >> +          --no-ovsdb-server --no-monitoring start \
>> >> +          --system-id=random $OPTIONS
>> >> +ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
>> >> +ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
>> >> +          --no-monitoring restart \
>> >> +          --system-id=random
>> >> +RuntimeDirectory=openvswitch
>> >> +RuntimeDirectoryMode=0755
>> >> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> >> index e4c2a66..1fe09e5 100644
>> >> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>> >> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> >> @@ -1,15 +1,16 @@
>> >>  [Unit]
>> >> -Description=Open vSwitch Internal Unit
>> >> +Description=Open vSwitch Database Unit
>> >>  After=syslog.target
>> >>  PartOf=openvswitch.service
>> >>  Wants=openvswitch.service
>> >> +ReloadPropagatedFrom=openvswitch.service
>> >>  
>> >>  [Service]
>> >> -Type=oneshot
>> >> -RemainAfterExit=yes
>> >> +Type=forking
>> >>  EnvironmentFile=-/etc/sysconfig/openvswitch
>> >> -ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
>> >> +ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>> >> +          --no-ovs-vswitchd --no-monitoring start \
>> >>            --system-id=random $OPTIONS
>> >
>> >
>> > We now use $OPTIONS from /etc/sysconfig/openvswitch for both daemons.
>> > Does that work?  I think ovs-ctl will parse the options and only
>> > apply the ones that make sense for each daemon, so we would be OK.
>> 
>> Well, it was working for me, but I didn't do too much testing on that
>> front.  If you would prefer, I can change it to be per-daemon options
>> (so $OVS_VSWITCHD_OPTIONS and $OVSDB_SERVER_OPTIONS) if that would suit
>> better.
>
> I haven't tried and I wasn't sure after a quick look, so I asked :-)
> On a second look it seems to be fine though, no worries.