ClusterLabs / resource-agents

Combined repository of OCF agents from the RHCS and Linux-HA projects
GNU General Public License v2.0
492 stars 582 forks source link

IPaddr2 LVS_SUPPORT not working with labels on interfaces #200

Open tobiasstein opened 11 years ago

tobiasstein commented 11 years ago

The label/alias on the loopback interface (lo:vip) is not restored if the Virtual IP is migrated. This is a small patch to the commit of January 14th 2013.

443,453c443
<       case $NIC in
<           *:*)
<               IFLABEL=$NIC
<               NIC=`echo $NIC | sed 's/:.*//'`
<               ;;
<           *)
<               if [ -n "$IFLABEL" ]; then
<                       IFLABEL=${NIC}:${IFLABEL}
<               fi
<               ;;
<       esac

---
>       NIC=`echo $NIC | sed 's/:.*//'`
545,546c535,536
<               cmd="$cmd label $label"
<               msg="${msg} (with label $label)"

---
>               cmd="$cmd label $iface:$label"
>               msg="${msg} (with label $iface:$label)"
617c607
<               add_interface $ifinfo

---
>               add_interface $ifinfo $IFLABEL
kskmori commented 11 years ago
          add_interface $ifinfo $IFLABEL

I think that it does not really 'restore'; if lo:0 was configured at first and the RA was configured to use eth0:1 then lo:1 is 'restored' after a migration. Isn't that rather confusing?

Also the broadcast isn't restored either in the current implementation. The correct way to fix would be that it should obtain the correct iflabel/broadcast etc. from lo via the ip command and save them to $VLDIR/$ipaddr file when removing the Virtual IP (in remove_conflicting_loopback()) and then use the saved information when restoring.

As for the former part of the patch, it can not support nic=eth0:iflabel format. We would like to continue to support that format in order to keep the backward compatibility. I realized that the recent code did not work with it and submitted a patch to fix it. It would be appreciated if you could rewrite a patch based on this fix: https://github.com/ClusterLabs/resource-agents/pull/201

Finally, I am wondering if this issue is really worth to fix honestly. Again, I think lvs_support (removing a virtual IP on lo) should be no longer necessary; I have many load-balancing systems around me using the LVS-DR, ldirectord and IPaddr2 without enabling lvs_support. They are well tested and running in production. I am really grad if you could let me know if you had any problems without lvs_support.

Thanks,

dmuhamedagic commented 11 years ago

@tobiasstein do you want to discuss this further? Add code? Close?