[ovs-dev,v6,4/7] Support passing chassis name via CLI

This patch adds support for the desired system-id (chassis name) to be
passed via CLI:

$ ovn-controller -n <chassis-name>

If passed, CLI overrides any settings stored in ovsdb or in
system-id-override file.

This may be useful when running multiple controller instances using the
same vswitchd instance.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
---
 controller/chassis.c            |  5 ++++
 controller/chassis.h            |  1 +
 controller/ovn-controller.8.xml |  8 ++++---
 controller/ovn-controller.c     |  9 +++++++
 tests/ovn-macros.at             |  4 ++--
 tests/ovn.at                    | 42 +++++++++++++++++++++++++++++++++
 6 files changed, 64 insertions(+), 5 deletions(-)

On 10/18/22 14:33, Ihar Hrachyshka wrote:
> This patch adds support for the desired system-id (chassis name) to be
> passed via CLI:
> 
> $ ovn-controller -n <chassis-name>
> 
> If passed, CLI overrides any settings stored in ovsdb or in
> system-id-override file.
> 
> This may be useful when running multiple controller instances using the
> same vswitchd instance.
> 
> Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
> ---
>   controller/chassis.c            |  5 ++++
>   controller/chassis.h            |  1 +
>   controller/ovn-controller.8.xml |  8 ++++---
>   controller/ovn-controller.c     |  9 +++++++
>   tests/ovn-macros.at             |  4 ++--
>   tests/ovn.at                    | 42 +++++++++++++++++++++++++++++++++
>   6 files changed, 64 insertions(+), 5 deletions(-)
> 
> diff --git a/controller/chassis.c b/controller/chassis.c
> index f121af3c3..521bec2bd 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -37,6 +37,7 @@ VLOG_DEFINE_THIS_MODULE(chassis);
>   #define HOST_NAME_MAX 255
>   #endif /* HOST_NAME_MAX */
>   
> +char *cli_system_id = NULL;
>   char *file_system_id = NULL;
>   
>   /*
> @@ -279,6 +280,10 @@ chassis_parse_ovs_iface_types(char **iface_types, size_t n_iface_types,
>   const char *
>   get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table)
>   {
> +    if (cli_system_id) {
> +        return cli_system_id;
> +    }
> +
>       if (file_system_id) {
>           return file_system_id;
>       }
> diff --git a/controller/chassis.h b/controller/chassis.h
> index baa327059..309ced28f 100644
> --- a/controller/chassis.h
> +++ b/controller/chassis.h
> @@ -31,6 +31,7 @@ struct sset;
>   struct eth_addr;
>   struct smap;
>   
> +extern char *cli_system_id;
>   extern char *file_system_id;
>   
>   void chassis_register_ovs_idl(struct ovsdb_idl *);
> diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
> index fef6309d5..2f852d39f 100644
> --- a/controller/ovn-controller.8.xml
> +++ b/controller/ovn-controller.8.xml
> @@ -74,9 +74,11 @@
>         gracefully stop <code>ovn-controller</code> or manually delete the
>         stale <code>Chassis</code> and <code>Chassis_Private</code> records
>         after changing the <code>system-id</code>. Note that the chassis name can
> -      also be provided via the <code>system-id-override</code> file. The file
> -      configuration overrides the one from the database, if both are
> -      present.</dd>
> +      also be provided via the <code>system-id-override</code> file or via the
> +      <code>-n</code> command-line option. The following precedence is used:
> +      first, the command-line option is read; if not present, the
> +      <code>system-id-override</code> file is read; if not present, then the
> +      name configured in the database is used.</dd>
>   
>         <dt><code>external_ids:hostname</code></dt>
>         <dd>The hostname to use in the Chassis table.</dd>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index af1332f12..e26d46535 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -4613,6 +4613,9 @@ loop_done:
>       if (file_system_id) {
>           free(file_system_id);
>       }
> +    if (cli_system_id) {
> +        free(cli_system_id);
> +    }
>       service_stop();
>   
>       exit(retval);
> @@ -4638,6 +4641,7 @@ parse_options(int argc, char *argv[])
>           STREAM_SSL_LONG_OPTIONS,
>           {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
>           {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
> +        {"chassis", required_argument, NULL, 'n'},
>           {"enable-dummy-vif-plug", no_argument, NULL,
>            OPT_ENABLE_DUMMY_VIF_PLUG},
>           {NULL, 0, NULL, 0}
> @@ -4689,6 +4693,10 @@ parse_options(int argc, char *argv[])
>               vif_plug_dummy_enable();
>               break;
>   
> +        case 'n':
> +            cli_system_id = xstrdup(optarg);
> +            break;
> +
>           case '?':
>               exit(EXIT_FAILURE);
>   
> @@ -4724,6 +4732,7 @@ usage(void)
>       daemon_usage();
>       vlog_usage();
>       printf("\nOther options:\n"
> +           "  -n                      custom chassis name\n"
>              "  -h, --help              display this help message\n"
>              "  -V, --version           display version information\n");
>       exit(EXIT_SUCCESS);
> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
> index 35b7b3872..b7f236fa8 100644
> --- a/tests/ovn-macros.at
> +++ b/tests/ovn-macros.at
> @@ -290,7 +290,7 @@ net_attach () {
>   
>   # ovn_az_attach AZ NETWORK BRIDGE IP [MASKLEN]
>   ovn_az_attach() {
> -    local az=$1 net=$2 bridge=$3 ip=$4 masklen=${5-24} encap=${6-geneve,vxlan} systemid=${7-$sandbox}
> +    local az=$1 net=$2 bridge=$3 ip=$4 masklen=${5-24} encap=${6-geneve,vxlan} systemid=${7-$sandbox} cli_args=${@:8}
>       net_attach $net $bridge || return 1
>   
>       mac=`ovs-vsctl get Interface $bridge mac_in_use | sed s/\"//g`
> @@ -326,7 +326,7 @@ ovn_az_attach() {
>           ovs-vsctl set open . external_ids:ovn-monitor-all=true
>       fi
>   
> -    start_daemon ovn-controller --enable-dummy-vif-plug || return 1
> +    start_daemon ovn-controller --enable-dummy-vif-plug ${cli_args} || return 1
>   }
>   
>   # ovn_attach NETWORK BRIDGE IP [MASKLEN]
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 35c853618..2508a224e 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -33058,3 +33058,45 @@ AT_CHECK(test x$encap_hv1_ip == x)
>   OVN_CLEANUP([hv1],[hv2])
>   AT_CLEANUP
>   ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([chassis name override via CLI])
> +ovn_start
> +net_add n1
> +
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +
> +ovs-vsctl \
> +    -- set Open_vSwitch . external-ids:ovn-encap-type-hv3=geneve \
> +    -- set Open_vSwitch . external-ids:ovn-encap-ip-hv3=192.168.1.1
> +
> +as hv1 ovs-vsctl set-ssl \
> +   $PKIDIR/testpki-hv3-privkey.pem \
> +   $PKIDIR/testpki-hv3-cert.pem \
> +   $PKIDIR/testpki-cacert.pem
> +
> +# the last argument is passed to ovn-controller through cli
> +ovn_attach n1 br-phys 192.168.0.1 24 vxlan hv1 -n hv3
> +
> +sim_add hv2
> +as hv2
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.2 24 geneve
> +
> +# despite that we configured ovn-encap-ip=192.168.0.1, this setting is
> +# overridden by chassis specific ovn-encap-ip-hv3

Suggestion: Use both the -n option and set the system-id-override so 
that you can ensure that -n overrides both the OVSDB and system-id-override.

> +OVS_WAIT_UNTIL([
> +    test "1" = "$(ovn-sbctl list Encap | grep -c '192.168.1.1')"
> +])
> +
> +encap_hv3_ip=$(fetch_column Encap ip chassis_name=hv3 type=geneve)
> +AT_CHECK(test x$encap_hv3_ip == x192.168.1.1)
> +
> +encap_hv1_ip=$(fetch_column Encap ip chassis_name=hv1 type=vxlan)
> +AT_CHECK(test x$encap_hv1_ip == x)
> +

Just like the last two patches, calls to wait_column and check_column 
can simplify things.

> +OVN_CLEANUP([hv1],[hv2])
> +AT_CLEANUP
> +])