diff mbox

[ovs-dev,RFC,2/3] rhel/ovsdb-server.service: Rename the nonetwork service

Message ID 1467408383-5703-3-git-send-email-aconole@redhat.com
State Superseded
Headers show

Commit Message

Aaron Conole July 1, 2016, 9:26 p.m. UTC
Currently, openvswitch.service calls out to start
openvswitch-nonetwork.service.  However, openvswitch-nonetwork.service
is better called ovsdb-server, since that is truly nonetwork.  This
commit does make the file a bit of a misnomer - currently the
ovsdb-server service will start the ovs-vswitchd service as well.  A
future commit will clean this up.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 rhel/automake.mk                                          |  2 +-
 rhel/etc_sysconfig_network-scripts_ifdown-ovs             | 14 +++++++-------
 rhel/etc_sysconfig_network-scripts_ifup-ovs               | 14 +++++++-------
 rhel/openvswitch-fedora.spec.in                           |  4 ++--
 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service | 15 ---------------
 rhel/usr_lib_systemd_system_openvswitch.service           |  4 ++--
 rhel/usr_lib_systemd_system_ovsdb-server.service          | 15 +++++++++++++++
 7 files changed, 34 insertions(+), 34 deletions(-)
 delete mode 100644 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
 create mode 100644 rhel/usr_lib_systemd_system_ovsdb-server.service

Comments

Ben Pfaff July 2, 2016, 4:33 a.m. UTC | #1
On Fri, Jul 01, 2016 at 05:26:22PM -0400, Aaron Conole wrote:
> Currently, openvswitch.service calls out to start
> openvswitch-nonetwork.service.  However, openvswitch-nonetwork.service
> is better called ovsdb-server, since that is truly nonetwork.  This
> commit does make the file a bit of a misnomer - currently the
> ovsdb-server service will start the ovs-vswitchd service as well.  A
> future commit will clean this up.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>

Seems reasonable to me but I think that Flavio or Russell should really
review it.
Flavio Leitner July 6, 2016, 5:57 p.m. UTC | #2
On Fri, Jul 01, 2016 at 05:26:22PM -0400, Aaron Conole wrote:
> Currently, openvswitch.service calls out to start
> openvswitch-nonetwork.service.  However, openvswitch-nonetwork.service
> is better called ovsdb-server, since that is truly nonetwork.  This
> commit does make the file a bit of a misnomer - currently the
> ovsdb-server service will start the ovs-vswitchd service as well.  A
> future commit will clean this up.


For the record, the 'nonetwork' means that the systemd service
doesn't depend on the 'network' target to support cases where
OVS builds the network needed prior the 'network' target.

However, it doesn't make sense if we move to 1:1 mapping
between services and daemons which looks better nowadays.

> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  rhel/automake.mk                                          |  2 +-
>  rhel/etc_sysconfig_network-scripts_ifdown-ovs             | 14 +++++++-------
>  rhel/etc_sysconfig_network-scripts_ifup-ovs               | 14 +++++++-------
>  rhel/openvswitch-fedora.spec.in                           |  4 ++--
>  rhel/usr_lib_systemd_system_openvswitch-nonetwork.service | 15 ---------------
>  rhel/usr_lib_systemd_system_openvswitch.service           |  4 ++--
>  rhel/usr_lib_systemd_system_ovsdb-server.service          | 15 +++++++++++++++
>  7 files changed, 34 insertions(+), 34 deletions(-)
>  delete mode 100644 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>  create mode 100644 rhel/usr_lib_systemd_system_ovsdb-server.service
> 
> diff --git a/rhel/automake.mk b/rhel/automake.mk
> index dc30715..7907a87 100644
> --- a/rhel/automake.mk
> +++ b/rhel/automake.mk
> @@ -26,7 +26,7 @@ EXTRA_DIST += \
>  	rhel/usr_share_openvswitch_scripts_sysconfig.template \
>  	rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>  	rhel/usr_lib_systemd_system_openvswitch.service \
> -	rhel/usr_lib_systemd_system_openvswitch-nonetwork.service \
> +	rhel/usr_lib_systemd_system_ovsdb-server.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/etc_sysconfig_network-scripts_ifdown-ovs b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
> index 46b6ca5..b57aebf 100755
> --- a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
> +++ b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
> @@ -34,15 +34,15 @@ if [ ! -x ${OTHERSCRIPT} ]; then
>      OTHERSCRIPT="/etc/sysconfig/network-scripts/ifdown-eth"
>  fi
>  
> -SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
> +SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
>  if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
> -	if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
> -		systemctl start openvswitch-nonetwork.service
> -	fi
> +    if ! systemctl --quiet is-active ovsdb-server.service; then
> +        systemctl start ovsdb-server.service
> +    fi

