diff mbox series

[ovs-dev,1/2] fedora: Ability to auto enable openvswitch service.

Message ID 1556867135-29383-1-git-send-email-guru@ovn.org
State Accepted
Commit dcd0f1e8779c6623188f1a36753242e27db06cbb
Headers show
Series [ovs-dev,1/2] fedora: Ability to auto enable openvswitch service. | expand

Commit Message

Gurucharan Shetty May 3, 2019, 7:05 a.m. UTC
We currently have rhel/openvswitch.spec.in that automatically
enables openvswitch service when the package is installed using
chkconfig.

But fedora rpm may not enable openvswitch service automatically.
The macro currently being used in fedora rpm (systemd_post) will
look for preset files in /etc/systemd/system-preset/ to figure
out whether openvswitch service needs to be automatically enabled.
But, the fedora package does not provide such a file. The argument
is that people may want to install the package for binaries and
not necessarily to run OVS.

If someone now wants to install the fedora package and automatically
enable openvswitch, he will have to create a new package that OVS
package depends on to install the preset file. This is unwieldy.

This commit, provides a rpm build time option to enable the openvswitch
service automatically. If you now run the below command, openvswitch
service will be automatically enabled during package installation.

make rpm-fedora RPMBUILD_OPT="--with autoenable"

Signed-off-by: Gurucharan Shetty <guru@ovn.org>
---
 rhel/openvswitch-fedora.spec.in | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Ansis May 9, 2019, 10:27 p.m. UTC | #1
On Fri, 3 May 2019 at 11:19, Gurucharan Shetty <guru@ovn.org> wrote:
>
> We currently have rhel/openvswitch.spec.in that automatically
> enables openvswitch service when the package is installed using
> chkconfig.
>
> But fedora rpm may not enable openvswitch service automatically.
> The macro currently being used in fedora rpm (systemd_post) will
> look for preset files in /etc/systemd/system-preset/ to figure
> out whether openvswitch service needs to be automatically enabled.
> But, the fedora package does not provide such a file. The argument
> is that people may want to install the package for binaries and
> not necessarily to run OVS.
>
> If someone now wants to install the fedora package and automatically
> enable openvswitch, he will have to create a new package that OVS
> package depends on to install the preset file. This is unwieldy.

Is this the preset list you are referring to -
https://src.fedoraproject.org/rpms/fedora-release/blob/master/f/90-default.preset
? Maybe Aaron can comment if openvswitch should get in that list?
>
> This commit, provides a rpm build time option to enable the openvswitch
> service automatically. If you now run the below command, openvswitch
> service will be automatically enabled during package installation.
>
> make rpm-fedora RPMBUILD_OPT="--with autoenable"
>
> Signed-off-by: Gurucharan Shetty <guru@ovn.org>

Since you have #ifdef'ed out your newly introduced code and not
changed the default behavior, then this patch should not affect the
packages distributed by Fedora. So:

Acked-by: Ansis Atteka <aatteka@ovn.org>



> ---
>  rhel/openvswitch-fedora.spec.in | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index ce728b4..e8165f9 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -32,6 +32,9 @@
>  # This is enabled by default for versions of the distribution that
>  # have Python 3 by default (Fedora > 22).
>  %bcond_with build_python3
> +# If there is a need to automatically enable the package after installation,
> +# specify the "--with autoenable"
> +%bcond_with autoenable
>
>  # Enable PIE, bz#955181
>  %global _hardened_build 1
> @@ -382,6 +385,7 @@ fi
>  %endif
>
>  %if 0%{?systemd_post:1}
> +    # This may not enable openvswitch service or do daemon-reload.
>      %systemd_post %{name}.service
>  %else
>      # Package install, not upgrade
> @@ -390,6 +394,11 @@ fi
>      fi
>  %endif
>
> +%if %{with autoenable}
> +    systemctl daemon-reload
> +    systemctl enable openvswitch
> +%endif
> +
>  %post selinux-policy
>  %selinux_modules_install -s targeted %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp
>
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Timothy Redaelli May 10, 2019, 3:13 p.m. UTC | #2
On Fri,  3 May 2019 00:05:34 -0700
Gurucharan Shetty <guru@ovn.org> wrote:

> We currently have rhel/openvswitch.spec.in that automatically
> enables openvswitch service when the package is installed using
> chkconfig.
> 
> But fedora rpm may not enable openvswitch service automatically.
> The macro currently being used in fedora rpm (systemd_post) will
> look for preset files in /etc/systemd/system-preset/ to figure
> out whether openvswitch service needs to be automatically enabled.
> But, the fedora package does not provide such a file. The argument
> is that people may want to install the package for binaries and
> not necessarily to run OVS.
> 
> If someone now wants to install the fedora package and automatically
> enable openvswitch, he will have to create a new package that OVS
> package depends on to install the preset file. This is unwieldy.
> 
> This commit, provides a rpm build time option to enable the openvswitch
> service automatically. If you now run the below command, openvswitch
> service will be automatically enabled during package installation.
> 
> make rpm-fedora RPMBUILD_OPT="--with autoenable"
> 
> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
> ---
>  rhel/openvswitch-fedora.spec.in | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index ce728b4..e8165f9 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -32,6 +32,9 @@
>  # This is enabled by default for versions of the distribution that
>  # have Python 3 by default (Fedora > 22).
>  %bcond_with build_python3
> +# If there is a need to automatically enable the package after installation,
> +# specify the "--with autoenable"
> +%bcond_with autoenable
>  
>  # Enable PIE, bz#955181
>  %global _hardened_build 1
> @@ -382,6 +385,7 @@ fi
>  %endif
>  
>  %if 0%{?systemd_post:1}
> +    # This may not enable openvswitch service or do daemon-reload.
>      %systemd_post %{name}.service
>  %else
>      # Package install, not upgrade
> @@ -390,6 +394,11 @@ fi
>      fi
>  %endif
>  
> +%if %{with autoenable}
> +    systemctl daemon-reload
> +    systemctl enable openvswitch
> +%endif
> +
>  %post selinux-policy
>  %selinux_modules_install -s targeted %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp
>  

Hi,
usually a package on Fedora/RHEL7+ is never enabled by default, unless
it's necessary to enable it for a really good reason.

So any Fedora/RHEL7+ user knows (and expect) that he need to manually
enable/start a service after it's installed.

Probably it's better to put something like (postun?):

if /sbin/chkconfig --level 3 openvswitch; then                                     
    /bin/systemctl enable openvswitch.service >/dev/null 2>&1 || :
    /bin/systemctl restart openvswitch.service >/dev/null 2>&1 || :
fi

Just to enable it if it was enabled before on sysv?

I didn't tested it, but usually (on Fedora spec files) it's the correct
way to do that.

Thank you
Gurucharan Shetty May 10, 2019, 6:18 p.m. UTC | #3
>
>
> Hi,
> usually a package on Fedora/RHEL7+ is never enabled by default, unless
> it's necessary to enable it for a really good reason.
>

I would like to put an argument on why this a good reason in this case.

When I did a grep on 2 large repositories at VMware that use openvswitch
RPMs, there is a lot of test and dev code and places where there is an
assumption made that the openvswitch rpms enable services after
installation (which has been the case so far with rhel/openvswitch.spec)
and there is a history of 10 years there. So as a large user of openvswitch
RPMs, I should not have to go and change every bit of code that uses
openvswitch rpms to change the previous behavior.

And the code changes here does not get activated by default. Just provides
an option for openvswitch users to enable the service automatically if they
so desire.



>
> So any Fedora/RHEL7+ user knows (and expect) that he need to manually
> enable/start a service after it's installed.
>

> Probably it's better to put something like (postun?):
>
> if /sbin/chkconfig --level 3 openvswitch; then
>
>     /bin/systemctl enable openvswitch.service >/dev/null 2>&1 || :
>     /bin/systemctl restart openvswitch.service >/dev/null 2>&1 || :
> fi
>

I would like to have the ability to auto enable service even when it is a
new installation.

Thanks
diff mbox series

Patch

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index ce728b4..e8165f9 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -32,6 +32,9 @@ 
 # This is enabled by default for versions of the distribution that
 # have Python 3 by default (Fedora > 22).
 %bcond_with build_python3
+# If there is a need to automatically enable the package after installation,
+# specify the "--with autoenable"
+%bcond_with autoenable
 
 # Enable PIE, bz#955181
 %global _hardened_build 1
@@ -382,6 +385,7 @@  fi
 %endif
 
 %if 0%{?systemd_post:1}
+    # This may not enable openvswitch service or do daemon-reload.
     %systemd_post %{name}.service
 %else
     # Package install, not upgrade
@@ -390,6 +394,11 @@  fi
     fi
 %endif
 
+%if %{with autoenable}
+    systemctl daemon-reload
+    systemctl enable openvswitch
+%endif
+
 %post selinux-policy
 %selinux_modules_install -s targeted %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp