On Fri, Jul 01, 2016 at 05:26:23PM -0400, Aaron Conole wrote:
> This commit builds upon some of the recent ovs-ctl changes to build a
> more integrated systemd setup.  A new service (openvswitch-network) is

I think you renamed to 'ovs-vswitchd' after we talked offline :-)

> added to track the ovs-vswitchd, and openvswitch-nonetwork is reserved
> for the ovsdb-server daemon.  The systemd scripts still use ovs-ctl to
> actually initialize the daemons.

That's cool.

> Signed-off-by: Aaron Conole <acon...@redhat.com>
> ---
>  rhel/automake.mk                                 |  1 +
>  rhel/openvswitch-fedora.spec.in                  |  3 ++-
>  rhel/usr_lib_systemd_system_openvswitch.service  |  5 +++--
>  rhel/usr_lib_systemd_system_ovs-vswitchd.service | 21 +++++++++++++++++++++
>  rhel/usr_lib_systemd_system_ovsdb-server.service | 11 ++++++-----
>  5 files changed, 33 insertions(+), 8 deletions(-)
>  create mode 100644 rhel/usr_lib_systemd_system_ovs-vswitchd.service
> 
> diff --git a/rhel/automake.mk b/rhel/automake.mk
> index 7907a87..a3c180c 100644
> --- a/rhel/automake.mk
> +++ b/rhel/automake.mk
> @@ -27,6 +27,7 @@ EXTRA_DIST += \
>       rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>       rhel/usr_lib_systemd_system_openvswitch.service \
>       rhel/usr_lib_systemd_system_ovsdb-server.service \
> +     rhel/usr_lib_systemd_system_ovs-vswitchd.service \
>       rhel/usr_lib_systemd_system_ovn-controller.service \
>       rhel/usr_lib_systemd_system_ovn-controller-vtep.service \
>       rhel/usr_lib_systemd_system_ovn-northd.service
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index ed7b3c4..09756ec 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -189,7 +189,7 @@ install -d -m 0755 
> $RPM_BUILD_ROOT%{_sysconfdir}/openvswitch
>  install -p -D -m 0644 \
>          rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>          $RPM_BUILD_ROOT/%{_sysconfdir}/sysconfig/openvswitch
> -for service in openvswitch ovsdb-server \
> +for service in openvswitch ovsdb-server ovs-vswitchd \
>               ovn-controller ovn-controller-vtep ovn-northd; do
>       install -p -D -m 0644 \
>                       rhel/usr_lib_systemd_system_${service}.service \
> @@ -417,6 +417,7 @@ fi
>  %config(noreplace) %{_sysconfdir}/logrotate.d/openvswitch
>  %{_unitdir}/openvswitch.service
>  %{_unitdir}/ovsdb-server.service
> +%{_unitdir}/ovs-vswitchd.service
>  %{_datadir}/openvswitch/scripts/openvswitch.init
>  %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
>  %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> diff --git a/rhel/usr_lib_systemd_system_openvswitch.service 
> b/rhel/usr_lib_systemd_system_openvswitch.service
> index 96c697b..205cba7 100644
> --- a/rhel/usr_lib_systemd_system_openvswitch.service
> +++ b/rhel/usr_lib_systemd_system_openvswitch.service
> @@ -2,12 +2,13 @@
>  Description=Open vSwitch
>  After=syslog.target network.target ovsdb-server.service
>  Requires=ovsdb-server.service
> +Requires=ovs-vswitchd.service
>  
>  [Service]
>  Type=oneshot
> -ExecStart=/bin/true
> -ExecStop=/bin/true
>  RemainAfterExit=yes
> +ExecStart=/bin/true
> +ExecReload=/bin/true

You're adding ExecReload to avoid signaling any process and since the
reload signal is propagated, that is good. But I don't see why we
don't need ExecStop anymore.  I was quite sure they all fall into the
same problem.


>  [Install]
>  WantedBy=multi-user.target
> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service 
> b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
> new file mode 100644
> index 0000000..18546d7
> --- /dev/null
> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
> @@ -0,0 +1,21 @@
> +[Unit]
> +Description=Open vSwitch Forwarding Unit
> +After=ovsdb-server.target
> +PartOf=openvswitch.service
> +Wants=openvswitch.service
> +Requires=ovsdb-server.service
> +ReloadPropagatedFrom=ovsdb-server.service

If I am reading right: openvswitch propagates to ovsdb-server
which propagates to ovs-vswitchd.
Looks good.

> +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.


> +
> +[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.

> +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.

> -ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
> +ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
>  RuntimeDirectory=openvswitch
>  RuntimeDirectoryMode=0755
> -- 
> 2.5.5
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

-- 
fbl

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to