When shutting down or bringing down an interface, it seems that
having only ovsdb-server would be enough.

Traditionally SysV scripts use TABs and not spaces. You can see
some examples in /etc/rc.d/init.d/ yet today.  But I have no
strong opinion between TAB and spaces besides to keep consistency. 


>  else
> -	if [ ! -f /var/lock/subsys/openvswitch ]; then
> -		/sbin/service openvswitch start
> -	fi
> +    if [ ! -f /var/lock/subsys/openvswitch ]; then
> +        /sbin/service openvswitch start
> +    fi
>  fi

This seems unneeded or the intention was to fix the tabs.

>  case "$TYPE" in
> diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs b/rhel/etc_sysconfig_network-scripts_ifup-ovs
> index f3fc05e..9b2efbb 100755
> --- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
> +++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
> @@ -60,15 +60,15 @@ fi
>  	fi
>  done
>  
> -SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
> +SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
>  if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
> -	if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
> -		systemctl start openvswitch-nonetwork.service
> -	fi
> +    if ! systemctl --quiet is-active ovsdb-server.service; then
> +        systemctl start ovsdb-server.service
> +    fi

However, I am not sure that is correct here because when we run
'ifup ovsbr0' and it has DHCP on it, the vswitchd is not running yet
and dhcp would fail.  It seems we need to start the whole service
(openvswitch.service) at this point.


>  else
> -	if [ ! -f /var/lock/subsys/openvswitch ]; then
> -		/sbin/service openvswitch start
> -	fi
> +    if [ ! -f /var/lock/subsys/openvswitch ]; then
> +        /sbin/service openvswitch start
> +    fi
>  fi
>  
>  case "$TYPE" in
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index 16894b0..ed7b3c4 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 openvswitch-nonetwork \
> +for service in openvswitch ovsdb-server \
>  		ovn-controller ovn-controller-vtep ovn-northd; do
>  	install -p -D -m 0644 \
>  			rhel/usr_lib_systemd_system_${service}.service \
> @@ -416,7 +416,7 @@ fi
>  %config(noreplace) %{_sysconfdir}/sysconfig/openvswitch
>  %config(noreplace) %{_sysconfdir}/logrotate.d/openvswitch
>  %{_unitdir}/openvswitch.service
> -%{_unitdir}/openvswitch-nonetwork.service
> +%{_unitdir}/ovsdb-server.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-nonetwork.service b/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
> deleted file mode 100644
> index e4c2a66..0000000
> --- a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -[Unit]
> -Description=Open vSwitch Internal Unit
> -After=syslog.target
> -PartOf=openvswitch.service
> -Wants=openvswitch.service
> -
> -[Service]
> -Type=oneshot
> -RemainAfterExit=yes
> -EnvironmentFile=-/etc/sysconfig/openvswitch
> -ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
> -          --system-id=random $OPTIONS
> -ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
> -RuntimeDirectory=openvswitch
> -RuntimeDirectoryMode=0755
> diff --git a/rhel/usr_lib_systemd_system_openvswitch.service b/rhel/usr_lib_systemd_system_openvswitch.service
> index f0bc16f..96c697b 100644
> --- a/rhel/usr_lib_systemd_system_openvswitch.service
> +++ b/rhel/usr_lib_systemd_system_openvswitch.service
> @@ -1,7 +1,7 @@
>  [Unit]
>  Description=Open vSwitch
> -After=syslog.target network.target openvswitch-nonetwork.service
> -Requires=openvswitch-nonetwork.service
> +After=syslog.target network.target ovsdb-server.service
> +Requires=ovsdb-server.service

