[ovs-dev] [PATCH] ovs-ctl: Don't set hostname as external-id

[Daniel Alvarez Sanchez]

On Thu, May 21, 2020 at 9:22 AM Han Zhou <hzhou at ovn.org> wrote:
>
>
>
> On Wed, May 20, 2020 at 8:52 AM Daniel Alvarez <dalvarez at redhat.com> wrote:
> >
> > ovs-ctl started to add the hostname as external-id [0] at some point.
> >
> > However, this can be problematic as if it's already set by an external
> > entity it will get overwritten. In RHEL systems, systemd will invoke
> > ovs-ctl to start OVS and that will overwrite it to the hostname of the
> > machine.
> >
> > For OVN this can have a big impact because if, for whatever reason the
> > hostname changes and the host gets restarted, ovn-controller won't
> > claim the ports back leaving the workloads unaccessible.
> >
> > Also, it makes sense to leave this untouched as 1) it's an external_id,
> > so it will actually let external entities to configure it (unlike now),
> > and 2) it's optional. In the case of OVN, if the external-id doesn't
> > exist, it'll default to its hostname so nothing should get broken by
> > this change.
> >
> > [0] https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/312054.html
> >
> > Signed-off-by: Daniel Alvarez <dalvarez at redhat.com>
> > ---
> >  utilities/ovs-ctl.in | 12 ------------
> >  1 file changed, 12 deletions(-)
> >
> > diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> > index 8c5cd7032..87fc4934f 100644
> > --- a/utilities/ovs-ctl.in
> > +++ b/utilities/ovs-ctl.in
> > @@ -35,17 +35,6 @@ insert_mod_if_required () {
> >      ovs_kmod_ctl insert
> >  }
> >
> > -set_hostname () {
> > -    # 'hostname -f' needs network connectivity to work.  So we should
> > -    # call this only after ovs-vswitchd is running.
> > -    if test X$FULL_HOSTNAME = Xyes; then
> > -        hn="$(hostname -f)" || hn="$(uname -n)"
> > -    else
> > -        hn="$(uname -n)"
> > -    fi
> > -    ovs_vsctl set Open_vSwitch . external-ids:hostname="$hn"
> > -}
> > -
> >  set_system_ids () {
> >      set ovs_vsctl set Open_vSwitch .
> >
> > @@ -225,7 +214,6 @@ start_forwarding () {
> >      if test X"$OVS_VSWITCHD" = Xyes; then
> >          do_start_forwarding || return 1
> >      fi
> > -    set_hostname &
> >      return 0
> >  }
> >
> > --
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Hi Daniel,
>
> I believe there are OpenStack based OVN users already depends on this. (And we had also add the --no-full-hostname option so that it will set the short name, so that it matches with openstack's "requested-chassis" setting which uses nova compute node name.) For the scenarios that this behavior is not desired, I think it is better to add a new option to override it, such as "--no-hostname", so that existing environment won't get impacted. What do you think?

Hi Han, thanks a lot for your feedback!
I thought of adding a --no-hostname option. However I still see that
this patch has value. Let me try to explain.

Existing OpenStack deployments will have their 'requested-chassis' set
to the current name of the hosts at the time the VMs were created.
This hostname may or may not match the machine hostname as it's a
string consumed from the external_ids, hence external and expected to
be set by CMS (for example, it's configurable by puppet at OpenStack
deployment time).
Now let's imagine you restart one node so ovs-ctl will overwrite
whatever the CMS relied upon and set it to the machine hostname. From
this time on, the workloads on that VM will not be claimed by OVN and
are left inaccessible until manual intervention happens.

I think it's fundamentally wrong for OVS (ovs-ctl in this case) to set
an external id itself as its default behavior (as they're meant for
external entities IIUC). Can you please elaborate on how existing
environments can get impacted by this change? Also keep in mind that
if the external_id is not set, ovn-controller is taking the hostname
of the machine as the default [0]. I might be overlooking something,
for sure.

Thanks again!
Daniel

[0] https://github.com/ovn-org/ovn/blob/master/controller/chassis.c#L133

>
> Thanks,
> Han