| Message ID | 1525804993-22596-1-git-send-email-aginwala@ebay.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v2] ovndb-servers.ocf: add support for managing cluster | expand |
On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> wrote: > using pacemaker so that controllers can be placed in different fault > domains. > > Signed-off-by: aginwala <aginwala@ebay.com> > I see the below warning when applying the patch .git/rebase-apply/patch:153: new blank line at EOF. + warning: 1 line adds whitespace errors. > --- > ovn/utilities/ovndb-servers.ocf | 83 ++++++++++++++++++++++++++++++ > ++--------- > 1 file changed, 65 insertions(+), 18 deletions(-) > > diff --git a/ovn/utilities/ovndb-servers.ocf > b/ovn/utilities/ovndb-servers.ocf > index 164b6bc..1b4b6ab 100755 > --- a/ovn/utilities/ovndb-servers.ocf > +++ b/ovn/utilities/ovndb-servers.ocf > @@ -9,6 +9,8 @@ > : ${SB_MASTER_PROTO_DEFAULT="tcp"} > : ${MANAGE_NORTHD_DEFAULT="no"} > : ${INACTIVE_PROBE_DEFAULT="5000"} > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} > > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_ > MASTER_PROTO_DEFAULT}} > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}} > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ > INACTIVE_PROBE_DEFAULT}} > > +# In order for pacemaker to work with LB, we can keep > LISTEN_ON_MASTER_IP_ONLY > +# to false and pass LB vip IP while creating pcs resource. > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} > + > +# In order for pacemaker to work with LB, we can also set LISTEN_ON_SLAVE > +# to false so that slaves do not listen on 0.0.0.0. > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_SLAVE_DEFAULT}} > You need to define these parameters in the <parameters> section here - https://github.com/openvswitch/ovs/blob/master/ovn/utilities/ovndb-servers.ocf#L118 Otherwise we will not be able to pass these options when starting the resource. Ideally we want to do "pcs resource create ovndb_servers .... listen_on_master_ip_only=no listen_on_slave=no + > # Invalid IP address is an address that can never exist in the network, as > # mentioned in rfc-5737. The ovsdb servers connects to this IP address > till > # a master is promoted and the IPAddr2 resource is started. > @@ -157,22 +167,24 @@ ovsdb_server_notify() { > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd > fi > > - conn=`ovn-nbctl get NB_global . connections` > - if [ "$conn" == "[]" ] > - then > - ovn-nbctl -- --id=@conn_uuid create Connection \ > + # TODO: Need to troubleshoot as to removing target is ok as well. + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then > If LB VIP is used, you don't want to set the probe interval ? > + 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 > @@ -295,15 +307,15 @@ ovsdb_server_start() { > > set ${OVN_CTL} > > - set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} > - set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} > + # For LB vip to talk to master pool member on a specific tcp port, we > need > + # to listen on 0.0.0.0.instead of master_ip > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then > + set $@ --db-nb-port=${NB_MASTER_PORT} > + set $@ --db-sb-port=${SB_MASTER_PORT} > > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > - set $@ --db-nb-create-insecure-remote=yes > - fi > - > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > - set $@ --db-sb-create-insecure-remote=yes > + else > + set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} > + set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} > fi > > if [ "x${present_master}" = x ]; then > @@ -313,15 +325,44 @@ ovsdb_server_start() { > # Force all copies to come up as slaves by pointing them into > # space and let pacemaker pick one to promote: > # > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > + set $@ --db-nb-create-insecure-remote=yes > + fi > + > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > + set $@ --db-sb-create-insecure-remote=yes > + fi > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} > > elif [ ${present_master} != ${host_name} ]; then > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then > + # TODO: for using LB vip, need to test for ssl. > + set $@ --db-nb-create-insecure-remote=no > The default value of "db-nb-create-insecure-remote" is no. So there is no need to set this. We can just have if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then if ... ... fi + set $@ --db-sb-create-insecure-remote=no > + else > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > + set $@ --db-nb-create-insecure-remote=yes > + fi > + > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > + set $@ --db-sb-create-insecure-remote=yes > + fi > + fi > # An existing master is active, connect to it > set $@ --db-nb-sync-from-addr=${MASTER_IP} > --db-sb-sync-from-addr=${MASTER_IP} > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} > 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} > + > + else > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > + set $@ --db-nb-create-insecure-remote=yes > + fi > + > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > + set $@ --db-sb-create-insecure-remote=yes > + fi > fi > > $@ start_ovsdb > @@ -416,6 +457,11 @@ ovsdb_server_promote() { > ;; > esac > > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very confusing. I think it should be possible to have just one param - something like - "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). Thoughts ? Will there be a case where the user can select listen_on_master_ip_only=no and listen_on_slave=yes ? Thanks Numan > + # Restart ovs so that new master can listen on tcp port > + ${OVN_CTL} stop_ovsdb > + ovsdb_server_start > + fi > ${OVN_CTL} promote_ovnnb > ${OVN_CTL} promote_ovnsb > > @@ -514,3 +560,4 @@ esac > > rc=$? > exit $rc > + > -- > 1.9.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Thanks for the Review. On Wed, May 23, 2018 at 11:36 AM, Numan Siddique <nusiddiq@redhat.com> wrote: > On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> wrote: > > > using pacemaker so that controllers can be placed in different fault > > domains. > > > > Signed-off-by: aginwala <aginwala@ebay.com> > > > > I see the below warning when applying the patch > > .git/rebase-apply/patch:153: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > >>> Sure. Will fix. > > > --- > > ovn/utilities/ovndb-servers.ocf | 83 ++++++++++++++++++++++++++++++ > > ++--------- > > 1 file changed, 65 insertions(+), 18 deletions(-) > > > > diff --git a/ovn/utilities/ovndb-servers.ocf > > b/ovn/utilities/ovndb-servers.ocf > > index 164b6bc..1b4b6ab 100755 > > --- a/ovn/utilities/ovndb-servers.ocf > > +++ b/ovn/utilities/ovndb-servers.ocf > > @@ -9,6 +9,8 @@ > > : ${SB_MASTER_PROTO_DEFAULT="tcp"} > > : ${MANAGE_NORTHD_DEFAULT="no"} > > : ${INACTIVE_PROBE_DEFAULT="5000"} > > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} > > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} > > > > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" > > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config > > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" > > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_ > sb_master_protocol:-${SB_ > > MASTER_PROTO_DEFAULT}} > > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}} > > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ > > INACTIVE_PROBE_DEFAULT}} > > > > +# In order for pacemaker to work with LB, we can keep > > LISTEN_ON_MASTER_IP_ONLY > > +# to false and pass LB vip IP while creating pcs resource. > > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ > > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} > > + > > +# In order for pacemaker to work with LB, we can also set > LISTEN_ON_SLAVE > > +# to false so that slaves do not listen on 0.0.0.0. > > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_ > SLAVE_DEFAULT}} > > > > > You need to define these parameters in the <parameters> section here - > https://github.com/openvswitch/ovs/blob/master/ > ovn/utilities/ovndb-servers.ocf#L118 > Otherwise we will not be able to pass these options when starting the > resource. Ideally we want to do > "pcs resource create ovndb_servers .... listen_on_master_ip_only=no > listen_on_slave=no >> Thanks for the pointer. I will update that and re-test. > > > + > > # Invalid IP address is an address that can never exist in the network, > as > > # mentioned in rfc-5737. The ovsdb servers connects to this IP address > > till > > # a master is promoted and the IPAddr2 resource is started. > > @@ -157,22 +167,24 @@ ovsdb_server_notify() { > > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd > > fi > > > > - conn=`ovn-nbctl get NB_global . connections` > > - if [ "$conn" == "[]" ] > > - then > > - ovn-nbctl -- --id=@conn_uuid create Connection \ > > + # TODO: Need to troubleshoot as to removing target is ok as > well. > > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then > > > > If LB VIP is used, you don't want to set the probe interval ? >>> I thought we can stick with common probe interval for any case. Hence, kept it. > > > + 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 > > @@ -295,15 +307,15 @@ ovsdb_server_start() { > > > > set ${OVN_CTL} > > > > - set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} > > - set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} > > + # For LB vip to talk to master pool member on a specific tcp port, > we > > need > > + # to listen on 0.0.0.0.instead of master_ip > > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then > > + set $@ --db-nb-port=${NB_MASTER_PORT} > > + set $@ --db-sb-port=${SB_MASTER_PORT} > > > > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > > - set $@ --db-nb-create-insecure-remote=yes > > - fi > > - > > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > > - set $@ --db-sb-create-insecure-remote=yes > > + else > > + set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} > > + set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} > > fi > > > > if [ "x${present_master}" = x ]; then > > @@ -313,15 +325,44 @@ ovsdb_server_start() { > > # Force all copies to come up as slaves by pointing them into > > # space and let pacemaker pick one to promote: > > # > > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > > + set $@ --db-nb-create-insecure-remote=yes > > + fi > > + > > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > > + set $@ --db-sb-create-insecure-remote=yes > > + fi > > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} > > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} > > > > elif [ ${present_master} != ${host_name} ]; then > > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then > > + # TODO: for using LB vip, need to test for ssl. > > + set $@ --db-nb-create-insecure-remote=no > > > > The default value of "db-nb-create-insecure-remote" is no. So there is no > need to set this. We can just have > if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then > if ... > ... > fi >>> Sure let me re-change and test. Will update the new patch accordingly. > + set $@ --db-sb-create-insecure-remote=no > > + else > > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > > + set $@ --db-nb-create-insecure-remote=yes > > + fi > > + > > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > > + set $@ --db-sb-create-insecure-remote=yes > > + fi > > + fi > > # An existing master is active, connect to it > > set $@ --db-nb-sync-from-addr=${MASTER_IP} > > --db-sb-sync-from-addr=${MASTER_IP} > > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} > > 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} > > + > > + else > > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > > + set $@ --db-nb-create-insecure-remote=yes > > + fi > > + > > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > > + set $@ --db-sb-create-insecure-remote=yes > > + fi > > fi > > > > $@ start_ovsdb > > @@ -416,6 +457,11 @@ ovsdb_server_promote() { > > ;; > > esac > > > > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then > > > The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very > confusing. I think it should > be possible to have just one param - something like - > "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). > Thoughts ? >>> Sure, how about master_ip_lb_resource to avoid confusion? So you mean, just use this variable and completely get rid of LISTEN_ON_SLAVE and LISTEN_ON_MASTER_IP_ONLY? > Will there be a case where the user can select > listen_on_master_ip_only=no and > listen_on_slave=yes ? > >>> No. Thanks > Numan > > > > > + # Restart ovs so that new master can listen on tcp port > > + ${OVN_CTL} stop_ovsdb > > + ovsdb_server_start > > + fi > > ${OVN_CTL} promote_ovnnb > > ${OVN_CTL} promote_ovnsb > > > > @@ -514,3 +560,4 @@ esac > > > > rc=$? > > exit $rc > > + > > -- > > 1.9.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Wed, May 30, 2018 at 5:31 AM, aginwala <aginwala@asu.edu> wrote: > Thanks for the Review. > > On Wed, May 23, 2018 at 11:36 AM, Numan Siddique <nusiddiq@redhat.com> > wrote: > >> On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> wrote: >> >> > using pacemaker so that controllers can be placed in different fault >> > domains. >> > >> > Signed-off-by: aginwala <aginwala@ebay.com> >> > >> >> I see the below warning when applying the patch >> >> .git/rebase-apply/patch:153: new blank line at EOF. >> + >> warning: 1 line adds whitespace errors. >> > >>> Sure. Will fix. > >> >> > --- >> > ovn/utilities/ovndb-servers.ocf | 83 ++++++++++++++++++++++++++++++ >> >> > ++--------- >> > 1 file changed, 65 insertions(+), 18 deletions(-) >> > >> > diff --git a/ovn/utilities/ovndb-servers.ocf >> > b/ovn/utilities/ovndb-servers.ocf >> > index 164b6bc..1b4b6ab 100755 >> > --- a/ovn/utilities/ovndb-servers.ocf >> > +++ b/ovn/utilities/ovndb-servers.ocf >> > @@ -9,6 +9,8 @@ >> > : ${SB_MASTER_PROTO_DEFAULT="tcp"} >> > : ${MANAGE_NORTHD_DEFAULT="no"} >> > : ${INACTIVE_PROBE_DEFAULT="5000"} >> > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} >> > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} >> > >> > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" >> > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config >> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" >> > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_s >> b_master_protocol:-${SB_ >> > MASTER_PROTO_DEFAULT}} >> > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}} >> > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ >> > INACTIVE_PROBE_DEFAULT}} >> > >> > +# In order for pacemaker to work with LB, we can keep >> > LISTEN_ON_MASTER_IP_ONLY >> > +# to false and pass LB vip IP while creating pcs resource. >> > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ >> > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} >> > + >> > +# In order for pacemaker to work with LB, we can also set >> LISTEN_ON_SLAVE >> > +# to false so that slaves do not listen on 0.0.0.0. >> > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_S >> LAVE_DEFAULT}} >> > >> >> >> You need to define these parameters in the <parameters> section here - >> https://github.com/openvswitch/ovs/blob/master/ovn/ >> utilities/ovndb-servers.ocf#L118 >> Otherwise we will not be able to pass these options when starting the >> resource. Ideally we want to do >> "pcs resource create ovndb_servers .... listen_on_master_ip_only=no >> listen_on_slave=no >> > >> Thanks for the pointer. I will update that and re-test. > >> >> >> + >> > # Invalid IP address is an address that can never exist in the >> network, as >> > # mentioned in rfc-5737. The ovsdb servers connects to this IP address >> > till >> > # a master is promoted and the IPAddr2 resource is started. >> > @@ -157,22 +167,24 @@ ovsdb_server_notify() { >> > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd >> > fi >> > >> > - conn=`ovn-nbctl get NB_global . connections` >> > - if [ "$conn" == "[]" ] >> > - then >> > - ovn-nbctl -- --id=@conn_uuid create Connection \ >> > + # TODO: Need to troubleshoot as to removing target is ok as >> well. >> >> + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then >> > >> >> If LB VIP is used, you don't want to set the probe interval ? > > >>> I thought we can stick with common probe interval for any case. Hence, > kept it. > >> >> > + 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 >> > @@ -295,15 +307,15 @@ ovsdb_server_start() { >> > >> > set ${OVN_CTL} >> > >> > - set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} >> > - set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} >> > + # For LB vip to talk to master pool member on a specific tcp port, >> we >> > need >> > + # to listen on 0.0.0.0.instead of master_ip >> > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then >> > + set $@ --db-nb-port=${NB_MASTER_PORT} >> > + set $@ --db-sb-port=${SB_MASTER_PORT} >> > >> > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >> > - set $@ --db-nb-create-insecure-remote=yes >> > - fi >> > - >> > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >> > - set $@ --db-sb-create-insecure-remote=yes >> > + else >> > + set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} >> > + set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} >> > fi >> > >> > if [ "x${present_master}" = x ]; then >> > @@ -313,15 +325,44 @@ ovsdb_server_start() { >> > # Force all copies to come up as slaves by pointing them into >> > # space and let pacemaker pick one to promote: >> > # >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >> > + set $@ --db-nb-create-insecure-remote=yes >> > + fi >> > + >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >> > + set $@ --db-sb-create-insecure-remote=yes >> > + fi >> > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} >> > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} >> > >> > elif [ ${present_master} != ${host_name} ]; then >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >> > + # TODO: for using LB vip, need to test for ssl. >> > + set $@ --db-nb-create-insecure-remote=no >> > >> >> The default value of "db-nb-create-insecure-remote" is no. So there is no >> need to set this. We can just have >> if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then >> if ... >> ... >> fi > > >>> Sure let me re-change and test. Will update the new patch accordingly. > >> + set $@ --db-sb-create-insecure-remote=no >> > + else >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >> > + set $@ --db-nb-create-insecure-remote=yes >> > + fi >> > + >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >> > + set $@ --db-sb-create-insecure-remote=yes >> > + fi >> > + fi >> > # An existing master is active, connect to it >> > set $@ --db-nb-sync-from-addr=${MASTER_IP} >> > --db-sb-sync-from-addr=${MASTER_IP} >> > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} >> > 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} >> > + >> > + else >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >> > + set $@ --db-nb-create-insecure-remote=yes >> > + fi >> > + >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >> > + set $@ --db-sb-create-insecure-remote=yes >> > + fi >> > fi >> > >> > $@ start_ovsdb >> > @@ -416,6 +457,11 @@ ovsdb_server_promote() { >> > ;; >> > esac >> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >> >> >> The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very >> confusing. I think it should >> be possible to have just one param - something like - >> "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). >> Thoughts ? > > >>> Sure, how about master_ip_lb_resource to avoid confusion? So you > mean, just use this variable and completely get rid of LISTEN_ON_SLAVE > and LISTEN_ON_MASTER_IP_ONLY? > 'master_ip_lb_resource' seems fine to me. Thats' right. Just have one parameter. Will there be a case where the user can select >> listen_on_master_ip_only=no and >> listen_on_slave=yes ? >> > >>> No. > > Thanks >> Numan >> >> >> >> > + # Restart ovs so that new master can listen on tcp port >> > + ${OVN_CTL} stop_ovsdb >> > + ovsdb_server_start >> > + fi >> > ${OVN_CTL} promote_ovnnb >> > ${OVN_CTL} promote_ovnsb >> > >> > @@ -514,3 +560,4 @@ esac >> > >> > rc=$? >> > exit $rc >> > + >> > -- >> > 1.9.1 >> > >> > _______________________________________________ >> > dev mailing list >> > dev@openvswitch.org >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >
Sounds good! Addressed the comments in V3 @ https://patchwork.ozlabs. org/patch/923040/ . Please help review the same and let me know further. On Tue, May 29, 2018 at 11:40 PM, Numan Siddique <nusiddiq@redhat.com> wrote: > > > On Wed, May 30, 2018 at 5:31 AM, aginwala <aginwala@asu.edu> wrote: > >> Thanks for the Review. >> >> On Wed, May 23, 2018 at 11:36 AM, Numan Siddique <nusiddiq@redhat.com> >> wrote: >> >>> On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> wrote: >>> >>> > using pacemaker so that controllers can be placed in different fault >>> > domains. >>> > >>> > Signed-off-by: aginwala <aginwala@ebay.com> >>> > >>> >>> I see the below warning when applying the patch >>> >>> .git/rebase-apply/patch:153: new blank line at EOF. >>> + >>> warning: 1 line adds whitespace errors. >>> >> >>> Sure. Will fix. >> >>> >>> > --- >>> > ovn/utilities/ovndb-servers.ocf | 83 ++++++++++++++++++++++++++++++ >>> >>> > ++--------- >>> > 1 file changed, 65 insertions(+), 18 deletions(-) >>> > >>> > diff --git a/ovn/utilities/ovndb-servers.ocf >>> > b/ovn/utilities/ovndb-servers.ocf >>> > index 164b6bc..1b4b6ab 100755 >>> > --- a/ovn/utilities/ovndb-servers.ocf >>> > +++ b/ovn/utilities/ovndb-servers.ocf >>> > @@ -9,6 +9,8 @@ >>> > : ${SB_MASTER_PROTO_DEFAULT="tcp"} >>> > : ${MANAGE_NORTHD_DEFAULT="no"} >>> > : ${INACTIVE_PROBE_DEFAULT="5000"} >>> > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} >>> > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} >>> > >>> > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" >>> > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config >>> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" >>> > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_s >>> b_master_protocol:-${SB_ >>> > MASTER_PROTO_DEFAULT}} >>> > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}} >>> > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ >>> > INACTIVE_PROBE_DEFAULT}} >>> > >>> > +# In order for pacemaker to work with LB, we can keep >>> > LISTEN_ON_MASTER_IP_ONLY >>> > +# to false and pass LB vip IP while creating pcs resource. >>> > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ >>> > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} >>> > + >>> > +# In order for pacemaker to work with LB, we can also set >>> LISTEN_ON_SLAVE >>> > +# to false so that slaves do not listen on 0.0.0.0. >>> > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_S >>> LAVE_DEFAULT}} >>> > >>> >>> >>> You need to define these parameters in the <parameters> section here - >>> https://github.com/openvswitch/ovs/blob/master/ovn/utilities >>> /ovndb-servers.ocf#L118 >>> Otherwise we will not be able to pass these options when starting the >>> resource. Ideally we want to do >>> "pcs resource create ovndb_servers .... listen_on_master_ip_only=no >>> listen_on_slave=no >>> >> >> Thanks for the pointer. I will update that and re-test. >> >>> >>> >>> + >>> > # Invalid IP address is an address that can never exist in the >>> network, as >>> > # mentioned in rfc-5737. The ovsdb servers connects to this IP address >>> > till >>> > # a master is promoted and the IPAddr2 resource is started. >>> > @@ -157,22 +167,24 @@ ovsdb_server_notify() { >>> > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd >>> > fi >>> > >>> > - conn=`ovn-nbctl get NB_global . connections` >>> > - if [ "$conn" == "[]" ] >>> > - then >>> > - ovn-nbctl -- --id=@conn_uuid create Connection \ >>> > + # TODO: Need to troubleshoot as to removing target is ok as >>> well. >>> >>> + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then >>> > >>> >>> If LB VIP is used, you don't want to set the probe interval ? >> >> >>> I thought we can stick with common probe interval for any case. >> Hence, kept it. >> >>> >>> > + 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 >>> > @@ -295,15 +307,15 @@ ovsdb_server_start() { >>> > >>> > set ${OVN_CTL} >>> > >>> > - set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} >>> > - set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} >>> > + # For LB vip to talk to master pool member on a specific tcp >>> port, we >>> > need >>> > + # to listen on 0.0.0.0.instead of master_ip >>> > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then >>> > + set $@ --db-nb-port=${NB_MASTER_PORT} >>> > + set $@ --db-sb-port=${SB_MASTER_PORT} >>> > >>> > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>> > - set $@ --db-nb-create-insecure-remote=yes >>> > - fi >>> > - >>> > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>> > - set $@ --db-sb-create-insecure-remote=yes >>> > + else >>> > + set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} >>> > + set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} >>> > fi >>> > >>> > if [ "x${present_master}" = x ]; then >>> > @@ -313,15 +325,44 @@ ovsdb_server_start() { >>> > # Force all copies to come up as slaves by pointing them into >>> > # space and let pacemaker pick one to promote: >>> > # >>> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>> > + set $@ --db-nb-create-insecure-remote=yes >>> > + fi >>> > + >>> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>> > + set $@ --db-sb-create-insecure-remote=yes >>> > + fi >>> > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} >>> > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} >>> > >>> > elif [ ${present_master} != ${host_name} ]; then >>> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >>> > + # TODO: for using LB vip, need to test for ssl. >>> > + set $@ --db-nb-create-insecure-remote=no >>> > >>> >>> The default value of "db-nb-create-insecure-remote" is no. So there is >>> no >>> need to set this. We can just have >>> if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then >>> if ... >>> ... >>> fi >> >> >>> Sure let me re-change and test. Will update the new patch >> accordingly. >> >>> + set $@ --db-sb-create-insecure-remote=no >>> > + else >>> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>> > + set $@ --db-nb-create-insecure-remote=yes >>> > + fi >>> > + >>> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>> > + set $@ --db-sb-create-insecure-remote=yes >>> > + fi >>> > + fi >>> > # An existing master is active, connect to it >>> > set $@ --db-nb-sync-from-addr=${MASTER_IP} >>> > --db-sb-sync-from-addr=${MASTER_IP} >>> > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} >>> > 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} >>> > + >>> > + else >>> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>> > + set $@ --db-nb-create-insecure-remote=yes >>> > + fi >>> > + >>> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>> > + set $@ --db-sb-create-insecure-remote=yes >>> > + fi >>> > fi >>> > >>> > $@ start_ovsdb >>> > @@ -416,6 +457,11 @@ ovsdb_server_promote() { >>> > ;; >>> > esac >>> > >>> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >>> >>> >>> The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very >>> confusing. I think it should >>> be possible to have just one param - something like - >>> "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). >>> Thoughts ? >> >> >>> Sure, how about master_ip_lb_resource to avoid confusion? So you >> mean, just use this variable and completely get rid of LISTEN_ON_SLAVE >> and LISTEN_ON_MASTER_IP_ONLY? >> > > 'master_ip_lb_resource' seems fine to me. Thats' right. Just have one > parameter. > > > Will there be a case where the user can select >>> listen_on_master_ip_only=no and >>> listen_on_slave=yes ? >>> >> >>> No. >> >> Thanks >>> Numan >>> >>> >>> >>> > + # Restart ovs so that new master can listen on tcp port >>> > + ${OVN_CTL} stop_ovsdb >>> > + ovsdb_server_start >>> > + fi >>> > ${OVN_CTL} promote_ovnnb >>> > ${OVN_CTL} promote_ovnsb >>> > >>> > @@ -514,3 +560,4 @@ esac >>> > >>> > rc=$? >>> > exit $rc >>> > + >>> > -- >>> > 1.9.1 >>> > >>> > _______________________________________________ >>> > dev mailing list >>> > dev@openvswitch.org >>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> > >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >> >
On Tue, May 29, 2018 at 11:40 PM, Numan Siddique <nusiddiq@redhat.com> wrote: > > On Wed, May 30, 2018 at 5:31 AM, aginwala <aginwala@asu.edu> wrote: > > > Thanks for the Review. > > > > On Wed, May 23, 2018 at 11:36 AM, Numan Siddique <nusiddiq@redhat.com> > > wrote: > > > >> On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> wrote: > >> > >> > using pacemaker so that controllers can be placed in different fault > >> > domains. > >> > > >> > Signed-off-by: aginwala <aginwala@ebay.com> > >> > > >> > >> I see the below warning when applying the patch > >> > >> .git/rebase-apply/patch:153: new blank line at EOF. > >> + > >> warning: 1 line adds whitespace errors. > >> > > >>> Sure. Will fix. > > > >> > >> > --- > >> > ovn/utilities/ovndb-servers.ocf | 83 ++++++++++++++++++++++++++++++ > >> > >> > ++--------- > >> > 1 file changed, 65 insertions(+), 18 deletions(-) > >> > > >> > diff --git a/ovn/utilities/ovndb-servers.ocf > >> > b/ovn/utilities/ovndb-servers.ocf > >> > index 164b6bc..1b4b6ab 100755 > >> > --- a/ovn/utilities/ovndb-servers.ocf > >> > +++ b/ovn/utilities/ovndb-servers.ocf > >> > @@ -9,6 +9,8 @@ > >> > : ${SB_MASTER_PROTO_DEFAULT="tcp"} > >> > : ${MANAGE_NORTHD_DEFAULT="no"} > >> > : ${INACTIVE_PROBE_DEFAULT="5000"} > >> > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} > >> > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} > >> > > >> > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" > >> > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config > >> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" > >> > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_s > >> b_master_protocol:-${SB_ > >> > MASTER_PROTO_DEFAULT}} > >> > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}} > >> > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ > >> > INACTIVE_PROBE_DEFAULT}} > >> > > >> > +# In order for pacemaker to work with LB, we can keep > >> > LISTEN_ON_MASTER_IP_ONLY > >> > +# to false and pass LB vip IP while creating pcs resource. > >> > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ > >> > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} > >> > + > >> > +# In order for pacemaker to work with LB, we can also set > >> LISTEN_ON_SLAVE > >> > +# to false so that slaves do not listen on 0.0.0.0. > >> > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_S > >> LAVE_DEFAULT}} > >> > > >> > >> > >> You need to define these parameters in the <parameters> section here - > >> https://github.com/openvswitch/ovs/blob/master/ovn/ > >> utilities/ovndb-servers.ocf#L118 > >> Otherwise we will not be able to pass these options when starting the > >> resource. Ideally we want to do > >> "pcs resource create ovndb_servers .... listen_on_master_ip_only=no > >> listen_on_slave=no > >> > > >> Thanks for the pointer. I will update that and re-test. > > > >> > >> > >> + > >> > # Invalid IP address is an address that can never exist in the > >> network, as > >> > # mentioned in rfc-5737. The ovsdb servers connects to this IP address > >> > till > >> > # a master is promoted and the IPAddr2 resource is started. > >> > @@ -157,22 +167,24 @@ ovsdb_server_notify() { > >> > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd > >> > fi > >> > > >> > - conn=`ovn-nbctl get NB_global . connections` > >> > - if [ "$conn" == "[]" ] > >> > - then > >> > - ovn-nbctl -- --id=@conn_uuid create Connection \ > >> > + # TODO: Need to troubleshoot as to removing target is ok as > >> well. > >> > >> + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then > >> > > >> > >> If LB VIP is used, you don't want to set the probe interval ? > > > > >>> I thought we can stick with common probe interval for any case. Hence, > > kept it. > > > >> > >> > + 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 > >> > @@ -295,15 +307,15 @@ ovsdb_server_start() { > >> > > >> > set ${OVN_CTL} > >> > > >> > - set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} > >> > - set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} > >> > + # For LB vip to talk to master pool member on a specific tcp port, > >> we > >> > need > >> > + # to listen on 0.0.0.0.instead of master_ip > >> > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then > >> > + set $@ --db-nb-port=${NB_MASTER_PORT} > >> > + set $@ --db-sb-port=${SB_MASTER_PORT} > >> > > >> > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > >> > - set $@ --db-nb-create-insecure-remote=yes > >> > - fi > >> > - > >> > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > >> > - set $@ --db-sb-create-insecure-remote=yes > >> > + else > >> > + set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} > >> > + set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} > >> > fi > >> > > >> > if [ "x${present_master}" = x ]; then > >> > @@ -313,15 +325,44 @@ ovsdb_server_start() { > >> > # Force all copies to come up as slaves by pointing them into > >> > # space and let pacemaker pick one to promote: > >> > # > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > >> > + set $@ --db-nb-create-insecure-remote=yes > >> > + fi > >> > + > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > >> > + set $@ --db-sb-create-insecure-remote=yes > >> > + fi > >> > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} > >> > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} > >> > > >> > elif [ ${present_master} != ${host_name} ]; then > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then > >> > + # TODO: for using LB vip, need to test for ssl. > >> > + set $@ --db-nb-create-insecure-remote=no > >> > > >> > >> The default value of "db-nb-create-insecure-remote" is no. So there is no > >> need to set this. We can just have > >> if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then > >> if ... > >> ... > >> fi > > > > >>> Sure let me re-change and test. Will update the new patch accordingly. > > > >> + set $@ --db-sb-create-insecure-remote=no > >> > + else > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > >> > + set $@ --db-nb-create-insecure-remote=yes > >> > + fi > >> > + > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > >> > + set $@ --db-sb-create-insecure-remote=yes > >> > + fi > >> > + fi > >> > # An existing master is active, connect to it > >> > set $@ --db-nb-sync-from-addr=${MASTER_IP} > >> > --db-sb-sync-from-addr=${MASTER_IP} > >> > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} > >> > 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} > >> > + > >> > + else > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > >> > + set $@ --db-nb-create-insecure-remote=yes > >> > + fi > >> > + > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > >> > + set $@ --db-sb-create-insecure-remote=yes > >> > + fi > >> > fi > >> > > >> > $@ start_ovsdb > >> > @@ -416,6 +457,11 @@ ovsdb_server_promote() { > >> > ;; > >> > esac > >> > > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then > >> > >> > >> The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very > >> confusing. I think it should > >> be possible to have just one param - something like - > >> "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). > >> Thoughts ? > > > > >>> Sure, how about master_ip_lb_resource to avoid confusion? So you > > mean, just use this variable and completely get rid of LISTEN_ON_SLAVE > > and LISTEN_ON_MASTER_IP_ONLY? > > > > 'master_ip_lb_resource' seems fine to me. Thats' right. Just have one > parameter. > > > Will there be a case where the user can select > >> listen_on_master_ip_only=no and > >> listen_on_slave=yes ? > >> > > >>> No. > 'master_ip_lb_resource' seems a little confusing to me. What does it really mean? The desired behavior is, when the flag is "no", do the default: - On both master node and slave node, listen to MASTER_IP only When the flag is "yes", it works with an external LB: 1) On mater node, listen on 0.0.0.0 (because the MASTER_IP doesn't belong to the node) 2) On slave node, don't listen on TCP port (because we want the LB to be able to tell which one is the master according to whether the TCP port is open on the node) I am ok with just one parameter, but I'd suggest to make it more straighforward. I am also wondering, can we unify point 2), i.e. never listen on TCP port for slave node? Numan, do you have any use case for accessing slave node DBs using TCP? Thanks, Han
On Wed, May 30, 2018 at 2:52 PM, Han Zhou <zhouhan@gmail.com> wrote: > > > On Tue, May 29, 2018 at 11:40 PM, Numan Siddique <nusiddiq@redhat.com> > wrote: > > > > On Wed, May 30, 2018 at 5:31 AM, aginwala <aginwala@asu.edu> wrote: > > > > > Thanks for the Review. > > > > > > On Wed, May 23, 2018 at 11:36 AM, Numan Siddique <nusiddiq@redhat.com> > > > wrote: > > > > > >> On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> wrote: > > >> > > >> > using pacemaker so that controllers can be placed in different fault > > >> > domains. > > >> > > > >> > Signed-off-by: aginwala <aginwala@ebay.com> > > >> > > > >> > > >> I see the below warning when applying the patch > > >> > > >> .git/rebase-apply/patch:153: new blank line at EOF. > > >> + > > >> warning: 1 line adds whitespace errors. > > >> > > > >>> Sure. Will fix. > > > > > >> > > >> > --- > > >> > ovn/utilities/ovndb-servers.ocf | 83 > ++++++++++++++++++++++++++++++ > > >> > > >> > ++--------- > > >> > 1 file changed, 65 insertions(+), 18 deletions(-) > > >> > > > >> > diff --git a/ovn/utilities/ovndb-servers.ocf > > >> > b/ovn/utilities/ovndb-servers.ocf > > >> > index 164b6bc..1b4b6ab 100755 > > >> > --- a/ovn/utilities/ovndb-servers.ocf > > >> > +++ b/ovn/utilities/ovndb-servers.ocf > > >> > @@ -9,6 +9,8 @@ > > >> > : ${SB_MASTER_PROTO_DEFAULT="tcp"} > > >> > : ${MANAGE_NORTHD_DEFAULT="no"} > > >> > : ${INACTIVE_PROBE_DEFAULT="5000"} > > >> > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} > > >> > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} > > >> > > > >> > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" > > >> > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config > > >> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" > > >> > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_s > > >> b_master_protocol:-${SB_ > > >> > MASTER_PROTO_DEFAULT}} > > >> > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_ > NORTHD_DEFAULT}} > > >> > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ > > >> > INACTIVE_PROBE_DEFAULT}} > > >> > > > >> > +# In order for pacemaker to work with LB, we can keep > > >> > LISTEN_ON_MASTER_IP_ONLY > > >> > +# to false and pass LB vip IP while creating pcs resource. > > >> > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ > > >> > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} > > >> > + > > >> > +# In order for pacemaker to work with LB, we can also set > > >> LISTEN_ON_SLAVE > > >> > +# to false so that slaves do not listen on 0.0.0.0. > > >> > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_S > > >> LAVE_DEFAULT}} > > >> > > > >> > > >> > > >> You need to define these parameters in the <parameters> section here - > > >> https://github.com/openvswitch/ovs/blob/master/ovn/ > > >> utilities/ovndb-servers.ocf#L118 > > >> Otherwise we will not be able to pass these options when starting the > > >> resource. Ideally we want to do > > >> "pcs resource create ovndb_servers .... listen_on_master_ip_only=no > > >> listen_on_slave=no > > >> > > > >> Thanks for the pointer. I will update that and re-test. > > > > > >> > > >> > > >> + > > >> > # Invalid IP address is an address that can never exist in the > > >> network, as > > >> > # mentioned in rfc-5737. The ovsdb servers connects to this IP > address > > >> > till > > >> > # a master is promoted and the IPAddr2 resource is started. > > >> > @@ -157,22 +167,24 @@ ovsdb_server_notify() { > > >> > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd > > >> > fi > > >> > > > >> > - conn=`ovn-nbctl get NB_global . connections` > > >> > - if [ "$conn" == "[]" ] > > >> > - then > > >> > - ovn-nbctl -- --id=@conn_uuid create Connection \ > > >> > + # TODO: Need to troubleshoot as to removing target is ok as > > >> well. > > >> > > >> + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then > > >> > > > >> > > >> If LB VIP is used, you don't want to set the probe interval ? > > > > > > >>> I thought we can stick with common probe interval for any case. > Hence, > > > kept it. > > > > > >> > > >> > + 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 > > >> > @@ -295,15 +307,15 @@ ovsdb_server_start() { > > >> > > > >> > set ${OVN_CTL} > > >> > > > >> > - set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} > > >> > - set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} > > >> > + # For LB vip to talk to master pool member on a specific tcp > port, > > >> we > > >> > need > > >> > + # to listen on 0.0.0.0.instead of master_ip > > >> > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then > > >> > + set $@ --db-nb-port=${NB_MASTER_PORT} > > >> > + set $@ --db-sb-port=${SB_MASTER_PORT} > > >> > > > >> > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > > >> > - set $@ --db-nb-create-insecure-remote=yes > > >> > - fi > > >> > - > > >> > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > > >> > - set $@ --db-sb-create-insecure-remote=yes > > >> > + else > > >> > + set $@ --db-nb-addr=${MASTER_IP} > --db-nb-port=${NB_MASTER_PORT} > > >> > + set $@ --db-sb-addr=${MASTER_IP} > --db-sb-port=${SB_MASTER_PORT} > > >> > fi > > >> > > > >> > if [ "x${present_master}" = x ]; then > > >> > @@ -313,15 +325,44 @@ ovsdb_server_start() { > > >> > # Force all copies to come up as slaves by pointing them > into > > >> > # space and let pacemaker pick one to promote: > > >> > # > > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > > >> > + set $@ --db-nb-create-insecure-remote=yes > > >> > + fi > > >> > + > > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > > >> > + set $@ --db-sb-create-insecure-remote=yes > > >> > + fi > > >> > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} > > >> > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} > > >> > > > >> > elif [ ${present_master} != ${host_name} ]; then > > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then > > >> > + # TODO: for using LB vip, need to test for ssl. > > >> > + set $@ --db-nb-create-insecure-remote=no > > >> > > > >> > > >> The default value of "db-nb-create-insecure-remote" is no. So there > is no > > >> need to set this. We can just have > > >> if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then > > >> if ... > > >> ... > > >> fi > > > > > > >>> Sure let me re-change and test. Will update the new patch > accordingly. > > > > > >> + set $@ --db-sb-create-insecure-remote=no > > >> > + else > > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > > >> > + set $@ --db-nb-create-insecure-remote=yes > > >> > + fi > > >> > + > > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > > >> > + set $@ --db-sb-create-insecure-remote=yes > > >> > + fi > > >> > + fi > > >> > # An existing master is active, connect to it > > >> > set $@ --db-nb-sync-from-addr=${MASTER_IP} > > >> > --db-sb-sync-from-addr=${MASTER_IP} > > >> > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} > > >> > 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} > > >> > + > > >> > + else > > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then > > >> > + set $@ --db-nb-create-insecure-remote=yes > > >> > + fi > > >> > + > > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then > > >> > + set $@ --db-sb-create-insecure-remote=yes > > >> > + fi > > >> > fi > > >> > > > >> > $@ start_ovsdb > > >> > @@ -416,6 +457,11 @@ ovsdb_server_promote() { > > >> > ;; > > >> > esac > > >> > > > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then > > >> > > >> > > >> The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very > > >> confusing. I think it should > > >> be possible to have just one param - something like - > > >> "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). > > >> Thoughts ? > > > > > > >>> Sure, how about master_ip_lb_resource to avoid confusion? So you > > > mean, just use this variable and completely get rid of LISTEN_ON_SLAVE > > > and LISTEN_ON_MASTER_IP_ONLY? > > > > > > > 'master_ip_lb_resource' seems fine to me. Thats' right. Just have one > > parameter. > > > > > > Will there be a case where the user can select > > >> listen_on_master_ip_only=no and > > >> listen_on_slave=yes ? > > >> > > > >>> No. > > > 'master_ip_lb_resource' seems a little confusing to me. What does it > really mean? > The desired behavior is, when the flag is "no", do the default: > > - On both master node and slave node, listen to MASTER_IP only > > When the flag is "yes", it works with an external LB: > > 1) On mater node, listen on 0.0.0.0 (because the MASTER_IP doesn't belong > to the node) > 2) On slave node, don't listen on TCP port (because we want the LB to be > able to tell which one is the master according to whether the TCP port is > open on the node) > > I am ok with just one parameter, but I'd suggest to make it more > straighforward. I am also wondering, can we unify point 2), i.e. never > listen on TCP port for slave node? Numan, do you have any use case for > accessing slave node DBs using TCP? > > >>> Thanks Han for the review. We agreed on this name just to avoid confusion that IP is coming from LB and not pacemaker ipaddr2 resource. I can update the documentation to avoid confusion since documentation have no details of using LB VIP. Let me know if that needs to be in same or different patch. Also, as discussed for the promote logic, we are restarting ovs for LB use case and you want to unify it for pacemaker vip resource too to address point2. $@ start_ovsdb@@ -416,6 +457,11 @@ ovsdb_server_promote() { ;; esac + if [ "x${MASTER_IP_LB_RESOURCE}" = xyes ]; then+ # Restart ovs so that new master can listen on tcp port+ ${OVN_CTL} stop_ovsdb+ ovsdb_server_start However, I can add TODO there to see if its ok to unify too. Will wait for inputs on Numan too for the same. Thanks, > Han >
On Wed, May 30, 2018 at 3:27 PM, aginwala <aginwala@asu.edu> wrote: > > > On Wed, May 30, 2018 at 2:52 PM, Han Zhou <zhouhan@gmail.com> wrote: > >> >> >> On Tue, May 29, 2018 at 11:40 PM, Numan Siddique <nusiddiq@redhat.com> >> wrote: >> > >> > On Wed, May 30, 2018 at 5:31 AM, aginwala <aginwala@asu.edu> wrote: >> > >> > > Thanks for the Review. >> > > >> > > On Wed, May 23, 2018 at 11:36 AM, Numan Siddique <nusiddiq@redhat.com >> > >> > > wrote: >> > > >> > >> On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> >> wrote: >> > >> >> > >> > using pacemaker so that controllers can be placed in different >> fault >> > >> > domains. >> > >> > >> > >> > Signed-off-by: aginwala <aginwala@ebay.com> >> > >> > >> > >> >> > >> I see the below warning when applying the patch >> > >> >> > >> .git/rebase-apply/patch:153: new blank line at EOF. >> > >> + >> > >> warning: 1 line adds whitespace errors. >> > >> >> > > >>> Sure. Will fix. >> > > >> > >> >> > >> > --- >> > >> > ovn/utilities/ovndb-servers.ocf | 83 >> ++++++++++++++++++++++++++++++ >> > >> >> > >> > ++--------- >> > >> > 1 file changed, 65 insertions(+), 18 deletions(-) >> > >> > >> > >> > diff --git a/ovn/utilities/ovndb-servers.ocf >> > >> > b/ovn/utilities/ovndb-servers.ocf >> > >> > index 164b6bc..1b4b6ab 100755 >> > >> > --- a/ovn/utilities/ovndb-servers.ocf >> > >> > +++ b/ovn/utilities/ovndb-servers.ocf >> > >> > @@ -9,6 +9,8 @@ >> > >> > : ${SB_MASTER_PROTO_DEFAULT="tcp"} >> > >> > : ${MANAGE_NORTHD_DEFAULT="no"} >> > >> > : ${INACTIVE_PROBE_DEFAULT="5000"} >> > >> > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} >> > >> > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} >> > >> > >> > >> > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" >> > >> > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type >> crm_config >> > >> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" >> > >> > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_s >> > >> b_master_protocol:-${SB_ >> > >> > MASTER_PROTO_DEFAULT}} >> > >> > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_ >> DEFAULT}} >> > >> > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ >> > >> > INACTIVE_PROBE_DEFAULT}} >> > >> > >> > >> > +# In order for pacemaker to work with LB, we can keep >> > >> > LISTEN_ON_MASTER_IP_ONLY >> > >> > +# to false and pass LB vip IP while creating pcs resource. >> > >> > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ >> > >> > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} >> > >> > + >> > >> > +# In order for pacemaker to work with LB, we can also set >> > >> LISTEN_ON_SLAVE >> > >> > +# to false so that slaves do not listen on 0.0.0.0. >> > >> > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_S >> > >> LAVE_DEFAULT}} >> > >> > >> > >> >> > >> >> > >> You need to define these parameters in the <parameters> section here >> - >> > >> https://github.com/openvswitch/ovs/blob/master/ovn/ >> > >> utilities/ovndb-servers.ocf#L118 >> > >> Otherwise we will not be able to pass these options when starting the >> > >> resource. Ideally we want to do >> > >> "pcs resource create ovndb_servers .... listen_on_master_ip_only=no >> > >> listen_on_slave=no >> > >> >> > > >> Thanks for the pointer. I will update that and re-test. >> > > >> > >> >> > >> >> > >> + >> > >> > # Invalid IP address is an address that can never exist in the >> > >> network, as >> > >> > # mentioned in rfc-5737. The ovsdb servers connects to this IP >> address >> > >> > till >> > >> > # a master is promoted and the IPAddr2 resource is started. >> > >> > @@ -157,22 +167,24 @@ ovsdb_server_notify() { >> > >> > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd >> > >> > fi >> > >> > >> > >> > - conn=`ovn-nbctl get NB_global . connections` >> > >> > - if [ "$conn" == "[]" ] >> > >> > - then >> > >> > - ovn-nbctl -- --id=@conn_uuid create Connection \ >> > >> > + # TODO: Need to troubleshoot as to removing target is ok >> as >> > >> well. >> > >> >> > >> + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then >> > >> > >> > >> >> > >> If LB VIP is used, you don't want to set the probe interval ? >> > > >> > > >>> I thought we can stick with common probe interval for any case. >> Hence, >> > > kept it. >> > > >> > >> >> > >> > + 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 >> > >> > @@ -295,15 +307,15 @@ ovsdb_server_start() { >> > >> > >> > >> > set ${OVN_CTL} >> > >> > >> > >> > - set $@ --db-nb-addr=${MASTER_IP} >> --db-nb-port=${NB_MASTER_PORT} >> > >> > - set $@ --db-sb-addr=${MASTER_IP} >> --db-sb-port=${SB_MASTER_PORT} >> > >> > + # For LB vip to talk to master pool member on a specific tcp >> port, >> > >> we >> > >> > need >> > >> > + # to listen on 0.0.0.0.instead of master_ip >> > >> > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then >> > >> > + set $@ --db-nb-port=${NB_MASTER_PORT} >> > >> > + set $@ --db-sb-port=${SB_MASTER_PORT} >> > >> > >> > >> > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >> > >> > - set $@ --db-nb-create-insecure-remote=yes >> > >> > - fi >> > >> > - >> > >> > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >> > >> > - set $@ --db-sb-create-insecure-remote=yes >> > >> > + else >> > >> > + set $@ --db-nb-addr=${MASTER_IP} >> --db-nb-port=${NB_MASTER_PORT} >> > >> > + set $@ --db-sb-addr=${MASTER_IP} >> --db-sb-port=${SB_MASTER_PORT} >> > >> > fi >> > >> > >> > >> > if [ "x${present_master}" = x ]; then >> > >> > @@ -313,15 +325,44 @@ ovsdb_server_start() { >> > >> > # Force all copies to come up as slaves by pointing them >> into >> > >> > # space and let pacemaker pick one to promote: >> > >> > # >> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >> > >> > + set $@ --db-nb-create-insecure-remote=yes >> > >> > + fi >> > >> > + >> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >> > >> > + set $@ --db-sb-create-insecure-remote=yes >> > >> > + fi >> > >> > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} >> > >> > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} >> > >> > >> > >> > elif [ ${present_master} != ${host_name} ]; then >> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >> > >> > + # TODO: for using LB vip, need to test for ssl. >> > >> > + set $@ --db-nb-create-insecure-remote=no >> > >> > >> > >> >> > >> The default value of "db-nb-create-insecure-remote" is no. So >> there is no >> > >> need to set this. We can just have >> > >> if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then >> > >> if ... >> > >> ... >> > >> fi >> > > >> > > >>> Sure let me re-change and test. Will update the new patch >> accordingly. >> > > >> > >> + set $@ --db-sb-create-insecure-remote=no >> > >> > + else >> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >> > >> > + set $@ --db-nb-create-insecure-remote=yes >> > >> > + fi >> > >> > + >> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >> > >> > + set $@ --db-sb-create-insecure-remote=yes >> > >> > + fi >> > >> > + fi >> > >> > # An existing master is active, connect to it >> > >> > set $@ --db-nb-sync-from-addr=${MASTER_IP} >> > >> > --db-sb-sync-from-addr=${MASTER_IP} >> > >> > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} >> > >> > 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} >> > >> > + >> > >> > + else >> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >> > >> > + set $@ --db-nb-create-insecure-remote=yes >> > >> > + fi >> > >> > + >> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >> > >> > + set $@ --db-sb-create-insecure-remote=yes >> > >> > + fi >> > >> > fi >> > >> > >> > >> > $@ start_ovsdb >> > >> > @@ -416,6 +457,11 @@ ovsdb_server_promote() { >> > >> > ;; >> > >> > esac >> > >> > >> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >> > >> >> > >> >> > >> The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very >> > >> confusing. I think it should >> > >> be possible to have just one param - something like - >> > >> "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). >> > >> Thoughts ? >> > > >> > > >>> Sure, how about master_ip_lb_resource to avoid confusion? So you >> > > mean, just use this variable and completely get rid of LISTEN_ON_SLAVE >> > > and LISTEN_ON_MASTER_IP_ONLY? >> > > >> > >> > 'master_ip_lb_resource' seems fine to me. Thats' right. Just have one >> > parameter. >> > >> > >> > Will there be a case where the user can select >> > >> listen_on_master_ip_only=no and >> > >> listen_on_slave=yes ? >> > >> >> > > >>> No. >> > >> 'master_ip_lb_resource' seems a little confusing to me. What does it >> really mean? >> The desired behavior is, when the flag is "no", do the default: >> >> - On both master node and slave node, listen to MASTER_IP only >> >> When the flag is "yes", it works with an external LB: >> >> 1) On mater node, listen on 0.0.0.0 (because the MASTER_IP doesn't belong >> to the node) >> 2) On slave node, don't listen on TCP port (because we want the LB to be >> able to tell which one is the master according to whether the TCP port is >> open on the node) >> >> I am ok with just one parameter, but I'd suggest to make it more >> straighforward. I am also wondering, can we unify point 2), i.e. never >> listen on TCP port for slave node? Numan, do you have any use case for >> accessing slave node DBs using TCP? >> >> >>> Thanks Han for the review. We agreed on this name just to avoid > confusion that IP is coming from LB and not pacemaker ipaddr2 resource. I > can update the documentation to avoid confusion since documentation have no > details of using LB VIP. Let me know if that needs to be in same or > different patch. > I am fine if we can better document the purpose of the parameter and the expected behavior. I think it is always good to be in same patch for the change and its documentation. Thanks, Han
On Thu, May 31, 2018 at 4:06 AM, Han Zhou <zhouhan@gmail.com> wrote: > > > On Wed, May 30, 2018 at 3:27 PM, aginwala <aginwala@asu.edu> wrote: > >> >> >> On Wed, May 30, 2018 at 2:52 PM, Han Zhou <zhouhan@gmail.com> wrote: >> >>> >>> >>> On Tue, May 29, 2018 at 11:40 PM, Numan Siddique <nusiddiq@redhat.com> >>> wrote: >>> > >>> > On Wed, May 30, 2018 at 5:31 AM, aginwala <aginwala@asu.edu> wrote: >>> > >>> > > Thanks for the Review. >>> > > >>> > > On Wed, May 23, 2018 at 11:36 AM, Numan Siddique < >>> nusiddiq@redhat.com> >>> > > wrote: >>> > > >>> > >> On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> >>> wrote: >>> > >> >>> > >> > using pacemaker so that controllers can be placed in different >>> fault >>> > >> > domains. >>> > >> > >>> > >> > Signed-off-by: aginwala <aginwala@ebay.com> >>> > >> > >>> > >> >>> > >> I see the below warning when applying the patch >>> > >> >>> > >> .git/rebase-apply/patch:153: new blank line at EOF. >>> > >> + >>> > >> warning: 1 line adds whitespace errors. >>> > >> >>> > > >>> Sure. Will fix. >>> > > >>> > >> >>> > >> > --- >>> > >> > ovn/utilities/ovndb-servers.ocf | 83 >>> ++++++++++++++++++++++++++++++ >>> > >> >>> > >> > ++--------- >>> > >> > 1 file changed, 65 insertions(+), 18 deletions(-) >>> > >> > >>> > >> > diff --git a/ovn/utilities/ovndb-servers.ocf >>> > >> > b/ovn/utilities/ovndb-servers.ocf >>> > >> > index 164b6bc..1b4b6ab 100755 >>> > >> > --- a/ovn/utilities/ovndb-servers.ocf >>> > >> > +++ b/ovn/utilities/ovndb-servers.ocf >>> > >> > @@ -9,6 +9,8 @@ >>> > >> > : ${SB_MASTER_PROTO_DEFAULT="tcp"} >>> > >> > : ${MANAGE_NORTHD_DEFAULT="no"} >>> > >> > : ${INACTIVE_PROBE_DEFAULT="5000"} >>> > >> > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} >>> > >> > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} >>> > >> > >>> > >> > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" >>> > >> > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type >>> crm_config >>> > >> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" >>> > >> > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_s >>> > >> b_master_protocol:-${SB_ >>> > >> > MASTER_PROTO_DEFAULT}} >>> > >> > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_D >>> EFAULT}} >>> > >> > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ >>> > >> > INACTIVE_PROBE_DEFAULT}} >>> > >> > >>> > >> > +# In order for pacemaker to work with LB, we can keep >>> > >> > LISTEN_ON_MASTER_IP_ONLY >>> > >> > +# to false and pass LB vip IP while creating pcs resource. >>> > >> > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ >>> > >> > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} >>> > >> > + >>> > >> > +# In order for pacemaker to work with LB, we can also set >>> > >> LISTEN_ON_SLAVE >>> > >> > +# to false so that slaves do not listen on 0.0.0.0. >>> > >> > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_S >>> > >> LAVE_DEFAULT}} >>> > >> > >>> > >> >>> > >> >>> > >> You need to define these parameters in the <parameters> section >>> here - >>> > >> https://github.com/openvswitch/ovs/blob/master/ovn/ >>> > >> utilities/ovndb-servers.ocf#L118 >>> > >> Otherwise we will not be able to pass these options when starting >>> the >>> > >> resource. Ideally we want to do >>> > >> "pcs resource create ovndb_servers .... listen_on_master_ip_only=no >>> > >> listen_on_slave=no >>> > >> >>> > > >> Thanks for the pointer. I will update that and re-test. >>> > > >>> > >> >>> > >> >>> > >> + >>> > >> > # Invalid IP address is an address that can never exist in the >>> > >> network, as >>> > >> > # mentioned in rfc-5737. The ovsdb servers connects to this IP >>> address >>> > >> > till >>> > >> > # a master is promoted and the IPAddr2 resource is started. >>> > >> > @@ -157,22 +167,24 @@ ovsdb_server_notify() { >>> > >> > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd >>> > >> > fi >>> > >> > >>> > >> > - conn=`ovn-nbctl get NB_global . connections` >>> > >> > - if [ "$conn" == "[]" ] >>> > >> > - then >>> > >> > - ovn-nbctl -- --id=@conn_uuid create Connection \ >>> > >> > + # TODO: Need to troubleshoot as to removing target is ok >>> as >>> > >> well. >>> > >> >>> > >> + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then >>> > >> > >>> > >> >>> > >> If LB VIP is used, you don't want to set the probe interval ? >>> > > >>> > > >>> I thought we can stick with common probe interval for any case. >>> Hence, >>> > > kept it. >>> > > >>> > >> >>> > >> > + 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 >>> > >> > @@ -295,15 +307,15 @@ ovsdb_server_start() { >>> > >> > >>> > >> > set ${OVN_CTL} >>> > >> > >>> > >> > - set $@ --db-nb-addr=${MASTER_IP} >>> --db-nb-port=${NB_MASTER_PORT} >>> > >> > - set $@ --db-sb-addr=${MASTER_IP} >>> --db-sb-port=${SB_MASTER_PORT} >>> > >> > + # For LB vip to talk to master pool member on a specific tcp >>> port, >>> > >> we >>> > >> > need >>> > >> > + # to listen on 0.0.0.0.instead of master_ip >>> > >> > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then >>> > >> > + set $@ --db-nb-port=${NB_MASTER_PORT} >>> > >> > + set $@ --db-sb-port=${SB_MASTER_PORT} >>> > >> > >>> > >> > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>> > >> > - set $@ --db-nb-create-insecure-remote=yes >>> > >> > - fi >>> > >> > - >>> > >> > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>> > >> > - set $@ --db-sb-create-insecure-remote=yes >>> > >> > + else >>> > >> > + set $@ --db-nb-addr=${MASTER_IP} >>> --db-nb-port=${NB_MASTER_PORT} >>> > >> > + set $@ --db-sb-addr=${MASTER_IP} >>> --db-sb-port=${SB_MASTER_PORT} >>> > >> > fi >>> > >> > >>> > >> > if [ "x${present_master}" = x ]; then >>> > >> > @@ -313,15 +325,44 @@ ovsdb_server_start() { >>> > >> > # Force all copies to come up as slaves by pointing them >>> into >>> > >> > # space and let pacemaker pick one to promote: >>> > >> > # >>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>> > >> > + fi >>> > >> > + >>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>> > >> > + fi >>> > >> > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} >>> > >> > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} >>> > >> > >>> > >> > elif [ ${present_master} != ${host_name} ]; then >>> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >>> > >> > + # TODO: for using LB vip, need to test for ssl. >>> > >> > + set $@ --db-nb-create-insecure-remote=no >>> > >> > >>> > >> >>> > >> The default value of "db-nb-create-insecure-remote" is no. So >>> there is no >>> > >> need to set this. We can just have >>> > >> if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then >>> > >> if ... >>> > >> ... >>> > >> fi >>> > > >>> > > >>> Sure let me re-change and test. Will update the new patch >>> accordingly. >>> > > >>> > >> + set $@ --db-sb-create-insecure-remote=no >>> > >> > + else >>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>> > >> > + fi >>> > >> > + >>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>> > >> > + fi >>> > >> > + fi >>> > >> > # An existing master is active, connect to it >>> > >> > set $@ --db-nb-sync-from-addr=${MASTER_IP} >>> > >> > --db-sb-sync-from-addr=${MASTER_IP} >>> > >> > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} >>> > >> > 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} >>> > >> > + >>> > >> > + else >>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>> > >> > + fi >>> > >> > + >>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>> > >> > + fi >>> > >> > fi >>> > >> > >>> > >> > $@ start_ovsdb >>> > >> > @@ -416,6 +457,11 @@ ovsdb_server_promote() { >>> > >> > ;; >>> > >> > esac >>> > >> > >>> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >>> > >> >>> > >> >>> > >> The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very >>> > >> confusing. I think it should >>> > >> be possible to have just one param - something like - >>> > >> "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). >>> > >> Thoughts ? >>> > > >>> > > >>> Sure, how about master_ip_lb_resource to avoid confusion? So you >>> > > mean, just use this variable and completely get rid of >>> LISTEN_ON_SLAVE >>> > > and LISTEN_ON_MASTER_IP_ONLY? >>> > > >>> > >>> > 'master_ip_lb_resource' seems fine to me. Thats' right. Just have one >>> > parameter. >>> > >>> > >>> > Will there be a case where the user can select >>> > >> listen_on_master_ip_only=no and >>> > >> listen_on_slave=yes ? >>> > >> >>> > > >>> No. >>> > >>> 'master_ip_lb_resource' seems a little confusing to me. What does it >>> really mean? >>> The desired behavior is, when the flag is "no", do the default: >>> >>> - On both master node and slave node, listen to MASTER_IP only >>> >>> When the flag is "yes", it works with an external LB: >>> >>> 1) On mater node, listen on 0.0.0.0 (because the MASTER_IP doesn't >>> belong to the node) >>> 2) On slave node, don't listen on TCP port (because we want the LB to be >>> able to tell which one is the master according to whether the TCP port is >>> open on the node) >>> >>> I am ok with just one parameter, but I'd suggest to make it more >>> straighforward. I am also wondering, can we unify point 2), i.e. never >>> listen on TCP port for slave node? Numan, do you have any use case for >>> accessing slave node DBs using TCP? >>> >> I don't think there is any use case. I actually thought about this and I think it makes sense to unify. The only concern I had was whenever a slave is promoted, it needs a restart of ovsdb-servers. But I think it's not a big deal. It would require a little more testing from my side to make sure that it doesn't break the existing behaviour. @Aliasgar - Are you OK spinning another patch unifying this ? What do you think ? Also we can rename the parameter back to "listen_on_master_ip_only" if we unify this. > >>> >>> Thanks Han for the review. We agreed on this name just to avoid >> confusion that IP is coming from LB and not pacemaker ipaddr2 resource. I >> can update the documentation to avoid confusion since documentation have no >> details of using LB VIP. Let me know if that needs to be in same or >> different patch. >> > > I am fine if we can better document the purpose of the parameter and the > expected behavior. I think it is always good to be in same patch for the > change and its documentation. > +1 Thanks Numan > > Thanks, > Han > >
On Wed, May 30, 2018 at 11:24 PM, Numan Siddique <nusiddiq@redhat.com> wrote: > > > On Thu, May 31, 2018 at 4:06 AM, Han Zhou <zhouhan@gmail.com> wrote: > >> >> >> On Wed, May 30, 2018 at 3:27 PM, aginwala <aginwala@asu.edu> wrote: >> >>> >>> >>> On Wed, May 30, 2018 at 2:52 PM, Han Zhou <zhouhan@gmail.com> wrote: >>> >>>> >>>> >>>> On Tue, May 29, 2018 at 11:40 PM, Numan Siddique <nusiddiq@redhat.com> >>>> wrote: >>>> > >>>> > On Wed, May 30, 2018 at 5:31 AM, aginwala <aginwala@asu.edu> wrote: >>>> > >>>> > > Thanks for the Review. >>>> > > >>>> > > On Wed, May 23, 2018 at 11:36 AM, Numan Siddique < >>>> nusiddiq@redhat.com> >>>> > > wrote: >>>> > > >>>> > >> On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> >>>> wrote: >>>> > >> >>>> > >> > using pacemaker so that controllers can be placed in different >>>> fault >>>> > >> > domains. >>>> > >> > >>>> > >> > Signed-off-by: aginwala <aginwala@ebay.com> >>>> > >> > >>>> > >> >>>> > >> I see the below warning when applying the patch >>>> > >> >>>> > >> .git/rebase-apply/patch:153: new blank line at EOF. >>>> > >> + >>>> > >> warning: 1 line adds whitespace errors. >>>> > >> >>>> > > >>> Sure. Will fix. >>>> > > >>>> > >> >>>> > >> > --- >>>> > >> > ovn/utilities/ovndb-servers.ocf | 83 >>>> ++++++++++++++++++++++++++++++ >>>> > >> >>>> > >> > ++--------- >>>> > >> > 1 file changed, 65 insertions(+), 18 deletions(-) >>>> > >> > >>>> > >> > diff --git a/ovn/utilities/ovndb-servers.ocf >>>> > >> > b/ovn/utilities/ovndb-servers.ocf >>>> > >> > index 164b6bc..1b4b6ab 100755 >>>> > >> > --- a/ovn/utilities/ovndb-servers.ocf >>>> > >> > +++ b/ovn/utilities/ovndb-servers.ocf >>>> > >> > @@ -9,6 +9,8 @@ >>>> > >> > : ${SB_MASTER_PROTO_DEFAULT="tcp"} >>>> > >> > : ${MANAGE_NORTHD_DEFAULT="no"} >>>> > >> > : ${INACTIVE_PROBE_DEFAULT="5000"} >>>> > >> > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} >>>> > >> > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} >>>> > >> > >>>> > >> > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" >>>> > >> > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type >>>> crm_config >>>> > >> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" >>>> > >> > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_s >>>> > >> b_master_protocol:-${SB_ >>>> > >> > MASTER_PROTO_DEFAULT}} >>>> > >> > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_D >>>> EFAULT}} >>>> > >> > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ >>>> > >> > INACTIVE_PROBE_DEFAULT}} >>>> > >> > >>>> > >> > +# In order for pacemaker to work with LB, we can keep >>>> > >> > LISTEN_ON_MASTER_IP_ONLY >>>> > >> > +# to false and pass LB vip IP while creating pcs resource. >>>> > >> > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ >>>> > >> > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} >>>> > >> > + >>>> > >> > +# In order for pacemaker to work with LB, we can also set >>>> > >> LISTEN_ON_SLAVE >>>> > >> > +# to false so that slaves do not listen on 0.0.0.0. >>>> > >> > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_S >>>> > >> LAVE_DEFAULT}} >>>> > >> > >>>> > >> >>>> > >> >>>> > >> You need to define these parameters in the <parameters> section >>>> here - >>>> > >> https://github.com/openvswitch/ovs/blob/master/ovn/ >>>> > >> utilities/ovndb-servers.ocf#L118 >>>> > >> Otherwise we will not be able to pass these options when starting >>>> the >>>> > >> resource. Ideally we want to do >>>> > >> "pcs resource create ovndb_servers .... listen_on_master_ip_only=no >>>> > >> listen_on_slave=no >>>> > >> >>>> > > >> Thanks for the pointer. I will update that and re-test. >>>> > > >>>> > >> >>>> > >> >>>> > >> + >>>> > >> > # Invalid IP address is an address that can never exist in the >>>> > >> network, as >>>> > >> > # mentioned in rfc-5737. The ovsdb servers connects to this IP >>>> address >>>> > >> > till >>>> > >> > # a master is promoted and the IPAddr2 resource is started. >>>> > >> > @@ -157,22 +167,24 @@ ovsdb_server_notify() { >>>> > >> > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd >>>> > >> > fi >>>> > >> > >>>> > >> > - conn=`ovn-nbctl get NB_global . connections` >>>> > >> > - if [ "$conn" == "[]" ] >>>> > >> > - then >>>> > >> > - ovn-nbctl -- --id=@conn_uuid create Connection \ >>>> > >> > + # TODO: Need to troubleshoot as to removing target is >>>> ok as >>>> > >> well. >>>> > >> >>>> > >> + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then >>>> > >> > >>>> > >> >>>> > >> If LB VIP is used, you don't want to set the probe interval ? >>>> > > >>>> > > >>> I thought we can stick with common probe interval for any case. >>>> Hence, >>>> > > kept it. >>>> > > >>>> > >> >>>> > >> > + 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 >>>> > >> > @@ -295,15 +307,15 @@ ovsdb_server_start() { >>>> > >> > >>>> > >> > set ${OVN_CTL} >>>> > >> > >>>> > >> > - set $@ --db-nb-addr=${MASTER_IP} >>>> --db-nb-port=${NB_MASTER_PORT} >>>> > >> > - set $@ --db-sb-addr=${MASTER_IP} >>>> --db-sb-port=${SB_MASTER_PORT} >>>> > >> > + # For LB vip to talk to master pool member on a specific >>>> tcp port, >>>> > >> we >>>> > >> > need >>>> > >> > + # to listen on 0.0.0.0.instead of master_ip >>>> > >> > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then >>>> > >> > + set $@ --db-nb-port=${NB_MASTER_PORT} >>>> > >> > + set $@ --db-sb-port=${SB_MASTER_PORT} >>>> > >> > >>>> > >> > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>>> > >> > - set $@ --db-nb-create-insecure-remote=yes >>>> > >> > - fi >>>> > >> > - >>>> > >> > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>>> > >> > - set $@ --db-sb-create-insecure-remote=yes >>>> > >> > + else >>>> > >> > + set $@ --db-nb-addr=${MASTER_IP} >>>> --db-nb-port=${NB_MASTER_PORT} >>>> > >> > + set $@ --db-sb-addr=${MASTER_IP} >>>> --db-sb-port=${SB_MASTER_PORT} >>>> > >> > fi >>>> > >> > >>>> > >> > if [ "x${present_master}" = x ]; then >>>> > >> > @@ -313,15 +325,44 @@ ovsdb_server_start() { >>>> > >> > # Force all copies to come up as slaves by pointing >>>> them into >>>> > >> > # space and let pacemaker pick one to promote: >>>> > >> > # >>>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>>> > >> > + fi >>>> > >> > + >>>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>>> > >> > + fi >>>> > >> > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} >>>> > >> > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} >>>> > >> > >>>> > >> > elif [ ${present_master} != ${host_name} ]; then >>>> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >>>> > >> > + # TODO: for using LB vip, need to test for ssl. >>>> > >> > + set $@ --db-nb-create-insecure-remote=no >>>> > >> > >>>> > >> >>>> > >> The default value of "db-nb-create-insecure-remote" is no. So >>>> there is no >>>> > >> need to set this. We can just have >>>> > >> if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then >>>> > >> if ... >>>> > >> ... >>>> > >> fi >>>> > > >>>> > > >>> Sure let me re-change and test. Will update the new patch >>>> accordingly. >>>> > > >>>> > >> + set $@ --db-sb-create-insecure-remote=no >>>> > >> > + else >>>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>>> > >> > + fi >>>> > >> > + >>>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>>> > >> > + fi >>>> > >> > + fi >>>> > >> > # An existing master is active, connect to it >>>> > >> > set $@ --db-nb-sync-from-addr=${MASTER_IP} >>>> > >> > --db-sb-sync-from-addr=${MASTER_IP} >>>> > >> > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} >>>> > >> > 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} >>>> > >> > + >>>> > >> > + else >>>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>>> > >> > + fi >>>> > >> > + >>>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>>> > >> > + fi >>>> > >> > fi >>>> > >> > >>>> > >> > $@ start_ovsdb >>>> > >> > @@ -416,6 +457,11 @@ ovsdb_server_promote() { >>>> > >> > ;; >>>> > >> > esac >>>> > >> > >>>> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >>>> > >> >>>> > >> >>>> > >> The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is very >>>> > >> confusing. I think it should >>>> > >> be possible to have just one param - something like - >>>> > >> "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). >>>> > >> Thoughts ? >>>> > > >>>> > > >>> Sure, how about master_ip_lb_resource to avoid confusion? So you >>>> > > mean, just use this variable and completely get rid of >>>> LISTEN_ON_SLAVE >>>> > > and LISTEN_ON_MASTER_IP_ONLY? >>>> > > >>>> > >>>> > 'master_ip_lb_resource' seems fine to me. Thats' right. Just have one >>>> > parameter. >>>> > >>>> > >>>> > Will there be a case where the user can select >>>> > >> listen_on_master_ip_only=no and >>>> > >> listen_on_slave=yes ? >>>> > >> >>>> > > >>> No. >>>> > >>>> 'master_ip_lb_resource' seems a little confusing to me. What does it >>>> really mean? >>>> The desired behavior is, when the flag is "no", do the default: >>>> >>>> - On both master node and slave node, listen to MASTER_IP only >>>> >>>> When the flag is "yes", it works with an external LB: >>>> >>>> 1) On mater node, listen on 0.0.0.0 (because the MASTER_IP doesn't >>>> belong to the node) >>>> 2) On slave node, don't listen on TCP port (because we want the LB to >>>> be able to tell which one is the master according to whether the TCP port >>>> is open on the node) >>>> >>>> I am ok with just one parameter, but I'd suggest to make it more >>>> straighforward. I am also wondering, can we unify point 2), i.e. never >>>> listen on TCP port for slave node? Numan, do you have any use case for >>>> accessing slave node DBs using TCP? >>>> >>> > > I don't think there is any use case. I actually thought about this and I > think it makes sense to unify. The only concern I had was whenever a slave > is promoted, it needs a restart of ovsdb-servers. But I think it's not a > big deal. It would require a little more testing from my side to make sure > that it doesn't break the existing behaviour. > @Aliasgar - Are you OK spinning another patch unifying this ? What do you > think ? Also we can rename the parameter back to "listen_on_master_ip_only" > if we unify this. > > >>> Sure I thought of keeping it for backward compatibility as I was not sure of the implications of unifying it. If little more testing is ok, I am fine with that . Will also rename to listen_on_master_ip_only and send out v4. > > > >> >>>> >>> Thanks Han for the review. We agreed on this name just to avoid >>> confusion that IP is coming from LB and not pacemaker ipaddr2 resource. I >>> can update the documentation to avoid confusion since documentation have no >>> details of using LB VIP. Let me know if that needs to be in same or >>> different patch. >>> >> >> I am fine if we can better document the purpose of the parameter and the >> expected behavior. I think it is always good to be in same patch for the >> change and its documentation. >> > +1 > > Thanks > Numan > >>> Noted. Will add docs in v4 too. > > > >> >> Thanks, >> Han >> >> >
Hi: Have sent out V4 @ https://patchwork.ozlabs.org/patch/924734/ . Please take a look and let me know further. Regards, Aliasgar On Thu, May 31, 2018 at 9:12 AM, aginwala <aginwala@asu.edu> wrote: > > > On Wed, May 30, 2018 at 11:24 PM, Numan Siddique <nusiddiq@redhat.com> > wrote: > >> >> >> On Thu, May 31, 2018 at 4:06 AM, Han Zhou <zhouhan@gmail.com> wrote: >> >>> >>> >>> On Wed, May 30, 2018 at 3:27 PM, aginwala <aginwala@asu.edu> wrote: >>> >>>> >>>> >>>> On Wed, May 30, 2018 at 2:52 PM, Han Zhou <zhouhan@gmail.com> wrote: >>>> >>>>> >>>>> >>>>> On Tue, May 29, 2018 at 11:40 PM, Numan Siddique <nusiddiq@redhat.com> >>>>> wrote: >>>>> > >>>>> > On Wed, May 30, 2018 at 5:31 AM, aginwala <aginwala@asu.edu> wrote: >>>>> > >>>>> > > Thanks for the Review. >>>>> > > >>>>> > > On Wed, May 23, 2018 at 11:36 AM, Numan Siddique < >>>>> nusiddiq@redhat.com> >>>>> > > wrote: >>>>> > > >>>>> > >> On Wed, May 9, 2018 at 12:13 AM, aginwala <amginwal@gmail.com> >>>>> wrote: >>>>> > >> >>>>> > >> > using pacemaker so that controllers can be placed in different >>>>> fault >>>>> > >> > domains. >>>>> > >> > >>>>> > >> > Signed-off-by: aginwala <aginwala@ebay.com> >>>>> > >> > >>>>> > >> >>>>> > >> I see the below warning when applying the patch >>>>> > >> >>>>> > >> .git/rebase-apply/patch:153: new blank line at EOF. >>>>> > >> + >>>>> > >> warning: 1 line adds whitespace errors. >>>>> > >> >>>>> > > >>> Sure. Will fix. >>>>> > > >>>>> > >> >>>>> > >> > --- >>>>> > >> > ovn/utilities/ovndb-servers.ocf | 83 >>>>> ++++++++++++++++++++++++++++++ >>>>> > >> >>>>> > >> > ++--------- >>>>> > >> > 1 file changed, 65 insertions(+), 18 deletions(-) >>>>> > >> > >>>>> > >> > diff --git a/ovn/utilities/ovndb-servers.ocf >>>>> > >> > b/ovn/utilities/ovndb-servers.ocf >>>>> > >> > index 164b6bc..1b4b6ab 100755 >>>>> > >> > --- a/ovn/utilities/ovndb-servers.ocf >>>>> > >> > +++ b/ovn/utilities/ovndb-servers.ocf >>>>> > >> > @@ -9,6 +9,8 @@ >>>>> > >> > : ${SB_MASTER_PROTO_DEFAULT="tcp"} >>>>> > >> > : ${MANAGE_NORTHD_DEFAULT="no"} >>>>> > >> > : ${INACTIVE_PROBE_DEFAULT="5000"} >>>>> > >> > +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} >>>>> > >> > +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} >>>>> > >> > >>>>> > >> > CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" >>>>> > >> > CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type >>>>> crm_config >>>>> > >> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server" >>>>> > >> > @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_s >>>>> > >> b_master_protocol:-${SB_ >>>>> > >> > MASTER_PROTO_DEFAULT}} >>>>> > >> > MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_D >>>>> EFAULT}} >>>>> > >> > INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${ >>>>> > >> > INACTIVE_PROBE_DEFAULT}} >>>>> > >> > >>>>> > >> > +# In order for pacemaker to work with LB, we can keep >>>>> > >> > LISTEN_ON_MASTER_IP_ONLY >>>>> > >> > +# to false and pass LB vip IP while creating pcs resource. >>>>> > >> > +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ >>>>> > >> > ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} >>>>> > >> > + >>>>> > >> > +# In order for pacemaker to work with LB, we can also set >>>>> > >> LISTEN_ON_SLAVE >>>>> > >> > +# to false so that slaves do not listen on 0.0.0.0. >>>>> > >> > +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_S >>>>> > >> LAVE_DEFAULT}} >>>>> > >> > >>>>> > >> >>>>> > >> >>>>> > >> You need to define these parameters in the <parameters> section >>>>> here - >>>>> > >> https://github.com/openvswitch/ovs/blob/master/ovn/ >>>>> > >> utilities/ovndb-servers.ocf#L118 >>>>> > >> Otherwise we will not be able to pass these options when starting >>>>> the >>>>> > >> resource. Ideally we want to do >>>>> > >> "pcs resource create ovndb_servers .... >>>>> listen_on_master_ip_only=no >>>>> > >> listen_on_slave=no >>>>> > >> >>>>> > > >> Thanks for the pointer. I will update that and re-test. >>>>> > > >>>>> > >> >>>>> > >> >>>>> > >> + >>>>> > >> > # Invalid IP address is an address that can never exist in the >>>>> > >> network, as >>>>> > >> > # mentioned in rfc-5737. The ovsdb servers connects to this IP >>>>> address >>>>> > >> > till >>>>> > >> > # a master is promoted and the IPAddr2 resource is started. >>>>> > >> > @@ -157,22 +167,24 @@ ovsdb_server_notify() { >>>>> > >> > ${OVN_CTL} --ovn-manage-ovsdb=no start_northd >>>>> > >> > fi >>>>> > >> > >>>>> > >> > - conn=`ovn-nbctl get NB_global . connections` >>>>> > >> > - if [ "$conn" == "[]" ] >>>>> > >> > - then >>>>> > >> > - ovn-nbctl -- --id=@conn_uuid create Connection \ >>>>> > >> > + # TODO: Need to troubleshoot as to removing target is >>>>> ok as >>>>> > >> well. >>>>> > >> >>>>> > >> + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then >>>>> > >> > >>>>> > >> >>>>> > >> If LB VIP is used, you don't want to set the probe interval ? >>>>> > > >>>>> > > >>> I thought we can stick with common probe interval for any >>>>> case. Hence, >>>>> > > kept it. >>>>> > > >>>>> > >> >>>>> > >> > + 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 >>>>> > >> > @@ -295,15 +307,15 @@ ovsdb_server_start() { >>>>> > >> > >>>>> > >> > set ${OVN_CTL} >>>>> > >> > >>>>> > >> > - set $@ --db-nb-addr=${MASTER_IP} >>>>> --db-nb-port=${NB_MASTER_PORT} >>>>> > >> > - set $@ --db-sb-addr=${MASTER_IP} >>>>> --db-sb-port=${SB_MASTER_PORT} >>>>> > >> > + # For LB vip to talk to master pool member on a specific >>>>> tcp port, >>>>> > >> we >>>>> > >> > need >>>>> > >> > + # to listen on 0.0.0.0.instead of master_ip >>>>> > >> > + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then >>>>> > >> > + set $@ --db-nb-port=${NB_MASTER_PORT} >>>>> > >> > + set $@ --db-sb-port=${SB_MASTER_PORT} >>>>> > >> > >>>>> > >> > - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>>>> > >> > - set $@ --db-nb-create-insecure-remote=yes >>>>> > >> > - fi >>>>> > >> > - >>>>> > >> > - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>>>> > >> > - set $@ --db-sb-create-insecure-remote=yes >>>>> > >> > + else >>>>> > >> > + set $@ --db-nb-addr=${MASTER_IP} >>>>> --db-nb-port=${NB_MASTER_PORT} >>>>> > >> > + set $@ --db-sb-addr=${MASTER_IP} >>>>> --db-sb-port=${SB_MASTER_PORT} >>>>> > >> > fi >>>>> > >> > >>>>> > >> > if [ "x${present_master}" = x ]; then >>>>> > >> > @@ -313,15 +325,44 @@ ovsdb_server_start() { >>>>> > >> > # Force all copies to come up as slaves by pointing >>>>> them into >>>>> > >> > # space and let pacemaker pick one to promote: >>>>> > >> > # >>>>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>>>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>>>> > >> > + fi >>>>> > >> > + >>>>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>>>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>>>> > >> > + fi >>>>> > >> > set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} >>>>> > >> > --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} >>>>> > >> > >>>>> > >> > elif [ ${present_master} != ${host_name} ]; then >>>>> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >>>>> > >> > + # TODO: for using LB vip, need to test for ssl. >>>>> > >> > + set $@ --db-nb-create-insecure-remote=no >>>>> > >> > >>>>> > >> >>>>> > >> The default value of "db-nb-create-insecure-remote" is no. So >>>>> there is no >>>>> > >> need to set this. We can just have >>>>> > >> if [ "x${LISTEN_ON_SLAVE} = xyes ] ; then >>>>> > >> if ... >>>>> > >> ... >>>>> > >> fi >>>>> > > >>>>> > > >>> Sure let me re-change and test. Will update the new patch >>>>> accordingly. >>>>> > > >>>>> > >> + set $@ --db-sb-create-insecure-remote=no >>>>> > >> > + else >>>>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>>>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>>>> > >> > + fi >>>>> > >> > + >>>>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>>>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>>>> > >> > + fi >>>>> > >> > + fi >>>>> > >> > # An existing master is active, connect to it >>>>> > >> > set $@ --db-nb-sync-from-addr=${MASTER_IP} >>>>> > >> > --db-sb-sync-from-addr=${MASTER_IP} >>>>> > >> > set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} >>>>> > >> > 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} >>>>> > >> > + >>>>> > >> > + else >>>>> > >> > + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then >>>>> > >> > + set $@ --db-nb-create-insecure-remote=yes >>>>> > >> > + fi >>>>> > >> > + >>>>> > >> > + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then >>>>> > >> > + set $@ --db-sb-create-insecure-remote=yes >>>>> > >> > + fi >>>>> > >> > fi >>>>> > >> > >>>>> > >> > $@ start_ovsdb >>>>> > >> > @@ -416,6 +457,11 @@ ovsdb_server_promote() { >>>>> > >> > ;; >>>>> > >> > esac >>>>> > >> > >>>>> > >> > + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then >>>>> > >> >>>>> > >> >>>>> > >> The use of LISTEN_ON_SLAVE param in "ovsdb_server_promote" is >>>>> very >>>>> > >> confusing. I think it should >>>>> > >> be possible to have just one param - something like - >>>>> > >> "master_ip_pacemaker_resource=(yes/no)" (or some thing better ). >>>>> > >> Thoughts ? >>>>> > > >>>>> > > >>> Sure, how about master_ip_lb_resource to avoid confusion? So >>>>> you >>>>> > > mean, just use this variable and completely get rid of >>>>> LISTEN_ON_SLAVE >>>>> > > and LISTEN_ON_MASTER_IP_ONLY? >>>>> > > >>>>> > >>>>> > 'master_ip_lb_resource' seems fine to me. Thats' right. Just have one >>>>> > parameter. >>>>> > >>>>> > >>>>> > Will there be a case where the user can select >>>>> > >> listen_on_master_ip_only=no and >>>>> > >> listen_on_slave=yes ? >>>>> > >> >>>>> > > >>> No. >>>>> > >>>>> 'master_ip_lb_resource' seems a little confusing to me. What does it >>>>> really mean? >>>>> The desired behavior is, when the flag is "no", do the default: >>>>> >>>>> - On both master node and slave node, listen to MASTER_IP only >>>>> >>>>> When the flag is "yes", it works with an external LB: >>>>> >>>>> 1) On mater node, listen on 0.0.0.0 (because the MASTER_IP doesn't >>>>> belong to the node) >>>>> 2) On slave node, don't listen on TCP port (because we want the LB to >>>>> be able to tell which one is the master according to whether the TCP port >>>>> is open on the node) >>>>> >>>>> I am ok with just one parameter, but I'd suggest to make it more >>>>> straighforward. I am also wondering, can we unify point 2), i.e. never >>>>> listen on TCP port for slave node? Numan, do you have any use case for >>>>> accessing slave node DBs using TCP? >>>>> >>>> >> >> I don't think there is any use case. I actually thought about this and I >> think it makes sense to unify. The only concern I had was whenever a slave >> is promoted, it needs a restart of ovsdb-servers. But I think it's not a >> big deal. It would require a little more testing from my side to make sure >> that it doesn't break the existing behaviour. >> @Aliasgar - Are you OK spinning another patch unifying this ? What do >> you think ? Also we can rename the parameter back to "listen_on_master_ip_only" >> if we unify this. >> >> >>> Sure I thought of keeping it for backward compatibility as I was not > sure of the implications of unifying it. If little more testing is ok, I am > fine with that . Will also rename to listen_on_master_ip_only and send > out v4. > >> >> >> >>> >>>>> >>> Thanks Han for the review. We agreed on this name just to avoid >>>> confusion that IP is coming from LB and not pacemaker ipaddr2 resource. I >>>> can update the documentation to avoid confusion since documentation have no >>>> details of using LB VIP. Let me know if that needs to be in same or >>>> different patch. >>>> >>> >>> I am fine if we can better document the purpose of the parameter and the >>> expected behavior. I think it is always good to be in same patch for the >>> change and its documentation. >>> >> +1 >> >> Thanks >> Numan >> > >>> Noted. Will add docs in v4 too. > >> >> >> >>> >>> Thanks, >>> Han >>> >>> >> >
diff --git a/ovn/utilities/ovndb-servers.ocf b/ovn/utilities/ovndb-servers.ocf index 164b6bc..1b4b6ab 100755 --- a/ovn/utilities/ovndb-servers.ocf +++ b/ovn/utilities/ovndb-servers.ocf @@ -9,6 +9,8 @@ : ${SB_MASTER_PROTO_DEFAULT="tcp"} : ${MANAGE_NORTHD_DEFAULT="no"} : ${INACTIVE_PROBE_DEFAULT="5000"} +: ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"} +: ${LISTEN_ON_SLAVE_DEFAULT="yes"} CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot" CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config --name OVN_REPL_INFO -s ovn_ovsdb_master_server" @@ -21,6 +23,14 @@ SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}} MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}} INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${INACTIVE_PROBE_DEFAULT}} +# In order for pacemaker to work with LB, we can keep LISTEN_ON_MASTER_IP_ONLY +# to false and pass LB vip IP while creating pcs resource. +LISTEN_ON_MASTER_IP_ONLY=${OCF_RESKEY_listen_on_master_ip_only:-${LISTEN_ON_MASTER_IP_ONLY_DEFAULT}} + +# In order for pacemaker to work with LB, we can also set LISTEN_ON_SLAVE +# to false so that slaves do not listen on 0.0.0.0. +LISTEN_ON_SLAVE=${OCF_RESKEY_listen_on_slave:-${LISTEN_ON_SLAVE_DEFAULT}} + # Invalid IP address is an address that can never exist in the network, as # mentioned in rfc-5737. The ovsdb servers connects to this IP address till # a master is promoted and the IPAddr2 resource is started. @@ -157,22 +167,24 @@ ovsdb_server_notify() { ${OVN_CTL} --ovn-manage-ovsdb=no start_northd fi - conn=`ovn-nbctl get NB_global . connections` - if [ "$conn" == "[]" ] - then - ovn-nbctl -- --id=@conn_uuid create Connection \ + # TODO: Need to troubleshoot as to removing target is ok as well. + 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 \ 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 @@ -295,15 +307,15 @@ ovsdb_server_start() { set ${OVN_CTL} - set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} - set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} + # For LB vip to talk to master pool member on a specific tcp port, we need + # to listen on 0.0.0.0.instead of master_ip + if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xno ]; then + set $@ --db-nb-port=${NB_MASTER_PORT} + set $@ --db-sb-port=${SB_MASTER_PORT} - if [ "x${NB_MASTER_PROTO}" = xtcp ]; then - set $@ --db-nb-create-insecure-remote=yes - fi - - if [ "x${SB_MASTER_PROTO}" = xtcp ]; then - set $@ --db-sb-create-insecure-remote=yes + else + set $@ --db-nb-addr=${MASTER_IP} --db-nb-port=${NB_MASTER_PORT} + set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT} fi if [ "x${present_master}" = x ]; then @@ -313,15 +325,44 @@ ovsdb_server_start() { # Force all copies to come up as slaves by pointing them into # space and let pacemaker pick one to promote: # + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then + set $@ --db-nb-create-insecure-remote=yes + fi + + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then + set $@ --db-sb-create-insecure-remote=yes + fi set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} --db-sb-sync-from-addr=${INVALID_IP_ADDRESS} elif [ ${present_master} != ${host_name} ]; then + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then + # TODO: for using LB vip, need to test for ssl. + set $@ --db-nb-create-insecure-remote=no + set $@ --db-sb-create-insecure-remote=no + else + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then + set $@ --db-nb-create-insecure-remote=yes + fi + + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then + set $@ --db-sb-create-insecure-remote=yes + fi + fi # An existing master is active, connect to it set $@ --db-nb-sync-from-addr=${MASTER_IP} --db-sb-sync-from-addr=${MASTER_IP} set $@ --db-nb-sync-from-port=${NB_MASTER_PORT} 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} + + else + if [ "x${NB_MASTER_PROTO}" = xtcp ]; then + set $@ --db-nb-create-insecure-remote=yes + fi + + if [ "x${SB_MASTER_PROTO}" = xtcp ]; then + set $@ --db-sb-create-insecure-remote=yes + fi fi $@ start_ovsdb @@ -416,6 +457,11 @@ ovsdb_server_promote() { ;; esac + if [ "x${LISTEN_ON_SLAVE}" = xno ]; then + # Restart ovs so that new master can listen on tcp port + ${OVN_CTL} stop_ovsdb + ovsdb_server_start + fi ${OVN_CTL} promote_ovnnb ${OVN_CTL} promote_ovnsb @@ -514,3 +560,4 @@ esac rc=$? exit $rc +
using pacemaker so that controllers can be placed in different fault domains. Signed-off-by: aginwala <aginwala@ebay.com> --- ovn/utilities/ovndb-servers.ocf | 83 ++++++++++++++++++++++++++++++++--------- 1 file changed, 65 insertions(+), 18 deletions(-)