diff mbox series

[ovs-dev] ovs-ctl: Allow recording hostname separately

Message ID 20210216163225.BBA39868AA@whitealder.osuosl.org
State Superseded
Headers show
Series [ovs-dev] ovs-ctl: Allow recording hostname separately | expand

Commit Message

Frode Nordahl Feb. 16, 2021, 2:30 p.m. UTC
ovs-ctl determines the system FQDN or hostname and records it in
the `external-ids:hostname` field of the `Open-vSwitch` table on
system startup.

This value may be consumed by downstream software and having it
unset or set to a incorrect value could lead to erratic behavior
of a system.

When a system is configured to use a Open vSwitch controlled
datapath as its only network connection, the current ordering of
events would always produce a unreliable hostname

Reported-At: https://bugs.launchpad.net/bugs/1915829
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
---
 utilities/ovs-ctl.in | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Christian Ehrhardt Feb. 19, 2021, 8:10 a.m. UTC | #1
On Tue, Feb 16, 2021 at 5:32 PM Frode Nordahl
<frode.nordahl@canonical.com> wrote:
>
> ovs-ctl determines the system FQDN or hostname and records it in
> the `external-ids:hostname` field of the `Open-vSwitch` table on
> system startup.
>
> This value may be consumed by downstream software and having it
> unset or set to a incorrect value could lead to erratic behavior
> of a system.
>
> When a system is configured to use a Open vSwitch controlled
> datapath as its only network connection, the current ordering of
> events would always produce a unreliable hostname
>
> Reported-At: https://bugs.launchpad.net/bugs/1915829
> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>

I've looked at this for the Ubuntu problem that started Frodes work,
and while I can't give any authoritative Ack I can at least say

Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

> ---
>  utilities/ovs-ctl.in | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index be3aa083b..9231ae4f4 100644
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -230,7 +230,9 @@ start_forwarding () {
>      if test X"$OVS_VSWITCHD" = Xyes; then
>          do_start_forwarding || return 1
>      fi
> -    set_hostname &
> +    if test X"$RECORD_HOSTNAME" = Xyes; then
> +        set_hostname &
> +    fi
>      return 0
>  }
>
> @@ -321,6 +323,7 @@ set_defaults () {
>      SYSTEM_ID=
>
>      FULL_HOSTNAME=yes
> +    RECORD_HOSTNAME=yes
>
>      DELETE_BRIDGES=no
>      DELETE_TRANSIENT_PORTS=no
> @@ -394,6 +397,8 @@ Commands:
>    delete-transient-ports  delete transient (other_config:transient=true) ports
>    start-ovs-ipsec         start Open vSwitch ipsec daemon
>    stop-ovs-ipsec          stop Open vSwitch ipsec daemon
> +  record-hostname         determine the system hostname and record it in the
> +                          Open vSwitch database if not already set
>    help                    display this help message
>
>  One of the following options is required for "start", "restart" and "force-reload-kmod":
> @@ -415,6 +420,8 @@ Less important options for "start", "restart" and "force-reload-kmod":
>    --ovsdb-server-priority=NICE   set ovsdb-server's niceness (default: $OVSDB_SERVER_PRIORITY)
>    --ovs-vswitchd-priority=NICE   set ovs-vswitchd's niceness (default: $OVS_VSWITCHD_PRIORITY)
>    --no-full-hostname             set short hostname instead of full hostname
> +  --no-record-hostname           do not attempt to determine/record system
> +                                 hostname as part of start command
>
>  Debugging options for "start", "restart" and "force-reload-kmod":
>    --ovsdb-server-wrapper=WRAPPER
> @@ -573,6 +580,9 @@ case $command in
>      stop-ovs-ipsec)
>          stop_ovs_ipsec
>          ;;
> +    record-hostname)
> +        set_hostname
> +        ;;
>      help)
>          usage
>          ;;
> --
> 2.30.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ilya Maximets Feb. 24, 2021, 4:36 p.m. UTC | #2
On 2/19/21 9:10 AM, Christian Ehrhardt wrote:
> On Tue, Feb 16, 2021 at 5:32 PM Frode Nordahl
> <frode.nordahl@canonical.com> wrote:
>>
>> ovs-ctl determines the system FQDN or hostname and records it in
>> the `external-ids:hostname` field of the `Open-vSwitch` table on
>> system startup.
>>
>> This value may be consumed by downstream software and having it
>> unset or set to a incorrect value could lead to erratic behavior
>> of a system.
>>
>> When a system is configured to use a Open vSwitch controlled
>> datapath as its only network connection, the current ordering of
>> events would always produce a unreliable hostname
>>
>> Reported-At: https://bugs.launchpad.net/bugs/1915829
>> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
> 
> I've looked at this for the Ubuntu problem that started Frodes work,
> and while I can't give any authoritative Ack I can at least say
> 
> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> 