If we change the above ifup-ovs to start the ovs service, we also
need to change the 'After' line to not require network.target.


>  [Service]
>  Type=oneshot
> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
> new file mode 100644
> index 0000000..e4c2a66
> --- /dev/null
> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
> @@ -0,0 +1,15 @@
> +[Unit]
> +Description=Open vSwitch Internal Unit
> +After=syslog.target
> +PartOf=openvswitch.service
> +Wants=openvswitch.service
> +
> +[Service]
> +Type=oneshot
> +RemainAfterExit=yes
> +EnvironmentFile=-/etc/sysconfig/openvswitch
> +ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
> +          --system-id=random $OPTIONS
> +ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
> +RuntimeDirectory=openvswitch
> +RuntimeDirectoryMode=0755
> -- 
> 2.5.5
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Aaron Conole July 6, 2016, 6:40 p.m. UTC | #3
Flavio Leitner <fbl@sysclose.org> writes:

> On Fri, Jul 01, 2016 at 05:26:22PM -0400, Aaron Conole wrote:
>> Currently, openvswitch.service calls out to start
>> openvswitch-nonetwork.service.  However, openvswitch-nonetwork.service
>> is better called ovsdb-server, since that is truly nonetwork.  This
>> commit does make the file a bit of a misnomer - currently the
>> ovsdb-server service will start the ovs-vswitchd service as well.  A
>> future commit will clean this up.
>
>
> For the record, the 'nonetwork' means that the systemd service
> doesn't depend on the 'network' target to support cases where
> OVS builds the network needed prior the 'network' target.
>
> However, it doesn't make sense if we move to 1:1 mapping
> between services and daemons which looks better nowadays.

Yeah, I know.  This patch merely does the rename.  So it's actually a
bit misleading because it says ovsdb-server, but it's really everything.

Thanks so much for the review, Flavio!

>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>  rhel/automake.mk                                          |  2 +-
>>  rhel/etc_sysconfig_network-scripts_ifdown-ovs             | 14 +++++++-------
>>  rhel/etc_sysconfig_network-scripts_ifup-ovs               | 14 +++++++-------
>>  rhel/openvswitch-fedora.spec.in                           |  4 ++--
>>  rhel/usr_lib_systemd_system_openvswitch-nonetwork.service | 15 ---------------
>>  rhel/usr_lib_systemd_system_openvswitch.service           |  4 ++--
>>  rhel/usr_lib_systemd_system_ovsdb-server.service          | 15 +++++++++++++++
>>  7 files changed, 34 insertions(+), 34 deletions(-)
>>  delete mode 100644 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>>  create mode 100644 rhel/usr_lib_systemd_system_ovsdb-server.service
>> 
>> diff --git a/rhel/automake.mk b/rhel/automake.mk
>> index dc30715..7907a87 100644
>> --- a/rhel/automake.mk
>> +++ b/rhel/automake.mk
>> @@ -26,7 +26,7 @@ EXTRA_DIST += \
>>  	rhel/usr_share_openvswitch_scripts_sysconfig.template \
>>  	rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>>  	rhel/usr_lib_systemd_system_openvswitch.service \
>> -	rhel/usr_lib_systemd_system_openvswitch-nonetwork.service \
>> +	rhel/usr_lib_systemd_system_ovsdb-server.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/etc_sysconfig_network-scripts_ifdown-ovs b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> index 46b6ca5..b57aebf 100755
>> --- a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> +++ b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> @@ -34,15 +34,15 @@ if [ ! -x ${OTHERSCRIPT} ]; then
>>      OTHERSCRIPT="/etc/sysconfig/network-scripts/ifdown-eth"
>>  fi
>>  
>> -SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
>> +SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
>>  if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
>> -	if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
>> -		systemctl start openvswitch-nonetwork.service
>> -	fi
>> +    if ! systemctl --quiet is-active ovsdb-server.service; then
>> +        systemctl start ovsdb-server.service
>> +    fi
>
> When shutting down or bringing down an interface, it seems that
> having only ovsdb-server would be enough.
>
> Traditionally SysV scripts use TABs and not spaces. You can see
> some examples in /etc/rc.d/init.d/ yet today.  But I have no
> strong opinion between TAB and spaces besides to keep consistency. 

