On Wed, Jul 2, 2025 at 11:50 AM Xavier Simonart via dev <
ovs-dev@openvswitch.org> wrote:

> Test "ovn multinode - Transit Router basic functionality" stops
> ovn-controller, restarts it using local sb, and finally, restarts it
> using ovn-central sb using tcp.
> If the cluster was started with ENABLE_SSL="yes", then all following tests
> were failing.
> This patch ensures that ssl continue to be used if it was initially used.
>
> Fixes: 7c3f7f415f1d ("northd, controller: Flood ARP and NA packet on
> transit router.")
> Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
> ---
>

Hi Xavier,

thank you for the patch. I can see the following comment
in the ovn-fake-multinode-tests.yml:

"# Disable SSL/TLS for now. Revisit this if required."

That kinda suggests that the CI wasn't meant to run with SSL enabled,
I'm fine with adding proper support I just wonder if we could do that
slightly differently. See down below.

 tests/multinode-macros.at | 43 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/tests/multinode-macros.at b/tests/multinode-macros.at
> index 3f610f2e0..c349a36be 100644
> --- a/tests/multinode-macros.at
> +++ b/tests/multinode-macros.at
> @@ -89,6 +89,37 @@ check_fake_multinode_setup_by_nodes() {
>          on_exit "m_as $c ovs-ofctl dump-flows br-int > flow-${c}.txt"
>          on_exit "m_as $c ovs-vsctl get open . external_ids >
> extids-${c}.txt"
>      done
> +
> +    # Only check ssl on one chassis - whole setup uses ssl or not.
> +    if [[ -z "${REMOTE_PROT}" ]]; then
> +        enable_ssl=$(m_as $c ps -efww | grep ovn-controller | grep
> private-key)
> +        if [[ -n "$enable_ssl" ]]; then
>

We have the 'ENABLE_SSL' env variable we could check. With that this can be
actually part of 'multinode_setup_northd()' and
'multinode_setup_controller()'.


> +            private_key=$(m_as $c ps -efww | grep ovn-controller | sed -n
> 's/.*--private-key=\([[^ ]]*\).*/\1/p')
> +            ssl_cert=$(m_as $c ps -efww | grep ovn-controller | sed -n
> 's/.*--certificate=\([[^ ]]*\).*/\1/p')
> +            ssl_ca_cert=$(m_as $c ps -efww | grep ovn-controller | sed -n
> 's/.*--ca-cert=\([[^ ]]*\).*/\1/p')
>

ovn-fake-multinode uses 'SSL_CERTS_PATH' env variable which is then
expanded to
proper arguments. I see as an option to utilize this env variable (making
it changeable
in ovn-fake-multinode) and setting it in ovn-fake-multinode-tests.yml. We
wouldn't
have to parse the arguments.

+            CONTROLLER_SSL_ARGS="--ovn-controller-ssl-key=$private_key \
> +                                 --ovn-controller-ssl-cert=$ssl_cert \
> +
>  --ovn-controller-ssl-ca-cert=$ssl_ca_cert"
> +            NORTHD_SSL_ARGS="--ovn-nb-db-ssl-key=$private_key \
> +                  --ovn-nb-db-ssl-cert=$ssl_cert \
> +                  --ovn-nb-db-ssl-ca-cert=$ssl_ca_cert \
> +                  --ovn-sb-db-ssl-key=$private_key \
> +                  --ovn-sb-db-ssl-cert=$ssl_cert \
> +                  --ovn-sb-db-ssl-ca-cert=$ssl_ca_cert \
> +                  --ovn-northd-ssl-key=$private_key \
> +                  --ovn-northd-ssl-cert=$ssl_cert \
> +                  --ovn-northd-ssl-ca-cert=$ssl_ca_cert"
> +
> +            REMOTE_PROT=ssl
> +        else
> +            CONTROLLER_SSL_ARGS=""
> +            NORTHD_SSL_ARGS=""
> +            REMOTE_PROT=tcp
> +        fi
> +        export CONTROLLER_SSL_ARGS
> +        export NORTHD_SSL_ARGS
> +        export REMOTE_PROT
> +    fi
>  }
>
>  check_fake_multinode_setup() {
> @@ -128,9 +159,10 @@ multinode_setup_northd() {
>
>      multinode_cleanup_northd $c
>
> -    m_as $c /usr/share/ovn/scripts/ovn-ctl start_northd
> -    m_as $c ovn-nbctl set-connection ptcp:6641
> -    m_as $c ovn-sbctl set-connection ptcp:6642
> +    echo "Using ${NORTHD_SSL_ARGS} for northd".
> +    m_as $c /usr/share/ovn/scripts/ovn-ctl start_northd ${NORTHD_SSL_ARGS}
> +    m_as $c ovn-nbctl set-connection p${REMOTE_PROT}:6641
> +    m_as $c ovn-sbctl set-connection p${REMOTE_PROT}:6642
>  }
>
>  # multinode_setup_controller NODE ENCAP_IP REMOTE_IP [ENCAP_TYPE]
> @@ -148,11 +180,12 @@ multinode_setup_controller() {
>      m_as $c sh -c "rm -f /etc/openvswitch/*.db"
>
>      m_as $c /usr/share/openvswitch/scripts/ovs-ctl start --system-id=$c
> -    m_as $c /usr/share/ovn/scripts/ovn-ctl start_controller
> +    echo "Using ${CONTROLLER_SSL_ARGS} for ovn-controller".
> +    m_as $c /usr/share/ovn/scripts/ovn-ctl start_controller
> ${CONTROLLER_SSL_ARGS}
>
>      m_as $c ovs-vsctl set open . external_ids:ovn-encap-ip=$encap_ip
>      m_as $c ovs-vsctl set open . external-ids:ovn-encap-type=$encap_type
> -    m_as $c ovs-vsctl set open .
> external-ids:ovn-remote=tcp:$remote_ip:6642
> +    m_as $c ovs-vsctl set open .
> external-ids:ovn-remote=${REMOTE_PROT}:$remote_ip:6642
>      m_as $c ovs-vsctl set open .
> external-ids:ovn-openflow-probe-interval=60
>      m_as $c ovs-vsctl set open .
> external-ids:ovn-remote-probe-interval=180000
>      m_as $c ovs-vsctl set open .
> external-ids:ovn-bridge-datapath-type=system
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thanks,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to