[ovs-dev,2/2] fedora: Handle upgrades from rhel package.
diff mbox series

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

Commit Message

Guru Shetty May 3, 2019, 7:05 a.m. UTC
Currently we have rhel/openvswitch.spec.in that provides
sysv scripts. The fedora package provides systemd scripts.
If one upgrades openvswitch package from sysv to systemd,
you will end up in a situation where old OVS daemons are
running, but systemd does not know about it.  One "restart"
is needed for systemd to see the old daemons. Another "restart"
or "force-reload-kmod" is needed to actually use the new
daemons.

This commit, just takes care of the first restart. The "real"
restart/force-reload-kmod will still have to be done outside
the package installation.

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

Comments

Aaron Conole May 3, 2019, 6:36 p.m. UTC | #1
Gurucharan Shetty <guru@ovn.org> writes:

> Currently we have rhel/openvswitch.spec.in that provides
> sysv scripts. The fedora package provides systemd scripts.
> If one upgrades openvswitch package from sysv to systemd,
> you will end up in a situation where old OVS daemons are
> running, but systemd does not know about it.  One "restart"
> is needed for systemd to see the old daemons. Another "restart"
> or "force-reload-kmod" is needed to actually use the new
> daemons.

Is this true?  I thought the restart would actually run the restart
action and that would spawn new instances of the daemons.  It seems like
a strange behavior.

> This commit, just takes care of the first restart. The "real"
> restart/force-reload-kmod will still have to be done outside
> the package installation.
>
> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
> ---

I'm still not clear on the reasoning to only do the 'restart' on
upgrade from the sysv style.  If we're going through the trouble to
auto-enable it seems confusing that the service gets enabled but not
started, but only sometimes.  Maybe it's best that if this autoenable
flag is set, the daemons are also spawned.

>  rhel/openvswitch-fedora.spec.in | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index e8165f9..d41d11c 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -364,6 +364,12 @@ getent passwd openvswitch >/dev/null || \
>      usermod -a -G hugetlbfs openvswitch
>  %endif
>  %endif
> +
> +%if %{with autoenable}
> +    if [ -x "/etc/init.d/openvswitch" ]; then
> +        touch %{_tmppath}/ovs-upgrade-from-sysv
> +    fi
> +%endif
>  exit 0
>  
>  %post
> @@ -397,6 +403,14 @@ fi
>  %if %{with autoenable}
>      systemctl daemon-reload
>      systemctl enable openvswitch
> +    # Handle upgrades to this package from the OVS repo's rhel packages.
> +    # One "restart" is needed for newer systemd files to see the old running
> +    # daemons. Another "restart" (outside the package postinst script) is
> +    # needed to actually run new daemons.
> +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
> +        systemctl restart openvswitch
> +        rm "%{_tmppath}/ovs-upgrade-from-sysv"
> +    fi
>  %endif
>  
>  %post selinux-policy
Guru Shetty May 3, 2019, 7:04 p.m. UTC | #2
On Fri, 3 May 2019 at 11:36, Aaron Conole <aconole@redhat.com> wrote:

> Gurucharan Shetty <guru@ovn.org> writes:
>
> > Currently we have rhel/openvswitch.spec.in that provides
> > sysv scripts. The fedora package provides systemd scripts.
> > If one upgrades openvswitch package from sysv to systemd,
> > you will end up in a situation where old OVS daemons are
> > running, but systemd does not know about it.  One "restart"
> > is needed for systemd to see the old daemons. Another "restart"
> > or "force-reload-kmod" is needed to actually use the new
> > daemons.
>
> Is this true?  I thought the restart would actually run the restart
> action and that would spawn new instances of the daemons.  It seems like
> a strange behavior.
>

systemd openvswitch scripts when installed, hasn't been started yet in its
history. So they do not know that any daemon has been started. So a "stop"
will not stop old daemons.


>
> > This commit, just takes care of the first restart. The "real"
> > restart/force-reload-kmod will still have to be done outside
> > the package installation.
> >
> > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
> > ---
>
> I'm still not clear on the reasoning to only do the 'restart' on
> upgrade from the sysv style.  If we're going through the trouble to
> auto-enable it seems confusing that the service gets enabled but not
> started, but only sometimes.  Maybe it's best that if this autoenable
> flag is set, the daemons are also spawned.
>
The "restart" being done here is only to make systemd aware of old daemons.
The old rhel rpm did not restart/force-reload-kmod old daemons either.
So this just brings fedora rpm to parity with rhel rpm. There is pre-built
infrastructure that handle restart vs force-reload-kmod for rhel based
openvswitch rpms. If we respawn daemons, it will cause them to get confused.


