On Wed, Feb 2, 2022 at 3:39 PM Vladislav Odintsov <[email protected]> wrote:
>
> Simlar to ovn-controller's behavior with checking internal
> version of ovn-northd and ovn-controller when option
> ovn-match-northd-version is defined, this commit adds same
> functionality for the ovn-controller-vtep daemon.
>
> This configuration option is located in the configured
> hardware_vtep database in the Global table's
> other_config:ovn-match-northd-version column/key.
> True value enforces check, while false or not defined values
> skip the check.
>
> The appropriate testcase is added as well.
>
> Signed-off-by: Vladislav Odintsov <[email protected]>

Thanks.  I applied this patch to the main branch.

Can you please submit another patch adding a news entry for this ?

Numan

> ---
>  controller-vtep/ovn-controller-vtep.8.xml | 23 +++++++++-
>  controller-vtep/ovn-controller-vtep.c     | 50 +++++++++++++++++++--
>  tests/ovn-controller-vtep.at              | 55 +++++++++++++++++++++++
>  3 files changed, 124 insertions(+), 4 deletions(-)
>
> diff --git a/controller-vtep/ovn-controller-vtep.8.xml 
> b/controller-vtep/ovn-controller-vtep.8.xml
> index 2c706e46e..0b9987bdb 100644
> --- a/controller-vtep/ovn-controller-vtep.8.xml
> +++ b/controller-vtep/ovn-controller-vtep.8.xml
> @@ -34,7 +34,7 @@
>        information from both the ovnsb and the vtep database.  If the
>        database locations are not given from command line, the default
>        is the <code>db.sock</code> in local OVSDB's 'run' directory.
> -      The datapath location must take one of the following forms:
> +      The database location must take one of the following forms:
>      </p>
>      <ul>
>        <li>
> @@ -77,4 +77,25 @@
>          </p>
>        </li>
>      </ul>
> +
> +    <p>
> +      <code>ovn-controller-vtep</code> assumes it gets configuration
> +      information from the following keys in the <code>Global</code>
> +      table of the connected <code>hardware_vtep</code> database:
> +    </p>
> +
> +    <p>
> +    <dl>
> +      <dt><code>other_config:ovn-match-northd-version</code></dt>
> +      <dd>
> +        The boolean flag indicates if <code>ovn-controller-vtep</code> needs 
> to
> +        check <code>ovn-northd</code> version. If this flag is set to true 
> and
> +        the <code>ovn-northd's</code> version (reported in the Southbound
> +        database) doesn't match with the <code>ovn-controller-vtep's</code>
> +        internal version, then it will stop processing the southbound and
> +        connected <code>hardware_vtep</code> database changes.
> +        The default value is considered false if this option is not defined.
> +      </dd>
> +    </dl>
> +    </p>
>  </manpage>
> diff --git a/controller-vtep/ovn-controller-vtep.c 
> b/controller-vtep/ovn-controller-vtep.c
> index 1d35c7f04..50f412b95 100644
> --- a/controller-vtep/ovn-controller-vtep.c
> +++ b/controller-vtep/ovn-controller-vtep.c
> @@ -30,6 +30,7 @@
>  #include "fatal-signal.h"
>  #include "openvswitch/poll-loop.h"
>  #include "simap.h"
> +#include "ovsdb-idl.h"
>  #include "stream.h"
>  #include "stream-ssl.h"
>  #include "unixctl.h"
> @@ -45,6 +46,8 @@
>  #include "vtep.h"
>  #include "ovn-controller-vtep.h"
>
> +VLOG_DEFINE_THIS_MODULE(main);
> +
>  static unixctl_cb_func ovn_controller_vtep_exit;
>
>  static void parse_options(int argc, char *argv[]);
> @@ -54,6 +57,37 @@ static char *vtep_remote;
>  static char *ovnsb_remote;
>  static char *default_db_;
>
> +/* Returns true if the northd internal version stored in SB_Global
> + * and ovn-controller-vtep internal version match.
> + */
> +static bool
> +check_northd_version(struct ovsdb_idl *vtep_idl, struct ovsdb_idl *ovnsb_idl,
> +                     const char *version)
> +{
> +    const struct vteprec_global *cfg = vteprec_global_first(vtep_idl);
> +    if (!cfg || !smap_get_bool(&cfg->other_config, 
> "ovn-match-northd-version",
> +                               false)) {
> +        return true;
> +    }
> +
> +    const struct sbrec_sb_global *sb = sbrec_sb_global_first(ovnsb_idl);
> +    if (!sb) {
> +        return false;
> +    }
> +
> +    const char *northd_version =
> +        smap_get_def(&sb->options, "northd_internal_version", "");
> +
> +    if (strcmp(northd_version, version)) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> +        VLOG_WARN_RL(&rl, "controller-vtep version - %s mismatch with northd 
> "
> +                     "version - %s", version, northd_version);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  int
>  main(int argc, char *argv[])
>  {
> @@ -91,6 +125,9 @@ main(int argc, char *argv[])
>          ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
>      ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
>
> +    char *ovn_version = ovn_get_internal_version();
> +    VLOG_INFO("OVN internal version is : [%s]", ovn_version);
> +
>      /* Main loop. */
>      exiting = false;
>      while (!exiting) {
> @@ -109,9 +146,16 @@ main(int argc, char *argv[])
>              memory_report(&usage);
>              simap_destroy(&usage);
>          }
> -        gateway_run(&ctx);
> -        binding_run(&ctx);
> -        vtep_run(&ctx);
> +
> +        if (ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl) &&
> +            ovsdb_idl_has_ever_connected(vtep_idl_loop.idl) &&
> +            check_northd_version(vtep_idl_loop.idl, ovnsb_idl_loop.idl,
> +                                 ovn_version)) {
> +            gateway_run(&ctx);
> +            binding_run(&ctx);
> +            vtep_run(&ctx);
> +        }
> +
>          unixctl_server_run(unixctl);
>
>          unixctl_server_wait(unixctl);
> diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
> index 08e1d13e7..35afeea74 100644
> --- a/tests/ovn-controller-vtep.at
> +++ b/tests/ovn-controller-vtep.at
> @@ -627,3 +627,58 @@ priority=110,tun_id=0x<>,in_port=<> 
> actions=move:NXM_NX_TUN_ID[[0..23]]->OXM_OF_
>  OVN_CLEANUP([hv1])
>  AT_CLEANUP
>  ])
> +
> +AT_SETUP([ovn -- check ovn-northd and ovn-controller-vtep version pinning])
> +ovn_start
> +OVN_CONTROLLER_VTEP_START(vtep1)
> +net_add n1
> +
> +# adds vlan_bindings to physical ports.
> +check as vtep1 vtep-ctl add-ls lswitch0 -- bind-ls vtep1 p0 100 lswitch0 -- 
> bind-ls vtep1 p0 200 lswitch0 -- bind-ls vtep1 p1 300 lswitch0
> +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Chassis | grep -- lswitch0`"])
> +AT_CHECK([ovn-sbctl --columns=vtep_logical_switches list Chassis | cut -d 
> ':' -f2 | tr -d ' ' ], [0], [dnl
> +[[lswitch0]]
> +])
> +
> +northd_version=$(ovn-sbctl get SB_Global . options:northd_internal_version | 
> sed s/\"//g)
> +echo "northd version = $northd_version"
> +AT_CHECK([grep -c $northd_version vtep1/ovn-controller-vtep.log], [0], [1
> +])
> +
> +# Stop ovn-northd so that we can modify the northd_version.
> +as northd
> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
> +
> +as northd-backup
> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
> +
> +check ovn-sbctl set SB_Global . options:northd_internal_version=foo
> +check ovn-sbctl set Chassis vtep1 vtep_logical_switches=foo
> +
> +# ovn-controller-vtep should reset vtep_logical_switches to previous value
> +# since ovn-match-northd-version is not true.
> +AT_CHECK([ovn-sbctl --columns=vtep_logical_switches list Chassis | cut -d 
> ':' -f2 | tr -d ' ' ], [0], [dnl
> +[[lswitch0]]
> +])
> +
> +check as vtep1 vtep-ctl set global . 
> other_config:ovn-match-northd-version=true
> +
> +OVS_WAIT_UNTIL(
> +    [test 1 = $(grep -c "controller-vtep version - $northd_version mismatch 
> with northd version - foo" vtep1/ovn-controller-vtep.log)
> +])
> +
> +check ovn-sbctl set Chassis vtep1 vtep_logical_switches=foo
> +
> +# ovn-controller-vtep should not set vtep_logical_switches back since there 
> is a version mismatch
> +AT_CHECK([ovn-sbctl --columns=vtep_logical_switches list Chassis | cut -d 
> ':' -f2 | tr -d ' ' ], [0], [dnl
> +[[foo]]
> +])
> +
> +check ovn-sbctl set SB_Global . 
> options:northd_internal_version=$northd_version
> +
> +# ovn-controller-vtep should set vtep_logical_switches back since versions 
> match
> +AT_CHECK([ovn-sbctl --columns=vtep_logical_switches list Chassis | cut -d 
> ':' -f2 | tr -d ' ' ], [0], [dnl
> +[[lswitch0]]
> +])
> +
> +AT_CLEANUP
> --
> 2.30.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to