[ovs-dev] rhel: Install the network scripts in a new subpackage

Message ID c92bc523316442fb532457972577e2315f9be092.1533660730.git.tredaelli@redhat.com
State Accepted
Headers show
Series
  • [ovs-dev] rhel: Install the network scripts in a new subpackage
Related show

Commit Message

Timothy Redaelli Aug. 7, 2018, 4:52 p.m.
Starting from Fedora 29, the legacy network scripts are installed in
the "network-scripts" package and so the network scripts ("ifup-ovs",
"ifdown-ovs") should be installed only when the "network-scripts" package
is installed.

This commit introduces (on Fedora 29+) a new subpackage
(network-scripts-openvswitch). This subpackage is installed, by default, only
if the "network-scripts" package is installed too (reverse weak dependency).

Reported-by: Lubomir Rintel <lkundrak@v3.sk>
Reported-at: https://src.fedoraproject.org/rpms/openvswitch/pull-request/4
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
---
 rhel/openvswitch-fedora.spec.in | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Flavio Leitner Aug. 8, 2018, 4:38 p.m. | #1
On Tue, Aug 07, 2018 at 06:52:10PM +0200, Timothy Redaelli wrote:
> Starting from Fedora 29, the legacy network scripts are installed in
> the "network-scripts" package and so the network scripts ("ifup-ovs",
> "ifdown-ovs") should be installed only when the "network-scripts" package
> is installed.
> 
> This commit introduces (on Fedora 29+) a new subpackage
> (network-scripts-openvswitch). This subpackage is installed, by default, only
> if the "network-scripts" package is installed too (reverse weak dependency).

The package's name is following a new format that other projects are
using too.

The dnf installs weak dependencies unless told otherwise.

It requires the network-scripts if installed.

Conditionals look okay.

Looks correct if I read the guidelines correctly:
https://fedoraproject.org/wiki/Packaging:Guidelines 

It build the rpms correctly over here.

Acked-by: Flavio Leitner <fbl@sysclose.org>

Thanks Timothy and Lubomir.
fbl