>
> >  rhel/openvswitch-fedora.spec.in | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
> openvswitch-fedora.spec.in
> > index e8165f9..d41d11c 100644
> > --- a/rhel/openvswitch-fedora.spec.in
> > +++ b/rhel/openvswitch-fedora.spec.in
> > @@ -364,6 +364,12 @@ getent passwd openvswitch >/dev/null || \
> >      usermod -a -G hugetlbfs openvswitch
> >  %endif
> >  %endif
> > +
> > +%if %{with autoenable}
> > +    if [ -x "/etc/init.d/openvswitch" ]; then
> > +        touch %{_tmppath}/ovs-upgrade-from-sysv
> > +    fi
> > +%endif
> >  exit 0
> >
> >  %post
> > @@ -397,6 +403,14 @@ fi
> >  %if %{with autoenable}
> >      systemctl daemon-reload
> >      systemctl enable openvswitch
> > +    # Handle upgrades to this package from the OVS repo's rhel packages.
> > +    # One "restart" is needed for newer systemd files to see the old
> running
> > +    # daemons. Another "restart" (outside the package postinst script)
> is
> > +    # needed to actually run new daemons.
> > +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
> > +        systemctl restart openvswitch
> > +        rm "%{_tmppath}/ovs-upgrade-from-sysv"
> > +    fi
> >  %endif
> >
> >  %post selinux-policy
>
Aaron Conole May 3, 2019, 8:05 p.m. UTC | #3
Guru Shetty <guru@ovn.org> writes:

> On Fri, 3 May 2019 at 11:36, Aaron Conole <aconole@redhat.com> wrote:
>
>  Gurucharan Shetty <guru@ovn.org> writes:
>
>  > Currently we have rhel/openvswitch.spec.in that provides
>  > sysv scripts. The fedora package provides systemd scripts.
>  > If one upgrades openvswitch package from sysv to systemd,
>  > you will end up in a situation where old OVS daemons are
>  > running, but systemd does not know about it.  One "restart"
>  > is needed for systemd to see the old daemons. Another "restart"
>  > or "force-reload-kmod" is needed to actually use the new
>  > daemons.
>
>  Is this true?  I thought the restart would actually run the restart
>  action and that would spawn new instances of the daemons.  It seems like
>  a strange behavior.
>
> systemd openvswitch scripts when installed, hasn't been started yet in its history. So they do not know that
> any daemon has been started. So a "stop" will not stop old daemons.

The restart does, though - right?  IIRC, the ExecStop= will be called
and terminate the old daemons first.  I just want to make sure that it's
documented if process will be terminated (via the ExecStop= action) so
that any audit log can be explained.  As it is written, the behavior
isn't clear whether the untracked daemons are stopped.

>  > This commit, just takes care of the first restart. The "real"
>  > restart/force-reload-kmod will still have to be done outside
>  > the package installation.
>  >
>  > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
>  > ---
>
>  I'm still not clear on the reasoning to only do the 'restart' on
>  upgrade from the sysv style.  If we're going through the trouble to
>  auto-enable it seems confusing that the service gets enabled but not
>  started, but only sometimes.  Maybe it's best that if this autoenable
>  flag is set, the daemons are also spawned.
>
> The "restart" being done here is only to make systemd aware of old daemons. The old rhel rpm did not
> restart/force-reload-kmod old daemons either. 
> So this just brings fedora rpm to parity with rhel rpm. There is pre-built infrastructure that handle restart vs
> force-reload-kmod for rhel based openvswitch rpms. If we respawn daemons, it will cause them to get
> confused.

I think the idea is to bring RHEL to match Fedora, though.  Not the
other way around.  Fedora is upstream for RHEL.

I still don't understand why on upgrade we will always enable ovs (even
if the user didn't want it), but not also always start it.  After all, a
system reboot will cause it to start.  Does it make sense to skip the
reboot step and just allow ovs to start?