ACK.  Will conserve tabs.

>>  else
>> -	if [ ! -f /var/lock/subsys/openvswitch ]; then
>> -		/sbin/service openvswitch start
>> -	fi
>> +    if [ ! -f /var/lock/subsys/openvswitch ]; then
>> +        /sbin/service openvswitch start
>> +    fi
>>  fi
>
> This seems unneeded or the intention was to fix the tabs.

Yeah, I switched the tabs to spaces, but I'll undo that.

>>  case "$TYPE" in
>> diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs b/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> index f3fc05e..9b2efbb 100755
>> --- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> +++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> @@ -60,15 +60,15 @@ fi
>>  	fi
>>  done
>>  
>> -SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
>> +SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
>>  if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
>> -	if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
>> -		systemctl start openvswitch-nonetwork.service
>> -	fi
>> +    if ! systemctl --quiet is-active ovsdb-server.service; then
>> +        systemctl start ovsdb-server.service
>> +    fi
>
> However, I am not sure that is correct here because when we run
> 'ifup ovsbr0' and it has DHCP on it, the vswitchd is not running yet
> and dhcp would fail.  It seems we need to start the whole service
> (openvswitch.service) at this point.

Well, ovsdb-server is the whole service but it has a weird name.  This
is just a rename; but I agree it's confusing and can just squash the two
together if you want.  Otherwise, you're right and 3/3 would need to
change this to openvswitch.service

>>  else
>> -	if [ ! -f /var/lock/subsys/openvswitch ]; then
>> -		/sbin/service openvswitch start
>> -	fi
>> +    if [ ! -f /var/lock/subsys/openvswitch ]; then
>> +        /sbin/service openvswitch start
>> +    fi
>>  fi
>>  
>>  case "$TYPE" in
>> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
>> index 16894b0..ed7b3c4 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 openvswitch-nonetwork \
>> +for service in openvswitch ovsdb-server \
>>  		ovn-controller ovn-controller-vtep ovn-northd; do
>>  	install -p -D -m 0644 \
>>  			rhel/usr_lib_systemd_system_${service}.service \
>> @@ -416,7 +416,7 @@ fi
>>  %config(noreplace) %{_sysconfdir}/sysconfig/openvswitch
>>  %config(noreplace) %{_sysconfdir}/logrotate.d/openvswitch
>>  %{_unitdir}/openvswitch.service
>> -%{_unitdir}/openvswitch-nonetwork.service
>> +%{_unitdir}/ovsdb-server.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-nonetwork.service b/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> deleted file mode 100644
>> index e4c2a66..0000000
>> --- a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> +++ /dev/null
>> @@ -1,15 +0,0 @@
>> -[Unit]
>> -Description=Open vSwitch Internal Unit
>> -After=syslog.target
>> -PartOf=openvswitch.service
>> -Wants=openvswitch.service
>> -
>> -[Service]
>> -Type=oneshot
>> -RemainAfterExit=yes
>> -EnvironmentFile=-/etc/sysconfig/openvswitch
>> -ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
>> -          --system-id=random $OPTIONS
>> -ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
>> -RuntimeDirectory=openvswitch
>> -RuntimeDirectoryMode=0755
>> diff --git a/rhel/usr_lib_systemd_system_openvswitch.service b/rhel/usr_lib_systemd_system_openvswitch.service
>> index f0bc16f..96c697b 100644
>> --- a/rhel/usr_lib_systemd_system_openvswitch.service
>> +++ b/rhel/usr_lib_systemd_system_openvswitch.service
>> @@ -1,7 +1,7 @@
>>  [Unit]
>>  Description=Open vSwitch
>> -After=syslog.target network.target openvswitch-nonetwork.service
>> -Requires=openvswitch-nonetwork.service
>> +After=syslog.target network.target ovsdb-server.service
>> +Requires=ovsdb-server.service
>
> If we change the above ifup-ovs to start the ovs service, we also
> need to change the 'After' line to not require network.target.

