Fix: resolve floating IP rules not deleting issue in multinodes env
Change-Id: Idcafcf9e9747ccc9e25cc9c6862707a28b3bef95
diff --git a/apps/openstacknetworking/api/src/main/java/org/onosproject/openstacknetworking/api/PreCommitPortService.java b/apps/openstacknetworking/api/src/main/java/org/onosproject/openstacknetworking/api/PreCommitPortService.java
index bc06e17..43d7901 100644
--- a/apps/openstacknetworking/api/src/main/java/org/onosproject/openstacknetworking/api/PreCommitPortService.java
+++ b/apps/openstacknetworking/api/src/main/java/org/onosproject/openstacknetworking/api/PreCommitPortService.java
@@ -20,5 +20,5 @@
/**
* Handles port precommit request.
*/
-public interface PreCommitPortService extends PreCommitService<String, Type> {
+public interface PreCommitPortService extends PreCommitService<String, Type, InstancePortAdminService> {
}
diff --git a/apps/openstacknetworking/api/src/main/java/org/onosproject/openstacknetworking/api/PreCommitService.java b/apps/openstacknetworking/api/src/main/java/org/onosproject/openstacknetworking/api/PreCommitService.java
index 4b94997..c5f26cd 100644
--- a/apps/openstacknetworking/api/src/main/java/org/onosproject/openstacknetworking/api/PreCommitService.java
+++ b/apps/openstacknetworking/api/src/main/java/org/onosproject/openstacknetworking/api/PreCommitService.java
@@ -18,7 +18,7 @@
/**
* Handles precommit request.
*/
-public interface PreCommitService<T, E> {
+public interface PreCommitService<T, E, S> {
/**
* Subscribes pre-update event for the given subject inside the given class.
@@ -34,9 +34,10 @@
*
* @param subject subject to unsubscribe
* @param eventType event type (update or remove)
+ * @param service service instance
* @param className target class name
*/
- void unsubscribePreCommit(T subject, E eventType, String className);
+ void unsubscribePreCommit(T subject, E eventType, S service, String className);
/**
* Obtains the count value of subscribers for the given subject and event type.
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 04ca594..0cc63b9 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
@@ -262,38 +262,6 @@
@Override
public Port removePort(String portId) {
-
- Port port = osPortStore.asJavaMap().get(portId);
-
- if (port == null) {
- return null;
- }
-
- eventExecutor.execute(() ->
- notifyDelegate(new OpenstackNetworkEvent(
- OPENSTACK_PORT_PRE_REMOVE,
- network(port.getNetworkId()), port))
- );
-
- 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<Port> osPort = osPortStore.remove(portId);
return osPort == null ? null : osPort.value();
}
@@ -417,6 +385,14 @@
break;
case REMOVE:
log.debug("OpenStack port removed");
+
+ eventExecutor.execute(() ->
+ notifyDelegate(new OpenstackNetworkEvent(
+ OPENSTACK_PORT_PRE_REMOVE,
+ network(event.oldValue().value().getNetworkId()),
+ event.oldValue().value()))
+ );
+
eventExecutor.execute(() ->
notifyDelegate(new OpenstackNetworkEvent(
OPENSTACK_PORT_REMOVED,
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 b23279d..26be07e 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
@@ -171,26 +171,21 @@
log.info("Stopped");
}
- private void setFloatingIpRules(NetFloatingIP floatingIp, Port osPort,
+ private void setFloatingIpRules(NetFloatingIP floatingIp, InstancePort instPort,
OpenstackNode gateway, boolean install) {
- Network osNet = osNetworkService.network(osPort.getNetworkId());
+
+ if (instPort == null) {
+ log.debug("No instance port found");
+ return;
+ }
+
+ Network osNet = osNetworkService.network(instPort.networkId());
if (osNet == null) {
final String errorFormat = ERR_FLOW + "no network(%s) exists";
final String error = String.format(errorFormat,
floatingIp.getFloatingIpAddress(),
- osPort.getNetworkId());
- throw new IllegalStateException(error);
- }
-
- MacAddress srcMac = MacAddress.valueOf(osPort.getMacAddress());
- log.trace("Mac address of openstack port: {}", srcMac);
- InstancePort instPort = instancePortService.instancePort(srcMac);
-
- if (instPort == null) {
- final String errorFormat = ERR_FLOW + "no host(MAC:%s) found";
- final String error = String.format(errorFormat,
- floatingIp.getFloatingIpAddress(), srcMac);
+ instPort.networkId());
throw new IllegalStateException(error);
}
@@ -201,13 +196,13 @@
}
if (install) {
- preCommitPortService.subscribePreCommit(osPort.getId(),
+ preCommitPortService.subscribePreCommit(instPort.portId(),
OPENSTACK_PORT_PRE_REMOVE, this.getClass().getName());
- log.info("Subscribed the port {} on listening pre-remove event", osPort.getId());
+ log.info("Subscribed the port {} on listening pre-remove event", instPort.portId());
} else {
- preCommitPortService.unsubscribePreCommit(osPort.getId(),
- OPENSTACK_PORT_PRE_REMOVE, this.getClass().getName());
- log.info("Unsubscribed the port {} on listening pre-remove event", osPort.getId());
+ preCommitPortService.unsubscribePreCommit(instPort.portId(),
+ OPENSTACK_PORT_PRE_REMOVE, instancePortService, this.getClass().getName());
+ log.info("Unsubscribed the port {} on listening pre-remove event", instPort.portId());
}
updateComputeNodeRules(instPort, osNet, gateway, install);
@@ -601,42 +596,32 @@
}
private void associateFloatingIp(NetFloatingIP osFip) {
- Port osPort = osNetworkService.port(osFip.getPortId());
- if (osPort == null) {
- final String errorFormat = ERR_FLOW + "port(%s) not found";
- final String error = String.format(errorFormat,
- osFip.getFloatingIpAddress(), osFip.getPortId());
- throw new IllegalStateException(error);
+ InstancePort instPort = instancePortService.instancePort(osFip.getPortId());
+
+ if (instPort == null) {
+ log.warn("Failed to insert floating IP rule for {} due to missing of port info.",
+ osFip.getFloatingIpAddress());
+ return;
}
+
// set floating IP rules only if the port is associated to a VM
- if (!Strings.isNullOrEmpty(osPort.getDeviceId())) {
-
- if (instancePortService.instancePort(osPort.getId()) == null) {
- return;
- }
-
- setFloatingIpRules(osFip, osPort, null, true);
+ if (!Strings.isNullOrEmpty(instPort.deviceId().toString())) {
+ setFloatingIpRules(osFip, instPort, null, true);
}
}
private void disassociateFloatingIp(NetFloatingIP osFip, String portId) {
- Port osPort = osNetworkService.port(portId);
+ InstancePort instPort = instancePortService.instancePort(portId);
- if (osPort == null) {
- final String errorFormat = ERR_FLOW + "port(%s) not found";
- final String error = String.format(errorFormat,
- osFip.getFloatingIpAddress(), osFip.getPortId());
- throw new IllegalStateException(error);
+ if (instPort == null) {
+ log.warn("Failed to remove floating IP rule for {} due to missing of port info.",
+ osFip.getFloatingIpAddress());
+ return;
}
// set floating IP rules only if the port is associated to a VM
- if (!Strings.isNullOrEmpty(osPort.getDeviceId())) {
-
- if (instancePortService.instancePort(osPort.getId()) == null) {
- return;
- }
-
- setFloatingIpRules(osFip, osPort, null, false);
+ if (!Strings.isNullOrEmpty(instPort.deviceId().toString())) {
+ setFloatingIpRules(osFip, instPort, null, false);
}
}
@@ -693,7 +678,8 @@
// 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());
+ OPENSTACK_PORT_PRE_REMOVE, instancePortService,
+ this.getClass().getName());
log.info("Unsubscribed the port {} on listening pre-remove event", osFip.getPortId());
}
log.info("Removed floating IP {}", osFip.getFloatingIpAddress());
@@ -738,16 +724,14 @@
}
Port osPort = osNetworkService.port(fip.getPortId());
- if (osPort == null) {
- log.warn("Failed to set floating IP {}", fip.getId());
+ InstancePort instPort = instancePortService.instancePort(fip.getPortId());
+
+ // we check both Openstack Port and Instance Port
+ if (osPort == null || instPort == null) {
continue;
}
- if (instancePortService.instancePort(fip.getPortId()) == null) {
- continue;
- }
-
- setFloatingIpRules(fip, osPort, event.subject(), true);
+ setFloatingIpRules(fip, instPort, event.subject(), true);
}
});
break;
@@ -838,14 +822,10 @@
switch (event.type()) {
case OPENSTACK_INSTANCE_PORT_DETECTED:
if (instPort != null && instPort.portId() != null) {
- String portId = instPort.portId();
-
- Port port = osNetworkService.port(portId);
-
osRouterAdminService.floatingIps().stream()
.filter(f -> f.getPortId() != null)
.filter(f -> f.getPortId().equals(instPort.portId()))
- .forEach(f -> setFloatingIpRules(f, port, null, true));
+ .forEach(f -> setFloatingIpRules(f, instPort, null, true));
}
break;
@@ -961,10 +941,6 @@
updateFipStore(instancePortService.instancePort(event.port().getId()))
);
break;
- case OPENSTACK_PORT_REMOVED:
- eventExecutor.execute(() ->
- instancePortService.removeInstancePort(event.port().getId()));
- break;
default:
break;
}
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/PreCommitPortManager.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/PreCommitPortManager.java
index e02ff35..e0788fb 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/PreCommitPortManager.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/PreCommitPortManager.java
@@ -24,6 +24,7 @@
import org.onlab.util.KryoNamespace;
import org.onosproject.core.ApplicationId;
import org.onosproject.core.CoreService;
+import org.onosproject.openstacknetworking.api.InstancePortAdminService;
import org.onosproject.openstacknetworking.api.OpenstackNetworkEvent;
import org.onosproject.openstacknetworking.api.OpenstackNetworkEvent.Type;
import org.onosproject.openstacknetworking.api.PreCommitPortService;
@@ -106,7 +107,8 @@
}
@Override
- public void unsubscribePreCommit(String portId, Type eventType, String className) {
+ public void unsubscribePreCommit(String portId, Type eventType,
+ InstancePortAdminService service, String className) {
store.computeIfPresent(portId, (k, v) -> {
@@ -121,6 +123,10 @@
return v;
});
+
+ if (subscriberCountByEventType(portId, eventType) == 0) {
+ service.removeInstancePort(portId);
+ }
}
@Override
diff --git a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/InstancePortAdminServiceAdapter.java b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/InstancePortAdminServiceAdapter.java
new file mode 100644
index 0000000..d103362
--- /dev/null
+++ b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/InstancePortAdminServiceAdapter.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2018-present Open Networking Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onosproject.openstacknetworking.impl;
+
+import org.onosproject.openstacknetworking.api.InstancePort;
+import org.onosproject.openstacknetworking.api.InstancePortAdminService;
+
+/**
+ * Test adapter for instance port admin service.
+ */
+public class InstancePortAdminServiceAdapter
+ extends InstancePortServiceAdapter implements InstancePortAdminService {
+ @Override
+ public void createInstancePort(InstancePort instancePort) {
+
+ }
+
+ @Override
+ public void updateInstancePort(InstancePort instancePort) {
+
+ }
+
+ @Override
+ public void removeInstancePort(String portId) {
+
+ }
+
+ @Override
+ public void clear() {
+
+ }
+}
diff --git a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManagerTest.java b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManagerTest.java
index a0b6af6..5e5dd9a 100644
--- a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManagerTest.java
+++ b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManagerTest.java
@@ -31,6 +31,7 @@
import org.onosproject.net.DeviceId;
import org.onosproject.net.device.DeviceServiceAdapter;
import org.onosproject.net.packet.PacketServiceAdapter;
+import org.onosproject.openstacknetworking.api.InstancePortAdminService;
import org.onosproject.openstacknetworking.api.OpenstackNetworkEvent;
import org.onosproject.openstacknetworking.api.OpenstackNetworkEvent.Type;
import org.onosproject.openstacknetworking.api.OpenstackNetworkListener;
@@ -677,7 +678,8 @@
}
@Override
- public void unsubscribePreCommit(String subject, Type eventType, String className) {
+ public void unsubscribePreCommit(String subject, Type eventType,
+ InstancePortAdminService service, String className) {
}
diff --git a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/PreCommitPortManagerTest.java b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/PreCommitPortManagerTest.java
index 3b32fc0..8575d7a 100644
--- a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/PreCommitPortManagerTest.java
+++ b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/PreCommitPortManagerTest.java
@@ -22,6 +22,7 @@
import org.onosproject.core.ApplicationId;
import org.onosproject.core.CoreServiceAdapter;
import org.onosproject.core.DefaultApplicationId;
+import org.onosproject.openstacknetworking.api.InstancePortAdminService;
import org.onosproject.store.service.TestStorageService;
import static org.junit.Assert.assertEquals;
@@ -89,8 +90,10 @@
sampleSubscribe();
- target.unsubscribePreCommit(PORT_ID_1, OPENSTACK_PORT_PRE_REMOVE, CLASS_NAME_1);
- target.unsubscribePreCommit(PORT_ID_2, OPENSTACK_PORT_PRE_REMOVE, CLASS_NAME_2);
+ InstancePortAdminService service = new TestInstancePortAdminService();
+
+ target.unsubscribePreCommit(PORT_ID_1, OPENSTACK_PORT_PRE_REMOVE, service, CLASS_NAME_1);
+ target.unsubscribePreCommit(PORT_ID_2, OPENSTACK_PORT_PRE_REMOVE, service, CLASS_NAME_2);
assertEquals(0, target.subscriberCountByEventType(PORT_ID_1, OPENSTACK_PORT_PRE_REMOVE));
assertEquals(1, target.subscriberCountByEventType(PORT_ID_2, OPENSTACK_PORT_PRE_REMOVE));
@@ -119,4 +122,7 @@
return TEST_APP_ID;
}
}
+
+ private static class TestInstancePortAdminService extends InstancePortAdminServiceAdapter {
+ }
}