Hi.  Reading the description and the bug on launchpad it seems that
the issue is that hostname in database changes in unexpected way and
this is somewhat similar to what the following patch tried to fix:
  http://patchwork.ozlabs.org/project/openvswitch/patch/20200525152821.19838-1-dalvarez@redhat.com/

Is it still a problem with above patch applied?

Best regards, Ilya Maximets.
Frode Nordahl Feb. 25, 2021, 5:56 a.m. UTC | #3
On Wed, Feb 24, 2021 at 5:36 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 2/19/21 9:10 AM, Christian Ehrhardt wrote:
> > On Tue, Feb 16, 2021 at 5:32 PM Frode Nordahl
> > <frode.nordahl@canonical.com> wrote:
> >>
> >> ovs-ctl determines the system FQDN or hostname and records it in
> >> the `external-ids:hostname` field of the `Open-vSwitch` table on
> >> system startup.
> >>
> >> This value may be consumed by downstream software and having it
> >> unset or set to a incorrect value could lead to erratic behavior
> >> of a system.
> >>
> >> When a system is configured to use a Open vSwitch controlled
> >> datapath as its only network connection, the current ordering of
> >> events would always produce a unreliable hostname
> >>
> >> Reported-At: https://bugs.launchpad.net/bugs/1915829
> >> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
> >
> > I've looked at this for the Ubuntu problem that started Frodes work,
> > and while I can't give any authoritative Ack I can at least say
> >
> > Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> >
>
> Hi.  Reading the description and the bug on launchpad it seems that
> the issue is that hostname in database changes in unexpected way and
> this is somewhat similar to what the following patch tried to fix:
>   http://patchwork.ozlabs.org/project/openvswitch/patch/20200525152821.19838-1-dalvarez@redhat.com/

Hello Ilya, and thank you for the review.

> Is it still a problem with above patch applied?

Yes, while the above patch is welcome and adds to the stability of the
identifier, it does not solve the problem this patch addresses.

What we see is that the data collected for the initial recording of
the hostname field is incorrect. This occurs because the information
might not be available at the time of initial Open vSwitch startup.

We support the use of Open vSwitch to manage the only connection a
computer has to a network, and to do that we start Open vSwitch early
as part of the network target (in systemd terms). At this point in
time there may be no network connectivity yet, depending on deployment
type initial system configuration may not yet be applied (i.e.
/etc/hostname may hold a default unconfigured value), FQDN hints may
not yet be recorded in /etc/hosts etc.

We seek to resolve this class of issues by breaking the hostname
recording part out into a separate oneshot service [0] run after the
network is online.

To help support that we need to be able to manage whether and when
`ovs-ctl` performs the recording of the hostname.

