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