>  >  rhel/openvswitch-fedora.spec.in | 14 ++++++++++++++
>  >  1 file changed, 14 insertions(+)
>  >
>  > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
>  > index e8165f9..d41d11c 100644
>  > --- a/rhel/openvswitch-fedora.spec.in
>  > +++ b/rhel/openvswitch-fedora.spec.in
>  > @@ -364,6 +364,12 @@ getent passwd openvswitch >/dev/null || \
>  >      usermod -a -G hugetlbfs openvswitch
>  >  %endif
>  >  %endif
>  > +
>  > +%if %{with autoenable}
>  > +    if [ -x "/etc/init.d/openvswitch" ]; then
>  > +        touch %{_tmppath}/ovs-upgrade-from-sysv
>  > +    fi
>  > +%endif
>  >  exit 0
>  >  
>  >  %post
>  > @@ -397,6 +403,14 @@ fi
>  >  %if %{with autoenable}
>  >      systemctl daemon-reload
>  >      systemctl enable openvswitch
>  > +    # Handle upgrades to this package from the OVS repo's rhel packages.
>  > +    # One "restart" is needed for newer systemd files to see the old running
>  > +    # daemons. Another "restart" (outside the package postinst script) is
>  > +    # needed to actually run new daemons.
>  > +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
>  > +        systemctl restart openvswitch
>  > +        rm "%{_tmppath}/ovs-upgrade-from-sysv"
>  > +    fi
>  >  %endif
>  >  
>  >  %post selinux-policy
Guru Shetty May 3, 2019, 8:22 p.m. UTC | #4
On Fri, 3 May 2019 at 13:05, Aaron Conole <aconole@redhat.com> wrote:

> Guru Shetty <guru@ovn.org> writes:
>
> > On Fri, 3 May 2019 at 11:36, Aaron Conole <aconole@redhat.com> wrote:
> >
> >  Gurucharan Shetty <guru@ovn.org> writes:
> >
> >  > Currently we have rhel/openvswitch.spec.in that provides
> >  > sysv scripts. The fedora package provides systemd scripts.
> >  > If one upgrades openvswitch package from sysv to systemd,
> >  > you will end up in a situation where old OVS daemons are
> >  > running, but systemd does not know about it.  One "restart"
> >  > is needed for systemd to see the old daemons. Another "restart"
> >  > or "force-reload-kmod" is needed to actually use the new
> >  > daemons.
> >
> >  Is this true?  I thought the restart would actually run the restart
> >  action and that would spawn new instances of the daemons.  It seems like
> >  a strange behavior.
> >
> > systemd openvswitch scripts when installed, hasn't been started yet in
> its history. So they do not know that
> > any daemon has been started. So a "stop" will not stop old daemons.
>
> The restart does, though - right?

No. It does not. Not when you run it the first time.



>   IIRC, the ExecStop= will be called
> and terminate the old daemons first.  I just want to make sure that it's
> documented if process will be terminated (via the ExecStop= action) so
> that any audit log can be explained.  As it is written, the behavior
> isn't clear whether the untracked daemons are stopped.
>

To stop the untracked daemons and start the new daemons, you need to run
"systemctl restart openvswitch" twice.


>
> >  > This commit, just takes care of the first restart. The "real"
> >  > restart/force-reload-kmod will still have to be done outside
> >  > the package installation.
> >  >
> >  > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
> >  > ---
> >
> >  I'm still not clear on the reasoning to only do the 'restart' on
> >  upgrade from the sysv style.  If we're going through the trouble to
> >  auto-enable it seems confusing that the service gets enabled but not
> >  started, but only sometimes.  Maybe it's best that if this autoenable
> >  flag is set, the daemons are also spawned.
> >
> > The "restart" being done here is only to make systemd aware of old
> daemons. The old rhel rpm did not
> > restart/force-reload-kmod old daemons either.
> > So this just brings fedora rpm to parity with rhel rpm. There is
> pre-built infrastructure that handle restart vs
> > force-reload-kmod for rhel based openvswitch rpms. If we respawn
> daemons, it will cause them to get
> > confused.
>
> I think the idea is to bring RHEL to match Fedora, though.  Not the
> other way around.  Fedora is upstream for RHEL.
>

The rpms in OVS repo need to maintain some kind of compatibility if the
expectation is for the user of OVS to smoothly transition from RHEL rpm to
fedora rpm - the goal being to retire RHEL rpm spec file. We should ideally
not tangle ourselves with semantics and make the user jump hoops to get
things to work.  If we have the 2 rpms behave differently, then you have a
nightmarish situation of having different set of instruction manuals for
different set of OVS versions.


>
> I still don't understand why on upgrade we will always enable ovs (even
> if the user didn't want it),

The user wants it. So he has passed the "--with autoenable" option while
building the rpm. If he did not want it, we would not have passed it.




> but not also always start it.  After all, a
> system reboot will cause it to start.  Does it make sense to skip the
> reboot step and just allow ovs to start?
>
> That is how RHEL rpm spec file that has been used behaved so far.  It will
enable openvswitch service and let the user start it (either via rebooting
or simply calling the "start" script or running a "force-reload-kmod" to
use the new kernel module. Fedora rpm needs to maintain that compatibility
if we need to make this transition.

The way we have used RHEL rpm so far is to install it (which enabled
openvswitch) and then see whether we need to do a "force-reload-kmod" or a
"restart" depending on the kernel module used.





> >  >  rhel/openvswitch-fedora.spec.in | 14 ++++++++++++++
> >  >  1 file changed, 14 insertions(+)
> >  >
> >  > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
> openvswitch-fedora.spec.in
> >  > index e8165f9..d41d11c 100644
> >  > --- a/rhel/openvswitch-fedora.spec.in
> >  > +++ b/rhel/openvswitch-fedora.spec.in
> >  > @@ -364,6 +364,12 @@ getent passwd openvswitch >/dev/null || \
> >  >      usermod -a -G hugetlbfs openvswitch
> >  >  %endif
> >  >  %endif
> >  > +
> >  > +%if %{with autoenable}
> >  > +    if [ -x "/etc/init.d/openvswitch" ]; then
> >  > +        touch %{_tmppath}/ovs-upgrade-from-sysv
> >  > +    fi
> >  > +%endif
> >  >  exit 0
> >  >
> >  >  %post
> >  > @@ -397,6 +403,14 @@ fi
> >  >  %if %{with autoenable}
> >  >      systemctl daemon-reload
> >  >      systemctl enable openvswitch
> >  > +    # Handle upgrades to this package from the OVS repo's rhel
> packages.
> >  > +    # One "restart" is needed for newer systemd files to see the old
> running
> >  > +    # daemons. Another "restart" (outside the package postinst
> script) is
> >  > +    # needed to actually run new daemons.
> >  > +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
> >  > +        systemctl restart openvswitch
> >  > +        rm "%{_tmppath}/ovs-upgrade-from-sysv"
> >  > +    fi
> >  >  %endif
> >  >
> >  >  %post selinux-policy
>
Ansis Atteka May 10, 2019, 12:19 a.m. UTC | #5
On Fri, 3 May 2019 at 11:19, Gurucharan Shetty <guru@ovn.org> wrote:
>
> Currently we have rhel/openvswitch.spec.in that provides
> sysv scripts. The fedora package provides systemd scripts.
> If one upgrades openvswitch package from sysv to systemd,
> you will end up in a situation where old OVS daemons are
> running, but systemd does not know about it.  One "restart"
> is needed for systemd to see the old daemons. Another "restart"
> or "force-reload-kmod" is needed to actually use the new
> daemons.
>
> This commit, just takes care of the first restart. The "real"
> restart/force-reload-kmod will still have to be done outside
> the package installation.
>
> Signed-off-by: Gurucharan Shetty <guru@ovn.org>

Do I understand correctly that Aaron has concern whether the
autoenable build flag that you introduced in patch 1/2 should
automatically enable openvswitch on every installation (opposed to
only first installation)? Did you look into how other fedora packages
that enable services on installation behave? I kinda see the point
that if admin explicitly disabled openvswitch then we should not
blindly re-enable openvswitch back...
> ---
>  rhel/openvswitch-fedora.spec.in | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index e8165f9..d41d11c 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -364,6 +364,12 @@ getent passwd openvswitch >/dev/null || \
>      usermod -a -G hugetlbfs openvswitch
>  %endif
>  %endif
> +
> +%if %{with autoenable}
> +    if [ -x "/etc/init.d/openvswitch" ]; then
> +        touch %{_tmppath}/ovs-upgrade-from-sysv
> +    fi
> +%endif
>  exit 0
>
>  %post
> @@ -397,6 +403,14 @@ fi
>  %if %{with autoenable}
>      systemctl daemon-reload
>      systemctl enable openvswitch
> +    # Handle upgrades to this package from the OVS repo's rhel packages.
> +    # One "restart" is needed for newer systemd files to see the old running
> +    # daemons. Another "restart" (outside the package postinst script) is
The double restart thingy seems kinda weird to me as well, but I don't
have insight why systemd behaves that way. Would calling systemctl
stop before daemon-reload and then systemctl [re]start after
daemon-reload solve the problem in a more elegant way? If not probably
this is not worth something worth to bother with.

> +    # needed to actually run new daemons.
> +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then

It seems that if rpm package that was built from rhel spec file was
removed (without upgrade) and then you do fresh install, then you hit
this upgrade path too. Unless the whole host was rebooted before fresh
install and all contents of /tmp directory were purged. Though this
may not be an issue after invoking the "sytemctl enable", right?

Do you think it may make sense to commit to remove this migration code
after couple releases? I kinda see this as a temporary solution that
is supposed to help admins with migrating from rhel spec files to
fedora spec files. Once everyone has migrated, then this code becomes
clutter.



> +        systemctl restart openvswitch
> +        rm "%{_tmppath}/ovs-upgrade-from-sysv"
> +    fi
>  %endif
>
>  %post selinux-policy
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ansis Atteka May 10, 2019, 12:36 a.m. UTC | #6
On Fri, 3 May 2019 at 11:19, Gurucharan Shetty <guru@ovn.org> wrote:
>
> Currently we have rhel/openvswitch.spec.in that provides
> sysv scripts. The fedora package provides systemd scripts.
> If one upgrades openvswitch package from sysv to systemd,
> you will end up in a situation where old OVS daemons are
> running, but systemd does not know about it.  One "restart"
> is needed for systemd to see the old daemons. Another "restart"
> or "force-reload-kmod" is needed to actually use the new
> daemons.
>
> This commit, just takes care of the first restart. The "real"
> restart/force-reload-kmod will still have to be done outside
> the package installation.
>
> Signed-off-by: Gurucharan Shetty <guru@ovn.org>

Do I understand correctly that Aaron has concern whether the
autoenable build flag that you introduced in patch 1/2 should
automatically enable openvswitch on every installation (opposed to
only first installation)? Did you look into how other fedora packages
that enable services on installation behave? I kinda see the point
that if admin explicitly disabled openvswitch then we should not
blindly re-enable openvswitch back...

> ---
>  rhel/openvswitch-fedora.spec.in | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index e8165f9..d41d11c 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -364,6 +364,12 @@ getent passwd openvswitch >/dev/null || \
>      usermod -a -G hugetlbfs openvswitch
>  %endif
>  %endif
> +
> +%if %{with autoenable}
> +    if [ -x "/etc/init.d/openvswitch" ]; then
> +        touch %{_tmppath}/ovs-upgrade-from-sysv
> +    fi
> +%endif
>  exit 0
>
>  %post
> @@ -397,6 +403,14 @@ fi
>  %if %{with autoenable}
>      systemctl daemon-reload
>      systemctl enable openvswitch
> +    # Handle upgrades to this package from the OVS repo's rhel packages.
> +    # One "restart" is needed for newer systemd files to see the old running

The double restart thingy seems kinda weird to me as well, but I don't
have insight why systemd behaves that way. Would calling systemctl
stop before daemon-reload and then systemctl [re]start after
daemon-reload solve the problem in a more elegant way? If not probably
this is not worth something worth to bother with.

> +    # daemons. Another "restart" (outside the package postinst script) is
> +    # needed to actually run new daemons.
> +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
t seems that if rpm package that was built from rhel spec file was
removed (without upgrade) and then you do fresh install, then you hit
this upgrade path too. Unless the whole host was rebooted before fresh
install and all contents of /tmp directory were purged. Though this
may not be an issue after invoking the "sytemctl enable", right?

Do you think it may make sense to commit to remove this migration code
after couple releases? I kinda see this as a temporary solution that
is supposed to help admins with migrating from rhel spec files to
fedora spec files. Once everyone has migrated, then this code becomes
clutter.
> +        systemctl restart openvswitch
> +        rm "%{_tmppath}/ovs-upgrade-from-sysv"
> +    fi
>  %endif
>
>  %post selinux-policy
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Guru Shetty May 10, 2019, 9 p.m. UTC | #7
On Thu, 9 May 2019 at 17:37, Ansis Atteka <ansisatteka@gmail.com> wrote:

> On Fri, 3 May 2019 at 11:19, Gurucharan Shetty <guru@ovn.org> wrote:
> >
> > Currently we have rhel/openvswitch.spec.in that provides
> > sysv scripts. The fedora package provides systemd scripts.
> > If one upgrades openvswitch package from sysv to systemd,
> > you will end up in a situation where old OVS daemons are
> > running, but systemd does not know about it.  One "restart"
> > is needed for systemd to see the old daemons. Another "restart"
> > or "force-reload-kmod" is needed to actually use the new
> > daemons.
> >
> > This commit, just takes care of the first restart. The "real"
> > restart/force-reload-kmod will still have to be done outside
> > the package installation.
> >
> > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
>
> Do I understand correctly that Aaron has concern whether the
> autoenable build flag that you introduced in patch 1/2 should
> automatically enable openvswitch on every installation (opposed to
> only first installation)? Did you look into how other fedora packages
> that enable services on installation behave?


Fedora packages seem to look at system-preset files in
/etc/systemd/system-preset/ to decide whether a package needs to be
enabled. But we don't provide such a file.


> I kinda see the point
> that if admin explicitly disabled openvswitch then we should not
> blindly re-enable openvswitch back...
>

The openvswitch rhel rpm so far has always enabled it after installation or
upgrade (from 2011). So there is some compatibility there. What we can do
here is that after a few releases, only enable openvswitch for a fresh
installation and not upgrade - unless we want it right now and special case
upgrades from pre-systemd to systemd.  I don't see what we are doing as a
particularly bad behavior as this is not a default option and will only
happen if someone actually builds the RPM with a specfic non-default option.



>
> > ---
> >  rhel/openvswitch-fedora.spec.in | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
> openvswitch-fedora.spec.in
> > index e8165f9..d41d11c 100644
> > --- a/rhel/openvswitch-fedora.spec.in
> > +++ b/rhel/openvswitch-fedora.spec.in
> > @@ -364,6 +364,12 @@ getent passwd openvswitch >/dev/null || \
> >      usermod -a -G hugetlbfs openvswitch
> >  %endif
> >  %endif
> > +
> > +%if %{with autoenable}
> > +    if [ -x "/etc/init.d/openvswitch" ]; then
> > +        touch %{_tmppath}/ovs-upgrade-from-sysv
> > +    fi
> > +%endif
> >  exit 0
> >
> >  %post
> > @@ -397,6 +403,14 @@ fi
> >  %if %{with autoenable}
> >      systemctl daemon-reload
> >      systemctl enable openvswitch
> > +    # Handle upgrades to this package from the OVS repo's rhel packages.
> > +    # One "restart" is needed for newer systemd files to see the old
> running
>
> The double restart thingy seems kinda weird to me as well, but I don't
> have insight why systemd behaves that way.


This is the behavior broken down into steps
1. Old OVS daemons are running and new systemd unit files are installed.
2. "systemctl status openvswitch" - says that it is inactive as it has
never been started.
3. "systemctl stop openvswitch" - does not attempt to stop anything because
openvswitch is "inactive"
4. "systemctl start openvswitch" - will try to attempt to start openvswitch
- but does not do anything - likely because it notices that the PID file is
active and daemons are still running. Now "openvswitch" is active.
5. "systemctl stop openvswitch" - will stop old daemons.
6. "systemctl start openvswitch" - will start new daemons.



> Would calling systemctl
> stop before daemon-reload and then systemctl [re]start after
> daemon-reload solve the problem in a more elegant way? If not probably
> this is not worth something worth to bother with.
>

I tried your suggestion and that does not work.



>
> > +    # daemons. Another "restart" (outside the package postinst script)
> is
> > +    # needed to actually run new daemons.
> > +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
> t seems that if rpm package that was built from rhel spec file was
> removed (without upgrade) and then you do fresh install, then you hit
> this upgrade path too. Unless the whole host was rebooted before fresh
> install and all contents of /tmp directory were purged. Though this
> may not be an issue after invoking the "sytemctl enable", right?
>