0: https://code.launchpad.net/~fnordahl/ubuntu/+source/openvswitch/+git/openvswitch/+merge/398174
Ilya Maximets Feb. 25, 2021, 12:33 p.m. UTC | #4
On 2/25/21 6:56 AM, Frode Nordahl wrote:
> On Wed, Feb 24, 2021 at 5:36 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>>
>> On 2/19/21 9:10 AM, Christian Ehrhardt wrote:
>>> On Tue, Feb 16, 2021 at 5:32 PM Frode Nordahl
>>> <frode.nordahl@canonical.com> wrote:
>>>>
>>>> ovs-ctl determines the system FQDN or hostname and records it in
>>>> the `external-ids:hostname` field of the `Open-vSwitch` table on
>>>> system startup.
>>>>
>>>> This value may be consumed by downstream software and having it
>>>> unset or set to a incorrect value could lead to erratic behavior
>>>> of a system.
>>>>
>>>> When a system is configured to use a Open vSwitch controlled
>>>> datapath as its only network connection, the current ordering of
>>>> events would always produce a unreliable hostname
>>>>
>>>> Reported-At: https://bugs.launchpad.net/bugs/1915829
>>>> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
>>>
>>> I've looked at this for the Ubuntu problem that started Frodes work,
>>> and while I can't give any authoritative Ack I can at least say
>>>
>>> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>>>
>>
>> Hi.  Reading the description and the bug on launchpad it seems that
>> the issue is that hostname in database changes in unexpected way and
>> this is somewhat similar to what the following patch tried to fix:
>>   http://patchwork.ozlabs.org/project/openvswitch/patch/20200525152821.19838-1-dalvarez@redhat.com/
> 
> Hello Ilya, and thank you for the review.
> 
>> Is it still a problem with above patch applied?
> 
> Yes, while the above patch is welcome and adds to the stability of the
> identifier, it does not solve the problem this patch addresses.
> 
> What we see is that the data collected for the initial recording of
> the hostname field is incorrect. This occurs because the information
> might not be available at the time of initial Open vSwitch startup.
> 
> We support the use of Open vSwitch to manage the only connection a
> computer has to a network, and to do that we start Open vSwitch early
> as part of the network target (in systemd terms). At this point in
> time there may be no network connectivity yet, depending on deployment
> type initial system configuration may not yet be applied (i.e.
> /etc/hostname may hold a default unconfigured value), FQDN hints may
> not yet be recorded in /etc/hosts etc.
> 
> We seek to resolve this class of issues by breaking the hostname
> recording part out into a separate oneshot service [0] run after the
> network is online.
> 
> To help support that we need to be able to manage whether and when
> `ovs-ctl` performs the recording of the hostname.
> 
> 0: https://code.launchpad.net/~fnordahl/ubuntu/+source/openvswitch/+git/openvswitch/+merge/398174
> 

Thanks for clarification.  I see your point and I think it's OK to
have this change.  One comment though: with this change 'record-hostname'
command will not do anything if hostname was already configured.
While this behavior is described in a 'help' message, this looks a bit
counterintuitive anyway.  We should, probably, force it to re-write
currently set value or rename somehow, e.g. 'record-hostname-if-not-set'
or 'your better suggestion here'.

