Message ID | 20210325093210.196855-1-michele@acksyn.org |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev] Fix connection string in case of changes in the ovndb-servers.ocf RA | expand |
Bleep bloop. Greetings Michele Baldessari, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line has non-spaces leading whitespace
#52 FILE: utilities/ovndb-servers.ocf:262:
else
Lines checked: 70, Warnings: 1, Errors: 0
build:
/bin/python3 ./build-aux/dpdkstrip.py | \
sed \
-e 's,[@]PKIDIR[@],/usr/local/var/lib/openvswitch/pki,g' \
-e 's,[@]LOGDIR[@],/usr/local/var/log/ovn,g' \
-e 's,[@]DBDIR[@],/usr/local/etc/ovn,g' \
-e 's,[@]PYTHON3[@],/bin/python3,g' \
-e 's,[@]OVN_RUNDIR[@],/usr/local/var/run/ovn,g' \
-e 's,[@]OVSBUILDDIR[@],/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR,g' \
-e 's,[@]VERSION[@],21.03.90,g' \
-e 's,[@]OVSVERSION[@],2.15.90,g' \
-e 's,[@]localstatedir[@],/usr/local/var,g' \
-e 's,[@]pkgdatadir[@],/usr/local/share/ovn,g' \
-e 's,[@]sysconfdir[@],/usr/local/etc,g' \
-e 's,[@]bindir[@],/usr/local/bin,g' \
-e 's,[@]sbindir[@],/usr/local/sbin,g' \
-e 's,[@]abs_builddir[@],/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace,g' \
-e 's,[@]abs_top_srcdir[@],/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace,g' \
> rhel/ovn-fedora.spec.tmp
mv rhel/ovn-fedora.spec.tmp rhel/ovn-fedora.spec
utilities/ovndb-servers.ocf
See above for files that use tabs for indentation.
Please use spaces instead.
make[1]: *** [check-tabs] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace'
make: *** [all] Error 2
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
On Thu, Mar 25, 2021 at 3:02 PM Michele Baldessari <michele@acksyn.org> wrote: > > Currently if you update the master_ip attribute, pacemaker will restart > the resource but the ovn master role will still listen to the previous > old ip address. The reason for this is the following piece of code: > > 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}\:${LISTEN_ON_IP}" \ > inactivity_probe=$INACTIVE_PROBE -- set NB_Global . connections=@conn_uuid > fi > > Once the connection is set the first time 'get NB_global . connections' > will always return the UUID of the connection so we're never changing > the connection data. > > Let's set the connection info whenever the connection is not "[]" > > Tested as follows: > 1) Started with the following listening ip addresses (resource is configured with master_ip=172.17.1.44): > tcp LISTEN 0 10 172.17.1.44:6641 0.0.0.0:* users:(("ovsdb-server",pid=150360,fd=15)) ino:18609895 sk:4d <-> > tcp LISTEN 0 10 172.17.1.44:6642 0.0.0.0:* users:(("ovsdb-server",pid=150379,fd=15)) ino:18609038 sk:4e <-> > > 2) Updated the resource master_ip with: > pcs resource update ovndb_servers master_ip=4.5.6.7 > > 3) Observed the new listening ip addresses on the master node: > [root@controller-3 stdouts]# ss -natulpe |grep 664[12] > tcp LISTEN 0 10 4.5.6.7:6641 0.0.0.0:* users:(("ovsdb-server",pid=516770,fd=15)) ino:11641860 sk:4d <-> > tcp LISTEN 0 10 4.5.6.7:6642 0.0.0.0:* users:(("ovsdb-server",pid=516789,fd=15)) ino:11642885 sk:4e <-> > > Previously we'd observe ovsdb-server listening to the old IP even after > the resource change. > > Signed-off-by: Michele Baldessari <michele@acksyn.org> Thanks Michele for the patch. There was compilation issue due to tab being used. I fixed it and applied the patch to the main branch. Please let me know if it needs backports for the released branches. Thanks Numan > --- > utilities/ovndb-servers.ocf | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/utilities/ovndb-servers.ocf b/utilities/ovndb-servers.ocf > index 7351c7d64a94..896d590ecc78 100755 > --- a/utilities/ovndb-servers.ocf > +++ b/utilities/ovndb-servers.ocf > @@ -259,6 +259,9 @@ ovsdb_server_notify() { > ovn-nbctl -- --id=@conn_uuid create Connection \ > target="p${NB_MASTER_PROTO}\:${NB_MASTER_PORT}\:${LISTEN_ON_IP}" \ > inactivity_probe=$INACTIVE_PROBE -- set NB_Global . connections=@conn_uuid > + else > + CONN_UID=$(sed -e 's/^\[//' -e 's/\]$//' <<< ${conn}) > + ovn-nbctl set connection "${CONN_UID}" target="p${NB_MASTER_PROTO}\:${NB_MASTER_PORT}\:${LISTEN_ON_IP}" > fi > > conn=`ovn-sbctl get SB_global . connections` > @@ -267,6 +270,9 @@ inactivity_probe=$INACTIVE_PROBE -- set NB_Global . connections=@conn_uuid > ovn-sbctl -- --id=@conn_uuid create Connection \ > target="p${SB_MASTER_PROTO}\:${SB_MASTER_PORT}\:${LISTEN_ON_IP}" \ > inactivity_probe=$INACTIVE_PROBE -- set SB_Global . connections=@conn_uuid > + else > + CONN_UID=$(sed -e 's/^\[//' -e 's/\]$//' <<< ${conn}) > + ovn-sbctl set connection "${CONN_UID}" target="p${SB_MASTER_PROTO}\:${SB_MASTER_PORT}\:${LISTEN_ON_IP}" > fi > > else > -- > 2.30.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/utilities/ovndb-servers.ocf b/utilities/ovndb-servers.ocf index 7351c7d64a94..896d590ecc78 100755 --- a/utilities/ovndb-servers.ocf +++ b/utilities/ovndb-servers.ocf @@ -259,6 +259,9 @@ ovsdb_server_notify() { ovn-nbctl -- --id=@conn_uuid create Connection \ target="p${NB_MASTER_PROTO}\:${NB_MASTER_PORT}\:${LISTEN_ON_IP}" \ inactivity_probe=$INACTIVE_PROBE -- set NB_Global . connections=@conn_uuid + else + CONN_UID=$(sed -e 's/^\[//' -e 's/\]$//' <<< ${conn}) + ovn-nbctl set connection "${CONN_UID}" target="p${NB_MASTER_PROTO}\:${NB_MASTER_PORT}\:${LISTEN_ON_IP}" fi conn=`ovn-sbctl get SB_global . connections` @@ -267,6 +270,9 @@ inactivity_probe=$INACTIVE_PROBE -- set NB_Global . connections=@conn_uuid ovn-sbctl -- --id=@conn_uuid create Connection \ target="p${SB_MASTER_PROTO}\:${SB_MASTER_PORT}\:${LISTEN_ON_IP}" \ inactivity_probe=$INACTIVE_PROBE -- set SB_Global . connections=@conn_uuid + else + CONN_UID=$(sed -e 's/^\[//' -e 's/\]$//' <<< ${conn}) + ovn-sbctl set connection "${CONN_UID}" target="p${SB_MASTER_PROTO}\:${SB_MASTER_PORT}\:${LISTEN_ON_IP}" fi else
Currently if you update the master_ip attribute, pacemaker will restart the resource but the ovn master role will still listen to the previous old ip address. The reason for this is the following piece of code: 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}\:${LISTEN_ON_IP}" \ inactivity_probe=$INACTIVE_PROBE -- set NB_Global . connections=@conn_uuid fi Once the connection is set the first time 'get NB_global . connections' will always return the UUID of the connection so we're never changing the connection data. Let's set the connection info whenever the connection is not "[]" Tested as follows: 1) Started with the following listening ip addresses (resource is configured with master_ip=172.17.1.44): tcp LISTEN 0 10 172.17.1.44:6641 0.0.0.0:* users:(("ovsdb-server",pid=150360,fd=15)) ino:18609895 sk:4d <-> tcp LISTEN 0 10 172.17.1.44:6642 0.0.0.0:* users:(("ovsdb-server",pid=150379,fd=15)) ino:18609038 sk:4e <-> 2) Updated the resource master_ip with: pcs resource update ovndb_servers master_ip=4.5.6.7 3) Observed the new listening ip addresses on the master node: [root@controller-3 stdouts]# ss -natulpe |grep 664[12] tcp LISTEN 0 10 4.5.6.7:6641 0.0.0.0:* users:(("ovsdb-server",pid=516770,fd=15)) ino:11641860 sk:4d <-> tcp LISTEN 0 10 4.5.6.7:6642 0.0.0.0:* users:(("ovsdb-server",pid=516789,fd=15)) ino:11642885 sk:4e <-> Previously we'd observe ovsdb-server listening to the old IP even after the resource change. Signed-off-by: Michele Baldessari <michele@acksyn.org> --- utilities/ovndb-servers.ocf | 6 ++++++ 1 file changed, 6 insertions(+)