diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/DistributedOpenstackNetworkStore.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/DistributedOpenstackNetworkStore.java index 3581580b3b..04ca5942de 100644 --- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/DistributedOpenstackNetworkStore.java +++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/DistributedOpenstackNetworkStore.java @@ -93,6 +93,8 @@ public class DistributedOpenstackNetworkStore private static final String ERR_NOT_FOUND = " does not exist"; private static final String ERR_DUPLICATE = " already exists"; + private static final long TIMEOUT_MS = 2000; // wait for 2s + private static final KryoNamespace SERIALIZER_NEUTRON_L2 = KryoNamespace.newBuilder() .register(KryoNamespaces.API) .register(Network.class) @@ -275,11 +277,21 @@ public class DistributedOpenstackNetworkStore log.debug("Prepare OpenStack port remove"); + long timeoutExpiredMs = System.currentTimeMillis() + TIMEOUT_MS; + while (true) { + + long waitMs = timeoutExpiredMs - System.currentTimeMillis(); + if (preCommitPortService.subscriberCountByEventType( portId, OPENSTACK_PORT_PRE_REMOVE) == 0) { break; } + + if (waitMs <= 0) { + log.debug("Timeout waiting for port removal."); + break; + } } Versioned osPort = osPortStore.remove(portId); diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/InstancePortManager.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/InstancePortManager.java index 9878de0a70..3f204fbf4a 100644 --- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/InstancePortManager.java +++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/InstancePortManager.java @@ -146,6 +146,13 @@ public class InstancePortManager checkArgument(!Strings.isNullOrEmpty(instancePort.portId()), ERR_NULL_INSTANCE_PORT_ID); + // in case OpenStack removes the port prior to OVS, we will not update + // the instance port as it does not exist in the store + if (instancePortStore.instancePort(instancePort.portId()) == null) { + log.warn("Unable to update instance port {}, as it does not exist", instancePort.portId()); + return; + } + instancePortStore.updateInstancePort(instancePort); log.info(String.format(MSG_INSTANCE_PORT, instancePort.portId(), MSG_UPDATED)); } diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java index 9e47a0c72f..d751c03dca 100644 --- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java +++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java @@ -15,6 +15,7 @@ */ package org.onosproject.openstacknetworking.impl; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import org.apache.felix.scr.annotations.Activate; @@ -430,6 +431,12 @@ public class OpenstackRoutingArpHandler { return; } + if (portId == null || fip.getPortId() == null) { + log.trace("Unknown target ARP request for {}, ignore it", + fip.getFloatingIpAddress()); + return; + } + InstancePort instPort = instancePortService.instancePort(portId); MacAddress targetMac = instPort.macAddress(); @@ -521,40 +528,46 @@ public class OpenstackRoutingArpHandler { ); break; case OPENSTACK_FLOATING_IP_ASSOCIATED: - eventExecutor.execute(() -> { - NetFloatingIP osFip = event.floatingIp(); - - // associate a floating IP with an existing VM - setFloatingIpArpRule(event.floatingIp(), event.portId(), completedGws, true); - }); + if (getValidPortId(event) != null) { + eventExecutor.execute(() -> { + // associate a floating IP with an existing VM + setFloatingIpArpRule(event.floatingIp(), getValidPortId(event), + completedGws, true); + }); + } break; case OPENSTACK_FLOATING_IP_DISASSOCIATED: - eventExecutor.execute(() -> { - NetFloatingIP osFip = event.floatingIp(); - - // associate a floating IP with an existing VM - setFloatingIpArpRule(event.floatingIp(), event.portId(), completedGws, false); - }); + if (getValidPortId(event) != null) { + eventExecutor.execute(() -> { + // disassociate a floating IP with an existing VM + setFloatingIpArpRule(event.floatingIp(), getValidPortId(event), + completedGws, false); + }); + } break; case OPENSTACK_FLOATING_IP_CREATED: - eventExecutor.execute(() -> { - NetFloatingIP osFip = event.floatingIp(); - - // during floating IP creation, if the floating IP is - // associated with any port of VM, then we will set - // floating IP related ARP rules to gateway node - setFloatingIpArpRule(osFip, event.portId(), completedGws, true); - }); + // during floating IP creation, if the floating IP is + // associated with any port of VM, then we will set + // floating IP related ARP rules to gateway node + if (getValidPortId(event) != null) { + eventExecutor.execute(() -> { + // associate a floating IP with an existing VM + setFloatingIpArpRule(event.floatingIp(), getValidPortId(event), + completedGws, true); + }); + } break; case OPENSTACK_FLOATING_IP_REMOVED: - eventExecutor.execute(() -> { - NetFloatingIP osFip = event.floatingIp(); - - // during floating IP deletion, if the floating IP is - // still associated with any port of VM, then we will - // remove floating IP related ARP rules from gateway node - setFloatingIpArpRule(event.floatingIp(), event.portId(), completedGws, false); - }); + // during floating IP deletion, if the floating IP is + // still associated with any port of VM, then we will + // remove floating IP related ARP rules from gateway node + if (getValidPortId(event) != null) { + eventExecutor.execute(() -> { + // associate a floating IP with an existing VM + setFloatingIpArpRule(event.floatingIp(), getValidPortId(event), + completedGws, false); + }); + } break; default: // do nothing for the other events @@ -562,6 +575,21 @@ public class OpenstackRoutingArpHandler { } } + private String getValidPortId(OpenstackRouterEvent event) { + NetFloatingIP osFip = event.floatingIp(); + String portId = osFip.getPortId(); + + if (Strings.isNullOrEmpty(portId)) { + portId = event.portId(); + } + + if (portId != null && instancePortService.instancePort(portId) != null) { + return portId; + } + + return null; + } + private Set getExternalGatewaySnatIps(ExternalGateway extGw) { return osNetworkAdminService.ports().stream() .filter(port -> diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingFloatingIpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingFloatingIpHandler.java index 73e9f19b7b..f20eb5194c 100644 --- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingFloatingIpHandler.java +++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingFloatingIpHandler.java @@ -90,7 +90,6 @@ import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.i import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.swapStaleLocation; import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.buildExtension; import static org.onosproject.openstacknode.api.OpenstackNode.NodeType.GATEWAY; -import static org.openstack4j.model.network.NetworkType.FLAT; /** * Handles OpenStack floating IP events. @@ -204,11 +203,11 @@ public class OpenstackRoutingFloatingIpHandler { if (install) { preCommitPortService.subscribePreCommit(osPort.getId(), OPENSTACK_PORT_PRE_REMOVE, this.getClass().getName()); - log.info("Subscribed the port pre-remove event"); + log.info("Subscribed the port {} on listening pre-remove event", osPort.getId()); } else { preCommitPortService.unsubscribePreCommit(osPort.getId(), OPENSTACK_PORT_PRE_REMOVE, this.getClass().getName()); - log.info("Unsubscribed the port pre-remove event"); + log.info("Unsubscribed the port {} on listening pre-remove event", osPort.getId()); } updateComputeNodeRules(instPort, osNet, gateway, install); @@ -690,6 +689,12 @@ public class OpenstackRoutingFloatingIpHandler { if (osNetworkService.port(osFip.getPortId()) != null) { disassociateFloatingIp(osFip, osFip.getPortId()); } + + // since we skip floating IP disassociation, we need to + // manually unsubscribe the port pre-remove event + preCommitPortService.unsubscribePreCommit(osFip.getPortId(), + OPENSTACK_PORT_PRE_REMOVE, this.getClass().getName()); + log.info("Unsubscribed the port {} on listening pre-remove event", osFip.getPortId()); } log.info("Removed floating IP {}", osFip.getFloatingIpAddress()); }); @@ -945,8 +950,7 @@ public class OpenstackRoutingFloatingIpHandler { public boolean isRelevant(OpenstackNetworkEvent event) { // do not allow to proceed without leadership NodeId leader = leadershipService.getLeader(appId.name()); - return Objects.equals(localNodeId, leader) && - event.subject().getNetworkType() != FLAT; + return Objects.equals(localNodeId, leader); } @Override diff --git a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/InstancePortManagerTest.java b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/InstancePortManagerTest.java index d7f0291d2b..3f191a48a8 100644 --- a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/InstancePortManagerTest.java +++ b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/InstancePortManagerTest.java @@ -296,7 +296,7 @@ public class InstancePortManagerTest { /** * Tests if updating an unregistered instance port fails with an exception. */ - @Test(expected = IllegalArgumentException.class) + @Test public void testUpdateUnregisteredInstancePort() { target.updateInstancePort(instancePort1); }