What do you think?
Frode Nordahl Feb. 25, 2021, 1:34 p.m. UTC | #5
On Thu, Feb 25, 2021 at 1:33 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 2/25/21 6:56 AM, Frode Nordahl wrote:
> > On Wed, Feb 24, 2021 at 5:36 PM Ilya Maximets <i.maximets@ovn.org> wrote:
> >>
> >> On 2/19/21 9:10 AM, Christian Ehrhardt wrote:
> >>> On Tue, Feb 16, 2021 at 5:32 PM Frode Nordahl
> >>> <frode.nordahl@canonical.com> wrote:
> >>>>
> >>>> ovs-ctl determines the system FQDN or hostname and records it in
> >>>> the `external-ids:hostname` field of the `Open-vSwitch` table on
> >>>> system startup.
> >>>>
> >>>> This value may be consumed by downstream software and having it
> >>>> unset or set to a incorrect value could lead to erratic behavior
> >>>> of a system.
> >>>>
> >>>> When a system is configured to use a Open vSwitch controlled
> >>>> datapath as its only network connection, the current ordering of
> >>>> events would always produce a unreliable hostname
> >>>>
> >>>> Reported-At: https://bugs.launchpad.net/bugs/1915829
> >>>> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
> >>>
> >>> I've looked at this for the Ubuntu problem that started Frodes work,
> >>> and while I can't give any authoritative Ack I can at least say
> >>>
> >>> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> >>>
> >>
> >> Hi.  Reading the description and the bug on launchpad it seems that
> >> the issue is that hostname in database changes in unexpected way and
> >> this is somewhat similar to what the following patch tried to fix:
> >>   http://patchwork.ozlabs.org/project/openvswitch/patch/20200525152821.19838-1-dalvarez@redhat.com/
> >
> > Hello Ilya, and thank you for the review.
> >
> >> Is it still a problem with above patch applied?
> >
> > Yes, while the above patch is welcome and adds to the stability of the
> > identifier, it does not solve the problem this patch addresses.
> >
> > What we see is that the data collected for the initial recording of
> > the hostname field is incorrect. This occurs because the information
> > might not be available at the time of initial Open vSwitch startup.
> >
> > We support the use of Open vSwitch to manage the only connection a
> > computer has to a network, and to do that we start Open vSwitch early
> > as part of the network target (in systemd terms). At this point in
> > time there may be no network connectivity yet, depending on deployment
> > type initial system configuration may not yet be applied (i.e.
> > /etc/hostname may hold a default unconfigured value), FQDN hints may
> > not yet be recorded in /etc/hosts etc.
> >
> > We seek to resolve this class of issues by breaking the hostname
> > recording part out into a separate oneshot service [0] run after the
> > network is online.
> >
> > To help support that we need to be able to manage whether and when
> > `ovs-ctl` performs the recording of the hostname.
> >
> > 0: https://code.launchpad.net/~fnordahl/ubuntu/+source/openvswitch/+git/openvswitch/+merge/398174
> >
>
> Thanks for clarification.  I see your point and I think it's OK to
> have this change.  One comment though: with this change 'record-hostname'
> command will not do anything if hostname was already configured.
> While this behavior is described in a 'help' message, this looks a bit
> counterintuitive anyway.  We should, probably, force it to re-write
> currently set value or rename somehow, e.g. 'record-hostname-if-not-set'
> or 'your better suggestion here'.
>
> What do you think?

Thank you for the feedback, and I do indeed agree that the command
should be more clear as to what it actually does in the event of data
already being set.

The only other suggestion I have is to use '--may-exist' like the
*-ctl commands to overwrite, but it would probably not help with
clarity as one would then still have to look at the manual, and in
addition wonder which of the commands that switch applies to.

I'll post a v2 using your suggestion with 'record-hostname-if-not-set'
as the name of the command.

