Thanks for the review Han. Please find the comments inline below: On Thu, Oct 4, 2018 at 9:58 AM Han Zhou <[email protected]> wrote:
> Thanks Ali, please see my comments below > > On Fri, Sep 21, 2018 at 5:34 PM <[email protected]> wrote: > > > > For OVN DBs to work with SSL in HA, we need to have capability to > > pass ssl certs when starting OVN DBs. Say when starting OVN DBs in > active > > passive mode, in order for the standby DBs to sync from master node, it > > cannot sync because the required ssl certs are not passed when standby > DBs > > are initialized. Hence, we need to have this option. > > > > e.g. start nb db with ssl certs as below: > > /usr/share/openvswitch/scripts/ovn-ctl > --ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \ > > --ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \ > > --ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \ > > --db-nb-create-insecure-remote=no start_nb_ovsdb > > > > Certs can be generated based on ovs ssl docs: > > http://docs.openvswitch.org/en/latest/howto/ssl/ > > > > Signed-off-by: aginwala <[email protected]> > > --- > > ovn/utilities/ovn-ctl | 50 > +++++++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 43 insertions(+), 7 deletions(-) > > > > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl > > index 3ff0df6..4f45f3d 100755 > > --- a/ovn/utilities/ovn-ctl > > +++ b/ovn/utilities/ovn-ctl > > @@ -116,6 +116,9 @@ start_ovsdb__() { > > local addr > > local active_conf_file > > local use_remote_in_db > > + local ovn_db_ssl_key > > + local ovn_db_ssl_cert > > + local ovn_db_ssl_cacert > > eval pid=\$DB_${DB}_PID > > eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR > > eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT > > @@ -137,6 +140,9 @@ start_ovsdb__() { > > eval addr=\$DB_${DB}_ADDR > > eval active_conf_file=\$ovn${db}_active_conf_file > > eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB > > + eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY > > + eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT > > + eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT > > > > # Check and eventually start ovsdb-server for DB > > if pidfile_is_running $pid; then > > @@ -182,17 +188,32 @@ $cluster_remote_port > > > > if test X"$use_remote_in_db" != Xno; then > > set "$@" --remote=db:$schema_name,$table_name,connections > > + if test X"$create_insecure_remote" = Xno; then > > + set "$@" --remote=pssl:$port:$addr > > + elif test X"$create_insecure_remote" = Xyes; then > > + set "$@" --remote=ptcp:$port:$addr > > + fi > Why moving the logic here? This if block only says if the connection > settings in DB table should be used. Whether insecure mode is allowed was > supposed to be independent with this condition. Could you explain the > reason behind the change? > >> I moved it because remote=db is needed if ovsdb is running as a > standalone node or an active ovsdb server node. For standby nodes in case > of active_passive mode, remote=db will not be there because it uses > --sync-from. Hope its clear. > > > fi > > - set "$@" --private-key=db:$schema_name,SSL,private_key > > - set "$@" --certificate=db:$schema_name,SSL,certificate > > - set "$@" --ca-cert=db:$schema_name,SSL,ca_cert > > - set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols > > - set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers > > So it will not use the settings in DB any more? It seems this change > removed support for "--use-remote-in-db=yes", which is the default behavior > that should be kept. The DB settings should not be used only if > "--use-remote-in-db=no" > >> As discussed, this is similar support that we have for ovn-controller with ssl. If the key is passed from cli, it will use the key else fall back to default setters for ssl. > > > > - if test X"$create_insecure_remote" = Xyes; then > > - set "$@" --remote=ptcp:$port:$addr > > + if test X"$ovn_db_ssl_key" != X; then > > + set "$@" --private-key=$ovn_db_ssl_key > > + else > > + set "$@" --private-key=db:$schema_name,SSL,private_key > > + fi > > + if test X"$ovn_db_ssl_cert" != X; then > > + set "$@" --certificate=$ovn_db_ssl_cert > > + else > > + set "$@" --certificate=db:$schema_name,SSL,certificate > > + fi > > + if test X"$ovn_db_ssl_cacert" != X; then > > + set "$@" --ca-cert=$ovn_db_ssl_cacert > > + else > > + set "$@" --ca-cert=db:$schema_name,SSL,ca_cert > > fi > > > > + set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols > > + set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers > > + > > if test $mode = active_passive; then > > set "$@" --sync-from=`cat $active_conf_file` > > fi > > @@ -481,6 +502,15 @@ set_defaults () { > > OVN_NORTHD_SB_DB="unix:$DB_SB_SOCK" > > DB_NB_USE_REMOTE_IN_DB="yes" > > DB_SB_USE_REMOTE_IN_DB="yes" > > + > > + OVN_NB_DB_SSL_KEY="" > > + OVN_NB_DB_SSL_CERT="" > > + OVN_NB_DB_SSL_CA_CERT="" > > + > > + OVN_SB_DB_SSL_KEY="" > > + OVN_SB_DB_SSL_CERT="" > > + OVN_SB_DB_SSL_CA_CERT="" > > + > > } > > > > set_option () { > > @@ -536,6 +566,12 @@ Options: > > --ovn-controller-ssl-cert=CERT OVN Southbound SSL certificate file > > --ovn-controller-ssl-ca-cert=CERT OVN Southbound SSL CA certificate > file > > --ovn-controller-ssl-bootstrap-ca-cert=CERT Bootstrapped OVN > Southbound SSL CA certificate file > > + --ovn-nb-db-ssl-key=KEY OVN Northbound DB SSL private key file > > + --ovn-nb-db-ssl-cert=CERT OVN Northbound DB SSL certificate file > > + --ovn-nb-db-ssl-ca-cert=CERT OVN Northbound DB SSL CA certificate file > > + --ovn-sb-db-ssl-key=KEY OVN Southbound DB SSL private key file > > + --ovn-sb-db-ssl-cert=CERT OVN Southbound DB SSL certificate file > > + --ovn-sb-db-ssl-ca-cert=CERT OVN Southbound DB SSL CA certificate file > > --ovn-manage-ovsdb=yes|no Whether or not the OVN databases > should be > > automatically started and stopped > along > > with ovn-northd. The default is > "yes". If > > -- > > 1.9.1 > > > > _______________________________________________ > > 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
