From 7e2e87cfc7548375d7ec45a9b84b1f9a353eabdb Mon Sep 17 00:00:00 2001 From: Srini Seetharaman Date: Tue, 16 Sep 2014 10:07:08 -0700 Subject: [PATCH] Fixing Bug 1900 and 1908 Updating LBaaSPoolMemberHandler to process member add/delete correctly. Previously the rules were not pushed or deletede properly. Also committing the index subtraction in LoadBalancerService that was not merged properly in the earlier commit. Change-Id: If367895407a284333a0438ae76312063fe3d8ded Signed-off-by: Srini Seetharaman --- .../services/LoadBalancerService.java | 6 +- .../netvirt/LBaaSPoolMemberHandler.java | 71 ++++++++++++++++--- .../openstack/netvirt/NeutronCacheUtils.java | 37 +++++----- 3 files changed, 84 insertions(+), 30 deletions(-) diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/LoadBalancerService.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/LoadBalancerService.java index 6c06c2742..6f27ca6de 100644 --- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/LoadBalancerService.java +++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/LoadBalancerService.java @@ -92,7 +92,8 @@ public class LoadBalancerService extends AbstractServiceInstance implements Load logger.trace("Ignoring non-OpenFlow node {} from flow programming", node); return new Status(StatusCode.BADREQUEST); } - logger.debug("Performing {} rules for VIP {} and member {}", action, lbConfig.getVip(), member.getIP()); + logger.debug("Performing {} rules for member {} with index {} on LB with VIP {} and total members {}", + action, member.getIP(), member.getIndex(), lbConfig.getVip(), lbConfig.getMembers().size()); NodeBuilder nodeBuilder = new NodeBuilder(); nodeBuilder.setId(new NodeId(Constants.OPENFLOW_NODE_PREFIX + String.valueOf(node.getID()))); @@ -170,7 +171,8 @@ public class LoadBalancerService extends AbstractServiceInstance implements Load ab = new ActionBuilder(); ab.setAction(ActionUtils.nxMultipathAction(OfjNxHashFields.NXHASHFIELDSSYMMETRICL4, - (Integer)0, OfjNxMpAlgorithm.NXMPALGMODULON, (Integer)lbConfig.getMembers().size(), + (Integer)0, OfjNxMpAlgorithm.NXMPALGMODULON, + (Integer)lbConfig.getMembers().size()-1, //By Nicira-Ext spec, this field is max_link minus 1 (Long)0L, new DstNxRegCaseBuilder().setNxReg(REG_FIELD_B).build(), (Integer)0, (Integer)31)); ab.setOrder(1); diff --git a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/LBaaSPoolMemberHandler.java b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/LBaaSPoolMemberHandler.java index eeac77ef4..cd9417ba7 100755 --- a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/LBaaSPoolMemberHandler.java +++ b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/LBaaSPoolMemberHandler.java @@ -120,6 +120,35 @@ public class LBaaSPoolMemberHandler extends AbstractHandler enqueueEvent(new NorthboundEvent(neutronLBPoolMember, Action.DELETE)); } + private void doNeutronLoadBalancerPoolMemberDelete(NeutronLoadBalancerPoolMember neutronLBPoolMember) { + Preconditions.checkNotNull(loadBalancerProvider); + + LoadBalancerConfiguration lbConfig = extractLBConfiguration(neutronLBPoolMember); + if (lbConfig == null) { + logger.trace("Neutron LB configuration invalid for member {} ", neutronLBPoolMember.getPoolMemberAddress()); + } else if (lbConfig.getVip() == null) { + logger.trace("Neutron LB VIP not created yet for member {} ", neutronLBPoolMember.getPoolMemberID()); + } else if (!lbConfig.isValid()) { + logger.trace("Neutron LB pool configuration invalid for {} ", lbConfig.getName()); + } else if (this.switchManager.getNodes().size() == 0) { + logger.trace("Noop with LB pool member {} creation because no nodes available.", neutronLBPoolMember.getPoolMemberID()); + } else { + /* As of now, deleting a member involves recomputing member indices. + * This is best done through a complete update of the load balancer instance. + */ + for (Node node: this.switchManager.getNodes()) + loadBalancerProvider.programLoadBalancerRules(node, lbConfig, Action.DELETE); + + for (NeutronLoadBalancer neutronLB: neutronLBCache.getAllNeutronLoadBalancers()) { + String loadBalancerSubnetID = neutronLB.getLoadBalancerVipSubnetID(); + if (neutronLBPoolMember.getPoolMemberSubnetID().equals(loadBalancerSubnetID)) { + enqueueEvent(new NorthboundEvent(neutronLB, Action.ADD)); + break; + } + } + } + } + /** * Process the event. * @@ -128,6 +157,7 @@ public class LBaaSPoolMemberHandler extends AbstractHandler */ @Override public void processEvent(AbstractEvent abstractEvent) { + logger.debug("Processing Loadbalancer member event " + abstractEvent); if (!(abstractEvent instanceof NorthboundEvent)) { logger.error("Unable to process abstract event " + abstractEvent); return; @@ -138,17 +168,7 @@ public class LBaaSPoolMemberHandler extends AbstractHandler doNeutronLoadBalancerPoolMemberCreate(ev.getLoadBalancerPoolMember()); break; case DELETE: - /* As of now, deleting a member involves recomputing member indices. - * This is best done through a complete update of the load balancer instance. - */ - for (NeutronLoadBalancer neutronLB: neutronLBCache.getAllNeutronLoadBalancers()) { - String loadBalancerSubnetID = neutronLB.getLoadBalancerVipSubnetID(); - if (ev.getLoadBalancerPoolMember() - .getPoolMemberSubnetID().equals(loadBalancerSubnetID)) { - enqueueEvent(new NorthboundEvent(neutronLB, Action.UPDATE)); - break; - } - } + doNeutronLoadBalancerPoolMemberDelete(ev.getLoadBalancerPoolMember()); break; case UPDATE: /** @@ -200,11 +220,40 @@ public class LBaaSPoolMemberHandler extends AbstractHandler break; } } + /** * It is possible that the VIP has not been created yet. * In that case, we create dummy configuration that will not program rules. */ LoadBalancerConfiguration lbConfig = new LoadBalancerConfiguration(loadBalancerName, loadBalancerVip); + + /* Extract all other active members and include in LB config + */ + String otherMemberID, otherMemberSubnetID, otherMemberIP, otherMemberMAC, otherMemberProtocol; + Boolean otherMemberAdminStateIsUp; + Integer otherMemberPort; + + for (NeutronLoadBalancerPoolMember otherMember: neutronLBPool.getLoadBalancerPoolMembers()) { + otherMemberID = otherMember.getPoolMemberID(); + if (otherMemberID.equals(memberID)) + continue; //skip + + otherMemberIP = otherMember.getPoolMemberAddress(); + otherMemberAdminStateIsUp = otherMember.getPoolMemberAdminStateIsUp(); + otherMemberSubnetID = otherMember.getPoolMemberSubnetID(); + otherMemberPort = otherMember.getPoolMemberProtoPort(); + otherMemberProtocol = memberProtocol; + + if (otherMemberIP == null || otherMemberSubnetID == null || otherMemberAdminStateIsUp == null) + continue; + else if (otherMemberAdminStateIsUp.booleanValue()) { + otherMemberMAC = NeutronCacheUtils.getMacAddress(neutronPortsCache, otherMemberIP); + if (otherMemberMAC == null) + continue; + lbConfig.addMember(otherMemberID, otherMemberIP, otherMemberMAC, otherMemberProtocol, otherMemberPort); + } + } + lbConfig.addMember(memberID, memberIP, memberMAC, memberProtocol, memberPort); return lbConfig; } diff --git a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/NeutronCacheUtils.java b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/NeutronCacheUtils.java index 3b893d9a3..9f0984a94 100755 --- a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/NeutronCacheUtils.java +++ b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/NeutronCacheUtils.java @@ -24,24 +24,27 @@ public class NeutronCacheUtils { * @return MAC address registered with that IP address */ public static String getMacAddress(INeutronPortCRUD neutronPortsCache, String ipAddr) { - List fixedIPs; - Iterator fixedIPIterator; - Neutron_IPs ip; + if (ipAddr == null) + return null; - List allPorts = neutronPortsCache.getAllPorts(); - Iterator i = allPorts.iterator(); - while (i.hasNext()) { - NeutronPort port = i.next(); - fixedIPs = port.getFixedIPs(); - if (fixedIPs != null && fixedIPs.size() > 0) { - fixedIPIterator = fixedIPs.iterator(); - while (fixedIPIterator.hasNext()) { - ip = fixedIPIterator.next(); - if (ip.getIpAddress().equals(ipAddr)) - return port.getMacAddress(); - } - } - } + List fixedIPs; + Iterator fixedIPIterator; + Neutron_IPs ip; + + List allPorts = neutronPortsCache.getAllPorts(); + Iterator i = allPorts.iterator(); + while (i.hasNext()) { + NeutronPort port = i.next(); + fixedIPs = port.getFixedIPs(); + if (fixedIPs != null && fixedIPs.size() > 0) { + fixedIPIterator = fixedIPs.iterator(); + while (fixedIPIterator.hasNext()) { + ip = fixedIPIterator.next(); + if (ip.getIpAddress().equals(ipAddr)) + return port.getMacAddress(); + } + } + } return null; } } -- 2.36.6