--
Frode Nordahl
Ilya Maximets Feb. 25, 2021, 2:19 p.m. UTC | #6
On 2/25/21 2:34 PM, Frode Nordahl wrote:
> On Thu, Feb 25, 2021 at 1:33 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>>
>> On 2/25/21 6:56 AM, Frode Nordahl wrote:
>>> On Wed, Feb 24, 2021 at 5:36 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>>>>
>>>> On 2/19/21 9:10 AM, Christian Ehrhardt wrote:
>>>>> On Tue, Feb 16, 2021 at 5:32 PM Frode Nordahl
>>>>> <frode.nordahl@canonical.com> wrote:
>>>>>>
>>>>>> ovs-ctl determines the system FQDN or hostname and records it in
>>>>>> the `external-ids:hostname` field of the `Open-vSwitch` table on
>>>>>> system startup.
>>>>>>
>>>>>> This value may be consumed by downstream software and having it
>>>>>> unset or set to a incorrect value could lead to erratic behavior
>>>>>> of a system.
>>>>>>
>>>>>> When a system is configured to use a Open vSwitch controlled
>>>>>> datapath as its only network connection, the current ordering of
>>>>>> events would always produce a unreliable hostname
>>>>>>
>>>>>> Reported-At: https://bugs.launchpad.net/bugs/1915829
>>>>>> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
>>>>>
>>>>> I've looked at this for the Ubuntu problem that started Frodes work,
>>>>> and while I can't give any authoritative Ack I can at least say
>>>>>
>>>>> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>>>>>
>>>>
>>>> Hi.  Reading the description and the bug on launchpad it seems that
>>>> the issue is that hostname in database changes in unexpected way and
>>>> this is somewhat similar to what the following patch tried to fix:
>>>>   http://patchwork.ozlabs.org/project/openvswitch/patch/20200525152821.19838-1-dalvarez@redhat.com/
>>>
>>> Hello Ilya, and thank you for the review.
>>>
>>>> Is it still a problem with above patch applied?
>>>
>>> Yes, while the above patch is welcome and adds to the stability of the
>>> identifier, it does not solve the problem this patch addresses.
>>>
>>> What we see is that the data collected for the initial recording of
>>> the hostname field is incorrect. This occurs because the information
>>> might not be available at the time of initial Open vSwitch startup.
>>>
>>> We support the use of Open vSwitch to manage the only connection a
>>> computer has to a network, and to do that we start Open vSwitch early
>>> as part of the network target (in systemd terms). At this point in
>>> time there may be no network connectivity yet, depending on deployment
>>> type initial system configuration may not yet be applied (i.e.
>>> /etc/hostname may hold a default unconfigured value), FQDN hints may
>>> not yet be recorded in /etc/hosts etc.
>>>
>>> We seek to resolve this class of issues by breaking the hostname
>>> recording part out into a separate oneshot service [0] run after the
>>> network is online.
>>>
>>> To help support that we need to be able to manage whether and when
>>> `ovs-ctl` performs the recording of the hostname.
>>>
>>> 0: https://code.launchpad.net/~fnordahl/ubuntu/+source/openvswitch/+git/openvswitch/+merge/398174
>>>
>>
>> Thanks for clarification.  I see your point and I think it's OK to
>> have this change.  One comment though: with this change 'record-hostname'
>> command will not do anything if hostname was already configured.
>> While this behavior is described in a 'help' message, this looks a bit
>> counterintuitive anyway.  We should, probably, force it to re-write
>> currently set value or rename somehow, e.g. 'record-hostname-if-not-set'
>> or 'your better suggestion here'.
>>
>> What do you think?
> 
> Thank you for the feedback, and I do indeed agree that the command
> should be more clear as to what it actually does in the event of data
> already being set.
> 
> The only other suggestion I have is to use '--may-exist' like the
> *-ctl commands to overwrite, but it would probably not help with
> clarity as one would then still have to look at the manual, and in
> addition wonder which of the commands that switch applies to.
> 
> I'll post a v2 using your suggestion with 'record-hostname-if-not-set'
> as the name of the command.

OK.  Sounds good to me.
diff mbox series

Patch

diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index be3aa083b..9231ae4f4 100644
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -230,7 +230,9 @@  start_forwarding () {
     if test X"$OVS_VSWITCHD" = Xyes; then
         do_start_forwarding || return 1
     fi
-    set_hostname &
+    if test X"$RECORD_HOSTNAME" = Xyes; then
+        set_hostname &
+    fi
     return 0
 }
 
@@ -321,6 +323,7 @@  set_defaults () {
     SYSTEM_ID=
 
     FULL_HOSTNAME=yes
+    RECORD_HOSTNAME=yes
 
     DELETE_BRIDGES=no
     DELETE_TRANSIENT_PORTS=no
@@ -394,6 +397,8 @@  Commands:
   delete-transient-ports  delete transient (other_config:transient=true) ports
   start-ovs-ipsec         start Open vSwitch ipsec daemon
   stop-ovs-ipsec          stop Open vSwitch ipsec daemon
+  record-hostname         determine the system hostname and record it in the
+                          Open vSwitch database if not already set
   help                    display this help message
 
 One of the following options is required for "start", "restart" and "force-reload-kmod":
@@ -415,6 +420,8 @@  Less important options for "start", "restart" and "force-reload-kmod":
   --ovsdb-server-priority=NICE   set ovsdb-server's niceness (default: $OVSDB_SERVER_PRIORITY)
   --ovs-vswitchd-priority=NICE   set ovs-vswitchd's niceness (default: $OVS_VSWITCHD_PRIORITY)
   --no-full-hostname             set short hostname instead of full hostname
+  --no-record-hostname           do not attempt to determine/record system
+                                 hostname as part of start command
 
 Debugging options for "start", "restart" and "force-reload-kmod":
   --ovsdb-server-wrapper=WRAPPER
@@ -573,6 +580,9 @@  case $command in
     stop-ovs-ipsec)
         stop_ovs_ipsec
         ;;
+    record-hostname)
+        set_hostname
+        ;;
     help)
         usage
         ;;