[ovs-dev] [PATCH ovn v2] ovn-controller to no longer monitor Chassis' external_ids

Dumitru Ceara dceara at redhat.com
Wed May 13 10:38:09 UTC 2020


On 5/12/20 6:01 PM, lmartins at redhat.com wrote:
> From: Lucas Alvares Gomes <lucasagomes at gmail.com>
> 
> Prior to this patch, the external_ids column from the Chassis table were
> being used for two purposes:
> 
> 1. Holding configuration for the OVN (copied by ovn-controller from the
> local OVS database)
> 
> 2. Holding information from external systems (the main purpose of the
>    external_ids column across other resources).
> 
> The problem with mixing these two use cases is that, ovn-controller
> should be aware of changes to the configuration and it would trigger
> flow re-computations upon those changes. It shouldn't care tho about the
> information from external systems but, since these two things were put
> together, CMSs writing things to the external_ids column of the Chassis
> were waking up ovn-controller to recompute flows.
> 
> This patch is separating these two things by creating another column in
> the Chassis table called "other_config". This new table holds the OVN
> configuration that is copied over from the local OVS db leaving the
> external_ids column unmonitored and free for other systems to make use
> of it.
> 
> This change also keeps things backward compatible by continuing to
> write the OVN configuration to the external_ids column for external
> systems that may be reading them from there but, that column is no
> longer monitored so it won't generate any events. This behavior should
> be temporary and removed in the future. The note in the NEWS file and
> comments in the code itself points to the future removal of this
> behavior.
> 
> Reported-At: https://bugzilla.redhat.com/show_bug.cgi?id=1824220
> Signed-off-by: Lucas Alvares Gomes <lucasagomes at gmail.com>

Thanks Lucas for this new revision! I have a few minor comments inline
but except for that:

Acked-by: Dumitru Ceara <dceara at redhat.com>

Thanks,
Dumitru

