diff mbox series

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

Message ID 20220920000453.357057-5-ihrachys@redhat.com
State Superseded, archived
Headers show
Series Support 2+ controllers on the same vswitchd | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Ihar Hrachyshka Sept. 20, 2022, 12:04 a.m. UTC
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.c |  9 ++++++++
 tests/ovn-macros.at         |  4 ++--
 tests/ovn.at                | 42 +++++++++++++++++++++++++++++++++++++
 5 files changed, 59 insertions(+), 2 deletions(-)

Comments

Ihar Hrachyshka Sept. 20, 2022, 12:09 a.m. UTC | #1
On Mon, Sep 19, 2022 at 8:05 PM Ihar Hrachyshka <ihrachys@redhat.com> 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.c |  9 ++++++++
>  tests/ovn-macros.at         |  4 ++--
>  tests/ovn.at                | 42 +++++++++++++++++++++++++++++++++++++
>  5 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/controller/chassis.c b/controller/chassis.c
> index bc8fb5282..383223019 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.c b/controller/ovn-controller.c
> index c71b0851f..16cbb971d 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -4519,6 +4519,9 @@ loop_done:
>      if (file_system_id) {
>          free(file_system_id);
>      }
> +    if (file_system_id) {

This should of course check for cli_system_id, not file_system_id. I
will update it in the next revision.

> +        free(cli_system_id);
> +    }
>      service_stop();
>
>      exit(retval);
> @@ -4544,6 +4547,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}
> @@ -4595,6 +4599,10 @@ parse_options(int argc, char *argv[])
>              vif_plug_dummy_enable();
>              break;
>
> +        case 'n':
> +            cli_system_id = xstrdup(optarg);
> +            break;
> +
>          case '?':
>              exit(EXIT_FAILURE);
>
> @@ -4630,6 +4638,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 5972089f1..e7b65691e 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -32972,3 +32972,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
> +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)
> +
> +OVN_CLEANUP([hv1],[hv2])
> +AT_CLEANUP
> +])
> --
> 2.34.1
>
Ales Musil Sept. 20, 2022, 6:39 a.m. UTC | #2
On Tue, Sep 20, 2022 at 2:09 AM Ihar Hrachyshka <ihrachys@redhat.com> wrote:

> On Mon, Sep 19, 2022 at 8:05 PM Ihar Hrachyshka <ihrachys@redhat.com>
> 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.c |  9 ++++++++
> >  tests/ovn-macros.at         |  4 ++--
> >  tests/ovn.at                | 42 +++++++++++++++++++++++++++++++++++++
> >  5 files changed, 59 insertions(+), 2 deletions(-)
> >
> > diff --git a/controller/chassis.c b/controller/chassis.c
> > index bc8fb5282..383223019 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.c b/controller/ovn-controller.c
> > index c71b0851f..16cbb971d 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -4519,6 +4519,9 @@ loop_done:
> >      if (file_system_id) {
> >          free(file_system_id);
> >      }
> > +    if (file_system_id) {
>
> This should of course check for cli_system_id, not file_system_id. I
> will update it in the next revision.
>
> > +        free(cli_system_id);
> > +    }
> >      service_stop();
> >
> >      exit(retval);
> > @@ -4544,6 +4547,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}
> > @@ -4595,6 +4599,10 @@ parse_options(int argc, char *argv[])
> >              vif_plug_dummy_enable();
> >              break;
> >
> > +        case 'n':
> > +            cli_system_id = xstrdup(optarg);
> > +            break;
> > +
> >          case '?':
> >              exit(EXIT_FAILURE);
> >
> > @@ -4630,6 +4638,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 5972089f1..e7b65691e 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -32972,3 +32972,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
> > +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)
> > +
> > +OVN_CLEANUP([hv1],[hv2])
> > +AT_CLEANUP
> > +])
> > --
> > 2.34.1
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
With the change in v2:

Reviewed-by: Ales Musil <amusil@redhat.com>
diff mbox series

Patch

diff --git a/controller/chassis.c b/controller/chassis.c
index bc8fb5282..383223019 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.c b/controller/ovn-controller.c
index c71b0851f..16cbb971d 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -4519,6 +4519,9 @@  loop_done:
     if (file_system_id) {
         free(file_system_id);
     }
+    if (file_system_id) {
+        free(cli_system_id);
+    }
     service_stop();
 
     exit(retval);
@@ -4544,6 +4547,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}
@@ -4595,6 +4599,10 @@  parse_options(int argc, char *argv[])
             vif_plug_dummy_enable();
             break;
 
+        case 'n':
+            cli_system_id = xstrdup(optarg);
+            break;
+
         case '?':
             exit(EXIT_FAILURE);
 
@@ -4630,6 +4638,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 5972089f1..e7b65691e 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32972,3 +32972,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
+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)
+
+OVN_CLEANUP([hv1],[hv2])
+AT_CLEANUP
+])