Okay.  Why is that?

>>  [Service]
>>  Type=oneshot
>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service
>> b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> new file mode 100644
>> index 0000000..e4c2a66
>> --- /dev/null
>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> @@ -0,0 +1,15 @@
>> +[Unit]
>> +Description=Open vSwitch Internal Unit
>> +After=syslog.target
>> +PartOf=openvswitch.service
>> +Wants=openvswitch.service
>> +
>> +[Service]
>> +Type=oneshot
>> +RemainAfterExit=yes
>> +EnvironmentFile=-/etc/sysconfig/openvswitch
>> +ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
>> +          --system-id=random $OPTIONS
>> +ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
>> +RuntimeDirectory=openvswitch
>> +RuntimeDirectoryMode=0755
>> -- 
>> 2.5.5
>>
Flavio Leitner July 6, 2016, 9:46 p.m. UTC | #4
On Wed, Jul 06, 2016 at 02:40:01PM -0400, Aaron Conole wrote:
> Flavio Leitner <fbl@sysclose.org> writes:
> 
> > On Fri, Jul 01, 2016 at 05:26:22PM -0400, Aaron Conole wrote:
> >> diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs b/rhel/etc_sysconfig_network-scripts_ifup-ovs
> >> index f3fc05e..9b2efbb 100755
> >> --- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
> >> +++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
> >> @@ -60,15 +60,15 @@ fi
> >>  	fi
> >>  done
> >>  
> >> -SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
> >> +SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
> >>  if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
> >> -	if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
> >> -		systemctl start openvswitch-nonetwork.service
> >> -	fi
> >> +    if ! systemctl --quiet is-active ovsdb-server.service; then
> >> +        systemctl start ovsdb-server.service
> >> +    fi
> >
> > However, I am not sure that is correct here because when we run
> > 'ifup ovsbr0' and it has DHCP on it, the vswitchd is not running yet
> > and dhcp would fail.  It seems we need to start the whole service
> > (openvswitch.service) at this point.
> 
> Well, ovsdb-server is the whole service but it has a weird name.  This
> is just a rename; but I agree it's confusing and can just squash the two
> together if you want.  Otherwise, you're right and 3/3 would need to
> change this to openvswitch.service

Exactly the point. We need full service running because we will
actually use the interface further down in the script.

[...]
> >> diff --git a/rhel/usr_lib_systemd_system_openvswitch.service b/rhel/usr_lib_systemd_system_openvswitch.service
> >> index f0bc16f..96c697b 100644
> >> --- a/rhel/usr_lib_systemd_system_openvswitch.service
> >> +++ b/rhel/usr_lib_systemd_system_openvswitch.service
> >> @@ -1,7 +1,7 @@
> >>  [Unit]
> >>  Description=Open vSwitch
> >> -After=syslog.target network.target openvswitch-nonetwork.service
> >> -Requires=openvswitch-nonetwork.service
> >> +After=syslog.target network.target ovsdb-server.service
> >> +Requires=ovsdb-server.service
> >
> > If we change the above ifup-ovs to start the ovs service, we also
> > need to change the 'After' line to not require network.target.
> 
> Okay.  Why is that?

If I recall correctly, sysv network script provides the $network but
the ifup-ovs is executed as part if it, so it can't run after.

Also that OVS does provide network connectivity so using After=
will tell systemd to shutdown OVS before stopping the network and
that will break all apps that need to terminate before the network
goes down.

I am glad to be proved wrong though :-)
diff mbox

Patch

diff --git a/rhel/automake.mk b/rhel/automake.mk
index dc30715..7907a87 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -26,7 +26,7 @@  EXTRA_DIST += \
 	rhel/usr_share_openvswitch_scripts_sysconfig.template \
 	rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
 	rhel/usr_lib_systemd_system_openvswitch.service \
