Thanks for the review Han. Have addressed the comments and have split it into 2 patches: https://patchwork.ozlabs.org/patch/931228/ and https://patchwork.ozlabs.org/patch/931229/ . Please review the same and let me know for questions further.
Regards, On Mon, Jun 18, 2018 at 12:08 PM, Han Zhou <[email protected]> wrote: > Hi Ali, thanks for the fix. I reviewed and have some minor improvements > suggested. > > On Sun, Jun 3, 2018 at 7:50 PM, aginwala <[email protected]> wrote: > > > > only for master node with remote option when using load balancer to > manage > > ovndb clusters via pacemaker. > > > > This is will allow setting inactivity probe on the masters. > > For pacemaker to manage ovndb resources via LB, we skipped creating > connection > > table and hence the inactivity probe was getting set to 5000 by default. > > In order to over-ride it we need this table. However, we need to skip > slaves > > listening on local sb and nb connections table so that LB feature is > > intact and only master is listening on 0.0.0.0 > > > > e.g --remote=db:OVN_Southbound,SB_Global,connections and > > --remote=db:OVN_Northbound,NB_Global,connections > > > > will only be set on master node when using load balancer for sb and nb > dbs > > respectively. > > > > I feel it might make the purpose more clear if we split this into 2 > patches. > > 1). Allow ovn-ctl to start OVSDBs without using remote connections from DB > tables. > > 2). Set connections in DB tables in pacemaker ocf file, so that we can > adjust inactivity_probe for master node, while still not listening on TCP > on slave node with the help of change 1). > > > Signed-off-by: aginwala <[email protected]> > > --- > > ovn/utilities/ovn-ctl | 6 +++++- > > ovn/utilities/ovndb-servers.ocf | 31 ++++++++++++++++++------------- > > 2 files changed, 23 insertions(+), 14 deletions(-) > > > > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl > > index 4b7eef5..2df8580 100755 > > --- a/ovn/utilities/ovn-ctl > > +++ b/ovn/utilities/ovn-ctl > > @@ -176,7 +176,9 @@ $cluster_remote_port > > set exec "$@" > > fi > > > > - set "$@" --remote=db:$schema_name,$table_name,connections > > + if test X"$OVN_USE_REMOTE_CONN" != Xno; then > > + set "$@" --remote=db:$schema_name,$table_name,connections > > + 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 > > @@ -463,6 +465,7 @@ set_defaults () { > > > > OVN_NORTHD_NB_DB="unix:$DB_NB_SOCK" > > OVN_NORTHD_SB_DB="unix:$DB_SB_SOCK" > > + OVN_USE_REMOTE_CONN="yes" > > } > > > > set_option () { > > @@ -577,6 +580,7 @@ File location options: > > transport (default: $DB_SB_CLUSTER_REMOTE_PROTO) > > --ovn-northd-nb-db=NB DB address(es) (default: $OVN_NORTHD_NB_DB) > > --ovn-northd-sb-db=SB DB address(es) (default: $OVN_NORTHD_SB_DB) > > + --ovn-use-remote-conn=yes|no Listen on target connection table > (default: $OVN_USE_REMOTE_CONN) > > The name of the new option is confusing, since it doesn't control whether > to use remote connection, but just controls the whether to use the > connections configured in DB. > > Secondly, it is better to follow the pattern that all OVSDB options has NB > and SB separately. > > So, suggest to change to something like: --db-nb-use-remote-in-db and > --db-sb-use-remote-in-db. > > > > > Default directories with "configure" option and environment variable > override: > > logs: /usr/local/var/log/openvswitch (--with-logdir, OVS_LOGDIR) > > diff --git a/ovn/utilities/ovndb-servers.ocf > b/ovn/utilities/ovndb-servers.ocf > > index 9391b89..25b4811 100755 > > --- a/ovn/utilities/ovndb-servers.ocf > > +++ b/ovn/utilities/ovndb-servers.ocf > > @@ -172,25 +172,27 @@ ovsdb_server_notify() { > > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd > > fi > > > > - # Not needed while listening on 0.0.0.0 as we do not want to > allow > > - # local binds. However, it is needed if vip ip is binded to > nodes. > > - if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then > > - conn=`ovn-nbctl get NB_global . connections` > > - if [ "$conn" == "[]" ] > > - then > > - ovn-nbctl -- --id=@conn_uuid create Connection \ > > + # In order to over-ride inactivity_probe for LB use case, we > need to > > + # create connection entry to listen on 0.0.0.0 for master node. > > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then > > + MASTER_IP="0.0.0.0" > > Since MASTER_IP is the global variable that comes from the configuration, > it is better not change it directly, but instead use another temporary > variable to achieve the same purpose. > > > + fi > > + conn=`ovn-nbctl get NB_global . connections` > > + if [ "$conn" == "[]" ] > > + then > > + ovn-nbctl -- --id=@conn_uuid create Connection \ > > target="p${NB_MASTER_PROTO}\:${NB_MASTER_PORT}\:${MASTER_IP}" \ > > inactivity_probe=$INACTIVE_PROBE -- set NB_Global . > connections=@conn_uuid > > - fi > > + fi > > > > - conn=`ovn-sbctl get SB_global . connections` > > - if [ "$conn" == "[]" ] > > - then > > - ovn-sbctl -- --id=@conn_uuid create Connection \ > > + conn=`ovn-sbctl get SB_global . connections` > > + if [ "$conn" == "[]" ] > > + then > > + ovn-sbctl -- --id=@conn_uuid create Connection \ > > target="p${SB_MASTER_PROTO}\:${SB_MASTER_PORT}\:${MASTER_IP}" \ > > inactivity_probe=$INACTIVE_PROBE -- set SB_Global . > connections=@conn_uuid > > - fi > > fi > > + > > else > > if [ "$MANAGE_NORTHD" = "yes" ]; then > > # Stop ovn-northd service. Set --ovn-manage-ovsdb=no so that > > @@ -355,6 +357,9 @@ ovsdb_server_start() { > > set $@ --db-nb-sync-from-proto=${NB_MASTER_PROTO} > > set $@ --db-sb-sync-from-port=${SB_MASTER_PORT} > > set $@ --db-sb-sync-from-proto=${SB_MASTER_PROTO} > > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then > > + set $@ --ovn-use-remote-conn="no" > > + fi > > > > fi > > > > -- > > 1.9.1 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > Thanks, > Han > _______________________________________________ > 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
