On Tue, May 29, 2018 at 11:40 PM, Numan Siddique <[email protected]>
wrote:
>
> On Wed, May 30, 2018 at 5:31 AM, aginwala <[email protected]> wrote:
>
> > Thanks for the Review.
> >
> > On Wed, May 23, 2018 at 11:36 AM, Numan Siddique <[email protected]>
> > wrote:
> >
> >> On Wed, May 9, 2018 at 12:13 AM, aginwala <[email protected]> wrote:
> >>
> >> > using pacemaker so that controllers can be placed in different fault
> >> > domains.
> >> >
> >> > Signed-off-by: aginwala <[email protected]>
> >> >
> >>
> >> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to