-	rhel/usr_lib_systemd_system_openvswitch-nonetwork.service \
+	rhel/usr_lib_systemd_system_ovsdb-server.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/etc_sysconfig_network-scripts_ifdown-ovs b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
index 46b6ca5..b57aebf 100755
--- a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
@@ -34,15 +34,15 @@  if [ ! -x ${OTHERSCRIPT} ]; then
     OTHERSCRIPT="/etc/sysconfig/network-scripts/ifdown-eth"
 fi
 
-SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
+SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
 if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
-	if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
-		systemctl start openvswitch-nonetwork.service
-	fi
+    if ! systemctl --quiet is-active ovsdb-server.service; then
+        systemctl start ovsdb-server.service
+    fi
 else
-	if [ ! -f /var/lock/subsys/openvswitch ]; then
-		/sbin/service openvswitch start
-	fi
+    if [ ! -f /var/lock/subsys/openvswitch ]; then
+        /sbin/service openvswitch start
+    fi
 fi
 
 case "$TYPE" in
diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs b/rhel/etc_sysconfig_network-scripts_ifup-ovs
index f3fc05e..9b2efbb 100755
--- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
@@ -60,15 +60,15 @@  fi
 	fi
 done
 
-SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
+SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
 if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
-	if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
-		systemctl start openvswitch-nonetwork.service
-	fi
+    if ! systemctl --quiet is-active ovsdb-server.service; then
+        systemctl start ovsdb-server.service
+    fi
 else
-	if [ ! -f /var/lock/subsys/openvswitch ]; then
-		/sbin/service openvswitch start
-	fi
+    if [ ! -f /var/lock/subsys/openvswitch ]; then
+        /sbin/service openvswitch start
+    fi
 fi
 
 case "$TYPE" in
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 16894b0..ed7b3c4 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 openvswitch-nonetwork \
+for service in openvswitch ovsdb-server \
 		ovn-controller ovn-controller-vtep ovn-northd; do
 	install -p -D -m 0644 \
 			rhel/usr_lib_systemd_system_${service}.service \
@@ -416,7 +416,7 @@  fi
 %config(noreplace) %{_sysconfdir}/sysconfig/openvswitch
 %config(noreplace) %{_sysconfdir}/logrotate.d/openvswitch
 %{_unitdir}/openvswitch.service
-%{_unitdir}/openvswitch-nonetwork.service
+%{_unitdir}/ovsdb-server.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-nonetwork.service b/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
deleted file mode 100644
index e4c2a66..0000000
--- a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
+++ /dev/null
@@ -1,15 +0,0 @@ 
-[Unit]
-Description=Open vSwitch Internal Unit
-After=syslog.target
-PartOf=openvswitch.service
-Wants=openvswitch.service
-
-[Service]
-Type=oneshot
-RemainAfterExit=yes
-EnvironmentFile=-/etc/sysconfig/openvswitch
-ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
-          --system-id=random $OPTIONS
-ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
-RuntimeDirectory=openvswitch
-RuntimeDirectoryMode=0755
diff --git a/rhel/usr_lib_systemd_system_openvswitch.service b/rhel/usr_lib_systemd_system_openvswitch.service
index f0bc16f..96c697b 100644
--- a/rhel/usr_lib_systemd_system_openvswitch.service
+++ b/rhel/usr_lib_systemd_system_openvswitch.service
@@ -1,7 +1,7 @@ 
 [Unit]
 Description=Open vSwitch
-After=syslog.target network.target openvswitch-nonetwork.service
-Requires=openvswitch-nonetwork.service
+After=syslog.target network.target ovsdb-server.service
+Requires=ovsdb-server.service
 
 [Service]
 Type=oneshot
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
new file mode 100644
index 0000000..e4c2a66
--- /dev/null
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -0,0 +1,15 @@ 
+[Unit]
+Description=Open vSwitch Internal Unit
+After=syslog.target
+PartOf=openvswitch.service
+Wants=openvswitch.service
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+EnvironmentFile=-/etc/sysconfig/openvswitch
+ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
+          --system-id=random $OPTIONS
+ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
+RuntimeDirectory=openvswitch
+RuntimeDirectoryMode=0755