[ovs-dev] [PATCH v2] ovndb-servers.ocf: add support for managing cluster

aginwala aginwala at asu.edu
Wed May 30 21:44:32 UTC 2018


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 at redhat.com>
wrote:

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


More information about the dev mailing list