You are correct. I had not thought about it. The end result is that we have
a start of openvswitch daemons. I think I can fix this by making this code
only run during an upgrade. I will send a v2.


>
> Do you think it may make sense to commit to remove this migration code
> after couple releases? I kinda see this as a temporary solution that
> is supposed to help admins with migrating from rhel spec files to
> fedora spec files. Once everyone has migrated, then this code becomes
> clutter.
>
Yes. We can get rid of this code after a few releases.



> > +        systemctl restart openvswitch
> > +        rm "%{_tmppath}/ovs-upgrade-from-sysv"
> > +    fi
> >  %endif
> >
> >  %post selinux-policy
> > --
> > 1.9.1
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Guru Shetty May 10, 2019, 9:46 p.m. UTC | #8
>
>
>> > +    # daemons. Another "restart" (outside the package postinst script)
>> is
>> > +    # needed to actually run new daemons.
>> > +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
>> t seems that if rpm package that was built from rhel spec file was
>> removed (without upgrade) and then you do fresh install, then you hit
>> this upgrade path too. Unless the whole host was rebooted before fresh
>> install and all contents of /tmp directory were purged. Though this
>> may not be an issue after invoking the "sytemctl enable", right?
>>
>
> You are correct. I had not thought about it. The end result is that we
> have a start of openvswitch daemons. I think I can fix this by making this
> code only run during an upgrade. I will send a v2.
>
>
I may have misunderstood your comment. When we uninstall the previous
package and then we do a fresh install, the "/etc/init.d/openvswitch" will
not be there and we will not create %{_tmppath}/ovs-upgrade-from-sysv.  So
we will not restart openvswitch. I hope we are on the same page.
Ansis Atteka May 10, 2019, 10:13 p.m. UTC | #9
On Fri, 10 May 2019 at 14:47, Guru Shetty <guru@ovn.org> wrote:
>>>
>>>
>>> > +    # daemons. Another "restart" (outside the package postinst script) is
>>> > +    # needed to actually run new daemons.
>>> > +    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
>>> t seems that if rpm package that was built from rhel spec file was
>>> removed (without upgrade) and then you do fresh install, then you hit
>>> this upgrade path too. Unless the whole host was rebooted before fresh
>>> install and all contents of /tmp directory were purged. Though this
>>> may not be an issue after invoking the "sytemctl enable", right?
>>
>>
>> You are correct. I had not thought about it. The end result is that we have a start of openvswitch daemons. I think I can fix this by making this code only run during an upgrade. I will send a v2.
>>
>
> I may have misunderstood your comment. When we uninstall the previous package and then we do a fresh install, the "/etc/init.d/openvswitch" will not be there and we will not create %{_tmppath}/ovs-upgrade-from-sysv.  So we will not restart openvswitch. I hope we are on the same page.
>


You are right. The %pre of the new package creates this file in tmp.
For strange reasons I overlooked something and incorrectly assumed
that it is %preun of the old package that creates it (which would be
impossible btw because that is code we have already shipped). So:

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

Sorry for raising false alarm.

Patch
diff mbox series

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index e8165f9..d41d11c 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -364,6 +364,12 @@  getent passwd openvswitch >/dev/null || \
     usermod -a -G hugetlbfs openvswitch
 %endif
 %endif
+
+%if %{with autoenable}
+    if [ -x "/etc/init.d/openvswitch" ]; then
+        touch %{_tmppath}/ovs-upgrade-from-sysv
+    fi
+%endif
 exit 0
 
 %post
@@ -397,6 +403,14 @@  fi
 %if %{with autoenable}
     systemctl daemon-reload
     systemctl enable openvswitch
+    # Handle upgrades to this package from the OVS repo's rhel packages.
+    # One "restart" is needed for newer systemd files to see the old running
+    # daemons. Another "restart" (outside the package postinst script) is
+    # needed to actually run new daemons.
+    if [ -e "%{_tmppath}/ovs-upgrade-from-sysv" ]; then
+        systemctl restart openvswitch
+        rm "%{_tmppath}/ovs-upgrade-from-sysv"
+    fi
 %endif
 
 %post selinux-policy