> ---
> v1 -> v2:
>  * Added a note to TODO.rst file to stop writting configuration 
>    to the Chassis external_ids in the future
>  * Fixed the backward compatibility with ovn-ic "is-remote"
>    configuration
> 
>  NEWS                            | 10 ++++++-
>  TODO.rst                        |  4 +++
>  controller/bfd.c                |  2 +-
>  controller/chassis.c            | 48 ++++++++++++++++++---------------
>  controller/encaps.c             |  4 +--
>  controller/ovn-controller.8.xml |  2 +-
>  controller/ovn-controller.c     |  9 ++++---
>  controller/physical.c           |  4 +--
>  ic/ovn-ic.c                     |  8 ++++--
>  northd/ovn-northd.c             |  4 +--
>  ovn-sb.ovsschema                |  7 +++--
>  ovn-sb.xml                      | 14 +++++-----
>  tests/ovn-controller.at         | 16 +++++------
>  tests/ovn-ic.at                 |  6 +++++
>  14 files changed, 84 insertions(+), 54 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index fea196d34..f3c603375 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,7 +1,15 @@
>  Post-v20.03.0
>  --------------------------
>     - Added support for external_port_range in NAT table.
> -
> +   - ovn-controller no longer monitor the external_ids column from
> +     the Chassis table. This was done to avoid having to do a flow
> +     recalculation every time external systems wrote to this column. The
> +     chassis configuration has now being moved to a new column called
> +     "other_config". As a note, the configurations are still be written to
> +     the external_ids column (but no longer triggers an alert) to
> +     keep the backward compatibility with current systems that may be
> +     reading it from that column but, this behavior will be removed
> +     in the future.
>  
>  OVN v20.03.0 - xx xxx xxxx
>  --------------------------
> diff --git a/TODO.rst b/TODO.rst
> index 809d1c91c..da28fdd2e 100644
> --- a/TODO.rst
> +++ b/TODO.rst
> @@ -149,3 +149,7 @@ OVN To-do List
>  * OVN Interconnection
>  
>    * Packaging for RHEL, Debian, etc.
> +
> +* ovn-controller: Stop copying the local OVS configuration into the
> +  Chassis external_ids column (same for the "is-remote" configuration from
> +  ovn-ic) a few releases after the 20.06 version (21.06 maybe ?).
> diff --git a/controller/bfd.c b/controller/bfd.c
> index 2b1e87f6d..b305eb158 100644
> --- a/controller/bfd.c
> +++ b/controller/bfd.c
> @@ -152,7 +152,7 @@ bfd_calculate_chassis(
>              /* It's an HA chassis. So add the ref_chassis to the bfd set. */
>              for (size_t i = 0; i < ha_chassis_grp->n_ref_chassis; i++) {
>                  struct sbrec_chassis *ref_ch = ha_chassis_grp->ref_chassis[i];
> -                if (smap_get_bool(&ref_ch->external_ids, "is-remote", false)) {
> +                if (smap_get_bool(&ref_ch->other_config, "is-remote", false)) {
>                      continue;
>                  }
>                  sset_add(&grp_chassis, ref_ch->name);
> diff --git a/controller/chassis.c b/controller/chassis.c
> index 522893ead..c1c609028 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -299,24 +299,24 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table,
>  }
>  
>  static void
> -chassis_build_external_ids(struct smap *ext_ids, const char *bridge_mappings,
> +chassis_build_other_config(struct smap *config, const char *bridge_mappings,
>                             const char *datapath_type, const char *cms_options,
>                             const char *chassis_macs, const char *iface_types,
>                             bool is_interconn)
>  {
> -    smap_replace(ext_ids, "ovn-bridge-mappings", bridge_mappings);
> -    smap_replace(ext_ids, "datapath-type", datapath_type);
> -    smap_replace(ext_ids, "ovn-cms-options", cms_options);
> -    smap_replace(ext_ids, "iface-types", iface_types);
> -    smap_replace(ext_ids, "ovn-chassis-mac-mappings", chassis_macs);
> -    smap_replace(ext_ids, "is-interconn", is_interconn ? "true" : "false");
> +    smap_replace(config, "ovn-bridge-mappings", bridge_mappings);
> +    smap_replace(config, "datapath-type", datapath_type);
> +    smap_replace(config, "ovn-cms-options", cms_options);
> +    smap_replace(config, "iface-types", iface_types);
> +    smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs);
> +    smap_replace(config, "is-interconn", is_interconn ? "true" : "false");
>  }
>  
>  /*
>   * Returns true if any external-id doesn't match the values in 'chassis-rec'.
>   */
>  static bool
> -chassis_external_ids_changed(const char *bridge_mappings,
> +chassis_other_config_changed(const char *bridge_mappings,
>                               const char *datapath_type,
>                               const char *cms_options,
>                               const char *chassis_macs,
> @@ -325,41 +325,41 @@ chassis_external_ids_changed(const char *bridge_mappings,
>                               const struct sbrec_chassis *chassis_rec)
>  {
>      const char *chassis_bridge_mappings =
> -        get_bridge_mappings(&chassis_rec->external_ids);
> +        get_bridge_mappings(&chassis_rec->other_config);
>  
>      if (strcmp(bridge_mappings, chassis_bridge_mappings)) {
>          return true;
>      }
>  
>      const char *chassis_datapath_type =
> -        smap_get_def(&chassis_rec->external_ids, "datapath-type", "");
> +        smap_get_def(&chassis_rec->other_config, "datapath-type", "");
>  
>      if (strcmp(datapath_type, chassis_datapath_type)) {
>          return true;
>      }
>  
>      const char *chassis_cms_options =
> -        get_cms_options(&chassis_rec->external_ids);
> +        get_cms_options(&chassis_rec->other_config);
>  
>      if (strcmp(cms_options, chassis_cms_options)) {
>          return true;
>      }
>  
>      const char *chassis_mac_mappings =
> -        get_chassis_mac_mappings(&chassis_rec->external_ids);
> +        get_chassis_mac_mappings(&chassis_rec->other_config);
>      if (strcmp(chassis_macs, chassis_mac_mappings)) {
>          return true;
>      }
>  
>      const char *chassis_iface_types =
> -        smap_get_def(&chassis_rec->external_ids, "iface-types", "");
> +        smap_get_def(&chassis_rec->other_config, "iface-types", "");
>  
>      if (strcmp(ds_cstr_ro(iface_types), chassis_iface_types)) {
>          return true;
>      }
>  
>      bool chassis_is_interconn =
> -        smap_get_bool(&chassis_rec->external_ids, "is-interconn", false);
> +        smap_get_bool(&chassis_rec->other_config, "is-interconn", false);
>      if (chassis_is_interconn != is_interconn) {
>          return true;
>      }
> @@ -538,25 +538,29 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
>          sbrec_chassis_set_hostname(chassis_rec, ovs_cfg->hostname);
>      }
>  
> -    if (chassis_external_ids_changed(ovs_cfg->bridge_mappings,
> +    if (chassis_other_config_changed(ovs_cfg->bridge_mappings,
>                                       ovs_cfg->datapath_type,
>                                       ovs_cfg->cms_options,
>                                       ovs_cfg->chassis_macs,
>                                       &ovs_cfg->iface_types,
>                                       ovs_cfg->is_interconn,
>                                       chassis_rec)) {
> -        struct smap ext_ids;
> +        struct smap other_config;
>  
> -        smap_clone(&ext_ids, &chassis_rec->external_ids);
> -        chassis_build_external_ids(&ext_ids, ovs_cfg->bridge_mappings,
> +        smap_clone(&other_config, &chassis_rec->other_config);
> +        chassis_build_other_config(&other_config, ovs_cfg->bridge_mappings,
>                                     ovs_cfg->datapath_type,
>                                     ovs_cfg->cms_options,
>                                     ovs_cfg->chassis_macs,
>                                     ds_cstr_ro(&ovs_cfg->iface_types),
>                                     ovs_cfg->is_interconn);
> -        sbrec_chassis_verify_external_ids(chassis_rec);
> -        sbrec_chassis_set_external_ids(chassis_rec, &ext_ids);
> -        smap_destroy(&ext_ids);
> +        sbrec_chassis_verify_other_config(chassis_rec);
> +        sbrec_chassis_set_other_config(chassis_rec, &other_config);
> +        /* TODO(lucasagomes): Continue writing the configuration to the
> +         * external_ids column for backward compatibility with the current
> +         * systems, this behavior should be removed in the future */

Nit: missing '.' at end of sentence.

> +        sbrec_chassis_set_external_ids(chassis_rec, &other_config);
> +        smap_destroy(&other_config);
>      }
>  
>      update_chassis_transport_zones(transport_zones, chassis_rec);
> @@ -626,7 +630,7 @@ chassis_get_mac(const struct sbrec_chassis *chassis_rec,
>                  struct eth_addr *chassis_mac)
>  {
>      const char *tokens
> -        = get_chassis_mac_mappings(&chassis_rec->external_ids);
> +        = get_chassis_mac_mappings(&chassis_rec->other_config);
>      if (!tokens[0]) {
>         return false;
>      }
> diff --git a/controller/encaps.c b/controller/encaps.c
> index 846628ac1..7eac4bb06 100644
> --- a/controller/encaps.c
> +++ b/controller/encaps.c
> @@ -357,8 +357,8 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
>                  continue;
>              }
>  
> -            if (smap_get_bool(&chassis_rec->external_ids, "is-remote", false)
> -                && !smap_get_bool(&this_chassis->external_ids, "is-interconn",
> +            if (smap_get_bool(&chassis_rec->other_config, "is-remote", false)
> +                && !smap_get_bool(&this_chassis->other_config, "is-interconn",
>                                    false)) {
>                  VLOG_DBG("Skipping encap creation for Chassis '%s' because "
>                           "it is remote but this chassis is not interconn.",
> diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
> index 76bbbdc5f..92e0a6e43 100644
> --- a/controller/ovn-controller.8.xml
> +++ b/controller/ovn-controller.8.xml
> @@ -245,7 +245,7 @@
>        <dd>
>          This value is read from local OVS integration bridge row of
>          <ref table="Bridge" db="Open_vSwitch"/> table and populated in
> -        <ref key="datapath-type" table="Chassis" column="external_ids"
> +        <ref key="datapath-type" table="Chassis" column="other_config"
>          db="OVN_Southbound"/> of the <ref table="Chassis" db="OVN_Southbound"/>
>          table in the OVN_Southbound database.
>        </dd>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index c0a97a265..d2edcd876 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1810,10 +1810,6 @@ main(int argc, char *argv[])
>       *  - DNS. pinctrl.c uses the external_ids column of DNS,
>       *    which it shouldn't. This should be removed.
>       *
> -     *  - Chassis - chassis.c copies the chassis configuration from
> -     *              local open_vswitch table to the external_ids of
> -     *              chassis.
> -     *
>       *  - Datapath_binding - lflow.c is using this to check if the datapath
>       *                       is switch or not. This should be removed.
>       * */
> @@ -1840,6 +1836,11 @@ main(int argc, char *argv[])
>      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_status);
>      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_target);
>  
> +    /* Omit alerts to the Chassis external_ids column, the configuration
> +     * from the local open_vswitch table has now being moved to the
> +     * other_config column so we no longer need to monitor it */
> +    ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_chassis_col_external_ids);
> +
>      update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, false);
>  
>      stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS);
> diff --git a/controller/physical.c b/controller/physical.c
> index 144aeb7bd..f06313b9d 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -424,7 +424,7 @@ populate_remote_chassis_macs(const struct sbrec_chassis *my_chassis,
>          }
>  
>          const char *tokens
> -            = get_chassis_mac_mappings(&chassis->external_ids);
> +            = get_chassis_mac_mappings(&chassis->other_config);
>  
>          if (!strlen(tokens)) {
>              continue;
> @@ -1517,7 +1517,7 @@ physical_run(struct physical_ctx *p_ctx,
>                  /*
>                   * We split the tunnel_id to get the chassis-id
>                   * and hash the tunnel list on the chassis-id. The
> -                 * reason to use the chassis-id alone is because 
> +                 * reason to use the chassis-id alone is because
>                   * there might be cases (multicast, gateway chassis)
>                   * where we need to tunnel to the chassis, but won't
>                   * have the encap-ip specifically.
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index a1ed25623..4979cdef1 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -295,6 +295,10 @@ sync_isb_gw_to_sb(struct ic_context *ctx,
>                    const struct sbrec_chassis *chassis)
>  {
>      sbrec_chassis_set_hostname(chassis, gw->hostname);
> +    sbrec_chassis_update_other_config_setkey(chassis, "is-remote", "true");
> +    /* TODO(lucasagomes): Continue writing the configuration to the
> +        * external_ids column for backward compatibility with the current
> +        * systems, this behavior should be removed in the future */

Nit: The multiline comment is not properly indented, '*' characters
should be aligned; also missing '.' at end of sentence.

>      sbrec_chassis_update_external_ids_setkey(chassis, "is-remote", "true");
>  
>      /* Sync encaps used by this gateway. */
> @@ -362,7 +366,7 @@ gateway_run(struct ic_context *ctx, const struct icsbrec_availability_zone *az)
>  
>      const struct sbrec_chassis *chassis;
>      SBREC_CHASSIS_FOR_EACH (chassis, ctx->ovnsb_idl) {
> -        if (smap_get_bool(&chassis->external_ids, "is-interconn", false)) {
> +        if (smap_get_bool(&chassis->other_config, "is-interconn", false)) {
>              gw = shash_find_and_delete(&local_gws, chassis->name);
>              if (!gw) {
>                  gw = icsbrec_gateway_insert(ctx->ovnisb_txn);
> @@ -372,7 +376,7 @@ gateway_run(struct ic_context *ctx, const struct icsbrec_availability_zone *az)
>              } else if (is_gateway_data_changed(gw, chassis)) {
>                  sync_sb_gw_to_isb(ctx, chassis, gw);
>              }
> -        } else if (smap_get_bool(&chassis->external_ids, "is-remote", false)) {
> +        } else if (smap_get_bool(&chassis->other_config, "is-remote", false)) {
>              gw = shash_find_and_delete(&remote_gws, chassis->name);
>              if (!gw) {
>                  sbrec_chassis_delete(chassis);
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index b25152d74..f1e90a293 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11648,7 +11648,7 @@ update_northbound_cfg(struct northd_context *ctx,
>          const struct sbrec_chassis *chassis;
>          int64_t hv_cfg = nbg->nb_cfg;
>          SBREC_CHASSIS_FOR_EACH (chassis, ctx->ovnsb_idl) {
> -            if (!smap_get_bool(&chassis->external_ids, "is-remote", false) &&
> +            if (!smap_get_bool(&chassis->other_config, "is-remote", false) &&
>                  chassis->nb_cfg < hv_cfg) {
>                  hv_cfg = chassis->nb_cfg;
>              }
> @@ -11933,7 +11933,7 @@ main(int argc, char *argv[])
>      ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_chassis);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_nb_cfg);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_name);
> -    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_external_ids);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_other_config);
>  
>      ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_ha_chassis);
>      add_column_noalert(ovnsb_idl_loop.idl,
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index d89f8dbbb..c196ddaf3 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Southbound",
> -    "version": "2.7.0",
> -    "cksum": "4286723485 21693",
> +    "version": "2.8.0",
> +    "cksum": "1643994484 21853",
>      "tables": {
>          "SB_Global": {
>              "columns": {
> @@ -38,6 +38,9 @@
>                  "external_ids": {
>                      "type": {"key": "string", "value": "string",
>                               "min": 0, "max": "unlimited"}},
> +                "other_config": {
> +                    "type": {"key": "string", "value": "string",
> +                             "min": 0, "max": "unlimited"}},
>                  "transport_zones" : {"type": {"key": "string",
>                                                "min": 0,
>                                                "max": "unlimited"}}},
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 3aa7cd4da..417424fb8 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -262,14 +262,14 @@
>        from the <ref table="SB_Global"/> table into this column.
>      </column>
>  
> -    <column name="external_ids" key="ovn-bridge-mappings">
> +    <column name="other_config" key="ovn-bridge-mappings">
>        <code>ovn-controller</code> populates this key with the set of bridge
>        mappings it has been configured to use.  Other applications should treat
>        this key as read-only.  See <code>ovn-controller</code>(8) for more
>        information.
>      </column>
>  
> -    <column name="external_ids" key="datapath-type">
> +    <column name="other_config" key="datapath-type">
>        <code>ovn-controller</code> populates this key with the datapath type
>        configured in the <ref table="Bridge" column="datapath_type"/> column of
>        the Open_vSwitch database's <ref table="Bridge" db="Open_vSwitch"/>
> @@ -277,7 +277,7 @@
>        <code>ovn-controller</code>(8) for more information.
>      </column>
>  
> -    <column name="external_ids" key="iface-types">
> +    <column name="other_config" key="iface-types">
>        <code>ovn-controller</code> populates this key with the interface types
>        configured in the <ref table="Open_vSwitch" column="iface_types"/> column
>        of the Open_vSwitch database's <ref table="Open_vSwitch"
> @@ -285,7 +285,7 @@
>        read-only. See <code>ovn-controller</code>(8) for more information.
>      </column>
>  
> -    <column name="external_ids" key="ovn-cms-options">
> +    <column name="other_config" key="ovn-cms-options">
>        <code>ovn-controller</code> populates this key with the set of options
>        configured in the <ref table="Open_vSwitch"
>        column="external_ids:ovn-cms-options"/> column of the Open_vSwitch
> @@ -293,7 +293,7 @@
>        See <code>ovn-controller</code>(8) for more information.
>      </column>
>  
> -    <column name="external_ids" key="is-interconn">
> +    <column name="other_config" key="is-interconn">
>        <code>ovn-controller</code> populates this key with the setting
>        configured in the <ref table="Open_vSwitch"
>        column="external_ids:ovn-is-interconn"/> column of the Open_vSwitch
> @@ -302,7 +302,7 @@
>        See <code>ovn-controller</code>(8) for more information.
>      </column>
>  
> -    <column name="external_ids" key="is-remote">
> +    <column name="other_config" key="is-remote">
>        <code>ovn-ic</code> set this key to true for remote interconnection
>        gateway chassises learned from the interconnection southbound database.
>        See <code>ovn-ic</code>(8) for more information.
> @@ -316,7 +316,7 @@
>        See <code>ovn-controller</code>(8) for more information.
>      </column>
>  
> -    <column name="external_ids" key="ovn-chassis-mac-mappings">
> +    <column name="other_config" key="ovn-chassis-mac-mappings">
>        <code>ovn-controller</code> populates this key with the set of options
>        configured in the <ref table="Open_vSwitch"
>        column="external_ids:ovn-chassis-mac-mappings"/> column of the
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 63b2581c0..77936c776 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -51,7 +51,7 @@ patch
>  check_bridge_mappings () {
>      local_mappings=$1
>      sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id)
> -    OVS_WAIT_UNTIL([test x"${local_mappings}" = x$(ovn-sbctl get Chassis ${sysid} external_ids:ovn-bridge-mappings | sed -e 's/\"//g')])
> +    OVS_WAIT_UNTIL([test x"${local_mappings}" = x$(ovn-sbctl get Chassis ${sysid} other_config:ovn-bridge-mappings | sed -e 's/\"//g')])
>  }
>  
>  # Initially there should be no patch ports.
> @@ -118,8 +118,8 @@ OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>  AT_CLEANUP
>  
>  # Checks that ovn-controller populates datapath-type and iface-types
> -# correctly in the Chassis external-ids column.
> -AT_SETUP([ovn-controller - Chassis external_ids])
> +# correctly in the Chassis other_config column.
> +AT_SETUP([ovn-controller - Chassis other_config])
>  AT_KEYWORDS([ovn])
>  ovn_init_db ovn-sb
>  
> @@ -139,7 +139,7 @@ sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id)
>  # is mirrored into the Chassis record in the OVN_Southbound db.
>  check_datapath_type () {
>      datapath_type=$1
> -    chassis_datapath_type=$(ovn-sbctl get Chassis ${sysid} external_ids:datapath-type | sed -e 's/"//g') #"
> +    chassis_datapath_type=$(ovn-sbctl get Chassis ${sysid} other_config:datapath-type | sed -e 's/"//g') #"
>      test "${datapath_type}" = "${chassis_datapath_type}"
>  }
>  
> @@ -174,15 +174,15 @@ OVS_WAIT_UNTIL([check_datapath_type foobar])
>  
>  expected_iface_types=$(ovs-vsctl get Open_vSwitch . iface_types | tr -d '[[]] ""')
>  echo "expected_iface_types = ${expected_iface_types}"
> -chassis_iface_types=$(ovn-sbctl get Chassis ${sysid} external_ids:iface-types | sed -e 's/\"//g')
> +chassis_iface_types=$(ovn-sbctl get Chassis ${sysid} other_config:iface-types | sed -e 's/\"//g')
>  echo "chassis_iface_types = ${chassis_iface_types}"
>  AT_CHECK([test "${expected_iface_types}" = "${chassis_iface_types}"])
>  
> -# Change the value of external_ids:iface-types using ovn-sbctl.
> +# Change the value of other_config:iface-types using ovn-sbctl.
>  # ovn-controller should again set it back to proper one.
> -ovn-sbctl set Chassis ${sysid} external_ids:iface-types="foo"
> +ovn-sbctl set Chassis ${sysid} other_config:iface-types="foo"
>  OVS_WAIT_UNTIL([
> -    chassis_iface_types=$(ovn-sbctl get Chassis ${sysid} external_ids:iface-types | sed -e 's/\"//g')
> +    chassis_iface_types=$(ovn-sbctl get Chassis ${sysid} other_config:iface-types | sed -e 's/\"//g')
>      echo "chassis_iface_types = ${chassis_iface_types}"
>      test "${expected_iface_types}" = "${chassis_iface_types}"
>  ])
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index 17e720e9c..cf9224bab 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -90,6 +90,12 @@ Chassis gw1
>          ip: "192.168.0.1"
>  ])
>  
> +AT_CHECK([ovn_as az2 ovn-sbctl -f csv -d bare --no-headings --columns other_config list chassis], [0], [dnl
> +is-remote=true
> +])
> +
> +# TODO(lucasagomes): Remove this check when we get rid of the behavior
> +# of writting configuration to the Chassis external_ids column

Nit: missing '.' at end of sentence.

>  AT_CHECK([ovn_as az2 ovn-sbctl -f csv -d bare --no-headings --columns external_ids list chassis], [0], [dnl
>  is-remote=true
>  ])
> 



More information about the dev mailing list