> Reported-by: Lubomir Rintel <lkundrak@v3.sk>
> Reported-at: https://src.fedoraproject.org/rpms/openvswitch/pull-request/4
> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> ---
>  rhel/openvswitch-fedora.spec.in | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index 9f8664e95..eead10069 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -162,6 +162,18 @@ Provides: openvswitch-static = %{version}-%{release}
>  This provides static library, libopenswitch.a and the openvswitch header
>  files needed to build an external application.
>  
> +%if 0%{?rhel} > 7 || 0%{?fedora} > 28
> +%package -n network-scripts-%{name}
> +Summary: Open vSwitch legacy network service support
> +License: ASL 2.0
> +Requires: network-scripts
> +Supplements: (%{name} and network-scripts)
> +
> +%description -n network-scripts-%{name}
> +This provides the ifup and ifdown scripts for use with the legacy network
> +service.
> +%endif
> +
>  %package ovn-central
>  Summary: Open vSwitch - Open Virtual Network support
>  License: ASL 2.0
> @@ -529,6 +541,12 @@ fi
>  %{_includedir}/openflow/*
>  %{_includedir}/ovn/*
>  
> +%if 0%{?rhel} > 7 || 0%{?fedora} > 28
> +%files -n network-scripts-%{name}
> +%{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
> +%{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> +%endif
> +
>  %files
>  %defattr(-,openvswitch,openvswitch)
>  %dir %{_sysconfdir}/openvswitch
> @@ -546,8 +564,10 @@ fi
>  %{_unitdir}/ovs-vswitchd.service
>  %{_unitdir}/ovs-delete-transient-ports.service
>  %{_datadir}/openvswitch/scripts/openvswitch.init
> +%if ! (0%{?rhel} > 7 || 0%{?fedora} > 28)
>  %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
>  %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> +%endif
>  %{_datadir}/openvswitch/bugtool-plugins/
>  %{_datadir}/openvswitch/scripts/ovs-bugtool-*
>  %{_datadir}/openvswitch/scripts/ovs-check-dead-ifs
> -- 
> 2.17.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Flavio Leitner Aug. 8, 2018, 4:49 p.m. | #2
Actually, we also have dependencies in the systemd service
to the network service:

[Unit]                                                                                              
Description=Open vSwitch Forwarding Unit
After=ovsdb-server.service network-pre.target systemd-udev-settle.service
Before=network.target network.service
                      ^^^^^^^^^^^^^^^ 

Looks like that would be gone, right?

fbl

On Wed, Aug 08, 2018 at 01:38:07PM -0300, Flavio Leitner wrote:
> On Tue, Aug 07, 2018 at 06:52:10PM +0200, Timothy Redaelli wrote:
> > Starting from Fedora 29, the legacy network scripts are installed in
> > the "network-scripts" package and so the network scripts ("ifup-ovs",
> > "ifdown-ovs") should be installed only when the "network-scripts" package
> > is installed.
> > 
> > This commit introduces (on Fedora 29+) a new subpackage
> > (network-scripts-openvswitch). This subpackage is installed, by default, only
> > if the "network-scripts" package is installed too (reverse weak dependency).
> 
> The package's name is following a new format that other projects are
> using too.
> 
> The dnf installs weak dependencies unless told otherwise.
> 
> It requires the network-scripts if installed.
> 
> Conditionals look okay.
> 
> Looks correct if I read the guidelines correctly:
> https://fedoraproject.org/wiki/Packaging:Guidelines 
> 
> It build the rpms correctly over here.
> 
> Acked-by: Flavio Leitner <fbl@sysclose.org>
> 
> Thanks Timothy and Lubomir.
> fbl
> 
> > Reported-by: Lubomir Rintel <lkundrak@v3.sk>
> > Reported-at: https://src.fedoraproject.org/rpms/openvswitch/pull-request/4
> > Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> > ---
> >  rhel/openvswitch-fedora.spec.in | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> > index 9f8664e95..eead10069 100644
> > --- a/rhel/openvswitch-fedora.spec.in
> > +++ b/rhel/openvswitch-fedora.spec.in
> > @@ -162,6 +162,18 @@ Provides: openvswitch-static = %{version}-%{release}
> >  This provides static library, libopenswitch.a and the openvswitch header
> >  files needed to build an external application.
> >  
> > +%if 0%{?rhel} > 7 || 0%{?fedora} > 28
> > +%package -n network-scripts-%{name}
> > +Summary: Open vSwitch legacy network service support
> > +License: ASL 2.0
> > +Requires: network-scripts
> > +Supplements: (%{name} and network-scripts)
> > +
> > +%description -n network-scripts-%{name}
> > +This provides the ifup and ifdown scripts for use with the legacy network
> > +service.
> > +%endif
> > +
> >  %package ovn-central
> >  Summary: Open vSwitch - Open Virtual Network support
> >  License: ASL 2.0
> > @@ -529,6 +541,12 @@ fi
> >  %{_includedir}/openflow/*
> >  %{_includedir}/ovn/*
> >  
> > +%if 0%{?rhel} > 7 || 0%{?fedora} > 28
> > +%files -n network-scripts-%{name}
> > +%{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
> > +%{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> > +%endif
> > +
> >  %files
> >  %defattr(-,openvswitch,openvswitch)
> >  %dir %{_sysconfdir}/openvswitch
> > @@ -546,8 +564,10 @@ fi
> >  %{_unitdir}/ovs-vswitchd.service
> >  %{_unitdir}/ovs-delete-transient-ports.service
> >  %{_datadir}/openvswitch/scripts/openvswitch.init
> > +%if ! (0%{?rhel} > 7 || 0%{?fedora} > 28)
> >  %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
> >  %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> > +%endif
> >  %{_datadir}/openvswitch/bugtool-plugins/
> >  %{_datadir}/openvswitch/scripts/ovs-bugtool-*
> >  %{_datadir}/openvswitch/scripts/ovs-check-dead-ifs
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> -- 
> Flavio
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Timothy Redaelli Aug. 9, 2018, 9:50 a.m. | #3
On Wed, 8 Aug 2018 13:49:12 -0300
Flavio Leitner <fbl@sysclose.org> wrote:

> Actually, we also have dependencies in the systemd service
> to the network service:
> 
> [Unit]                                                                                              
> Description=Open vSwitch Forwarding Unit
> After=ovsdb-server.service network-pre.target
> systemd-udev-settle.service Before=network.target network.service
>                       ^^^^^^^^^^^^^^^ 
> 
> Looks like that would be gone, right?

I checked and "network.service" is an auto-generated service
(systemd-sysv-generator - Unit generator for SysV init scripts)
from /etc/rc.d/init.d/network that brings Up and Down the interfaces
(if you don't use Network Manager).

The explicit dependency is needed in order to avoid having it to run
before openvswitch / ovsdb-server.
Moreover it's also harmless to have it when network.service doesn't
exists since, in this case, systemd will only ignore the dependency.

For example, NetworkManager.service has "Before=network.target
network.service" on Fedora Rawhide too, since Fedora Rawhide still have
the support for the legacy network scripts (/etc/rc.d/init.d/network, in
network-scripts package).

> fbl
Flavio Leitner Aug. 10, 2018, 12:16 a.m. | #4
On Thu, Aug 09, 2018 at 11:50:38AM +0200, Timothy Redaelli wrote:
> On Wed, 8 Aug 2018 13:49:12 -0300
> Flavio Leitner <fbl@sysclose.org> wrote:
> 
> > Actually, we also have dependencies in the systemd service
> > to the network service:
> > 
> > [Unit]                                                                                              
> > Description=Open vSwitch Forwarding Unit
> > After=ovsdb-server.service network-pre.target
> > systemd-udev-settle.service Before=network.target network.service
> >                       ^^^^^^^^^^^^^^^ 
> > 
> > Looks like that would be gone, right?
> 
> I checked and "network.service" is an auto-generated service
> (systemd-sysv-generator - Unit generator for SysV init scripts)
> from /etc/rc.d/init.d/network that brings Up and Down the interfaces
> (if you don't use Network Manager).
> 
> The explicit dependency is needed in order to avoid having it to run
> before openvswitch / ovsdb-server.
> Moreover it's also harmless to have it when network.service doesn't
> exists since, in this case, systemd will only ignore the dependency.
>
> For example, NetworkManager.service has "Before=network.target
> network.service" on Fedora Rawhide too, since Fedora Rawhide still have
> the support for the legacy network scripts (/etc/rc.d/init.d/network, in
> network-scripts package).

Alright, thanks for looking into this.

Acked-by: Flavio Leitner <fbl@sysclose.org>

Thanks Timothy.
Ben Pfaff Aug. 10, 2018, 6:19 p.m. | #5
On Thu, Aug 09, 2018 at 09:16:16PM -0300, Flavio Leitner wrote:
> On Thu, Aug 09, 2018 at 11:50:38AM +0200, Timothy Redaelli wrote:
> > On Wed, 8 Aug 2018 13:49:12 -0300
> > Flavio Leitner <fbl@sysclose.org> wrote:
> > 
> > > Actually, we also have dependencies in the systemd service
> > > to the network service:
> > > 
> > > [Unit]                                                                                              
> > > Description=Open vSwitch Forwarding Unit
> > > After=ovsdb-server.service network-pre.target
> > > systemd-udev-settle.service Before=network.target network.service
> > >                       ^^^^^^^^^^^^^^^ 
> > > 
> > > Looks like that would be gone, right?
> > 
> > I checked and "network.service" is an auto-generated service
> > (systemd-sysv-generator - Unit generator for SysV init scripts)
> > from /etc/rc.d/init.d/network that brings Up and Down the interfaces
> > (if you don't use Network Manager).
> > 
> > The explicit dependency is needed in order to avoid having it to run
> > before openvswitch / ovsdb-server.
> > Moreover it's also harmless to have it when network.service doesn't
> > exists since, in this case, systemd will only ignore the dependency.
> >
> > For example, NetworkManager.service has "Before=network.target
> > network.service" on Fedora Rawhide too, since Fedora Rawhide still have
> > the support for the legacy network scripts (/etc/rc.d/init.d/network, in
> > network-scripts package).
> 
> Alright, thanks for looking into this.
> 
> Acked-by: Flavio Leitner <fbl@sysclose.org>
> 
> Thanks Timothy.

Thanks Timothy (and Flavio).  Applied to master.  Let me know if you
want it in branch-2.10 (or earlier?).

Patch

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 9f8664e95..eead10069 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -162,6 +162,18 @@  Provides: openvswitch-static = %{version}-%{release}
 This provides static library, libopenswitch.a and the openvswitch header
 files needed to build an external application.
 
+%if 0%{?rhel} > 7 || 0%{?fedora} > 28
+%package -n network-scripts-%{name}
+Summary: Open vSwitch legacy network service support
+License: ASL 2.0
+Requires: network-scripts
+Supplements: (%{name} and network-scripts)
+
+%description -n network-scripts-%{name}
+This provides the ifup and ifdown scripts for use with the legacy network
+service.
+%endif
+
 %package ovn-central
 Summary: Open vSwitch - Open Virtual Network support
 License: ASL 2.0
@@ -529,6 +541,12 @@  fi
 %{_includedir}/openflow/*
 %{_includedir}/ovn/*
 
+%if 0%{?rhel} > 7 || 0%{?fedora} > 28
+%files -n network-scripts-%{name}
+%{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
+%{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
+%endif
+
 %files
 %defattr(-,openvswitch,openvswitch)
 %dir %{_sysconfdir}/openvswitch
@@ -546,8 +564,10 @@  fi
 %{_unitdir}/ovs-vswitchd.service
 %{_unitdir}/ovs-delete-transient-ports.service
 %{_datadir}/openvswitch/scripts/openvswitch.init
+%if ! (0%{?rhel} > 7 || 0%{?fedora} > 28)
 %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
 %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
+%endif
 %{_datadir}/openvswitch/bugtool-plugins/
 %{_datadir}/openvswitch/scripts/ovs-bugtool-*
 %{_datadir}/openvswitch/scripts/ovs-check-dead-ifs