Refactor: remove code duplication for openstack related apps
Change-Id: I62867b7b41e3271d1272c2eb09d0fd25a6d03074
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 0cc63b9..63b4a49 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
@@ -29,7 +29,6 @@
import org.onosproject.openstacknetworking.api.OpenstackNetworkEvent;
import org.onosproject.openstacknetworking.api.OpenstackNetworkStore;
import org.onosproject.openstacknetworking.api.OpenstackNetworkStoreDelegate;
-import org.onosproject.openstacknetworking.api.PreCommitPortService;
import org.onosproject.store.AbstractStore;
import org.onosproject.store.serializers.KryoNamespaces;
import org.onosproject.store.service.ConsistentMap;
@@ -58,6 +57,7 @@
import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ExecutorService;
@@ -93,8 +93,6 @@
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)
@@ -122,9 +120,6 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected StorageService storageService;
- @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- protected PreCommitPortService preCommitPortService;
-
private final ExecutorService eventExecutor = newSingleThreadExecutor(
groupedThreads(this.getClass().getSimpleName(), "event-handler", log));
@@ -413,7 +408,7 @@
ImmutableList.of() : newPort.getSecurityGroups();
oldSecurityGroups.stream()
- .filter(sgId -> !newPort.getSecurityGroups().contains(sgId))
+ .filter(sgId -> !Objects.requireNonNull(newPort.getSecurityGroups()).contains(sgId))
.forEach(sgId -> notifyDelegate(new OpenstackNetworkEvent(
OPENSTACK_PORT_SECURITY_GROUP_REMOVED, newPort, sgId
)));
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenStackSwitchingDirectPortProvider.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenStackSwitchingDirectPortProvider.java
index 76c0197..14e6aa1 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenStackSwitchingDirectPortProvider.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenStackSwitchingDirectPortProvider.java
@@ -311,9 +311,6 @@
ports.forEach(port -> {
addIntfToDevice(node, port);
});
-
-
-
}
private void addIntfToDevice(OpenstackNode node, Port port) {
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManager.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManager.java
index 92257fd..1db1da1 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManager.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManager.java
@@ -339,7 +339,7 @@
osPort = osNetworkStore.ports()
.stream()
.filter(p -> p.getvNicType().equals(DIRECT) && p.getProfile().get(PCISLOT) != null)
- .filter(p -> getIntfNameFromPciAddress(p).equals(portName))
+ .filter(p -> Objects.requireNonNull(getIntfNameFromPciAddress(p)).equals(portName))
.findFirst();
return osPort.orElse(null);
@@ -484,11 +484,11 @@
ByteBuffer.wrap(ethRequest.serialize())));
externalPeerRouterMap.put(targetIp.toString(),
- DefaultExternalPeerRouter.builder()
- .ipAddress(targetIp)
- .macAddress(MacAddress.NONE)
- .vlanId(vlanId)
- .build());
+ DefaultExternalPeerRouter.builder()
+ .ipAddress(targetIp)
+ .macAddress(MacAddress.NONE)
+ .vlanId(vlanId)
+ .build());
log.info("Initializes external peer router map with peer router IP {}", targetIp.toString());
}
@@ -641,7 +641,7 @@
return subnets(network.getId()).stream()
.filter(s -> IpPrefix.valueOf(s.getCidr()).contains(ipAddress))
- .map(s -> s.getGateway())
+ .map(Subnet::getGateway)
.findAny().orElse(null);
}
@@ -704,4 +704,4 @@
return externalSubnet.map(subnet -> IpAddress.valueOf(subnet.getGateway())).orElse(null);
}
-}
+}
\ No newline at end of file
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java
index 764cefe..a7e008a 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java
@@ -221,7 +221,6 @@
});
} catch (IllegalArgumentException e) {
log.warn("IllegalArgumentException occurred because of {}", e.toString());
- return;
}
}
}
diff --git a/apps/openstacknetworkingui/src/main/java/org/onosproject/openstacknetworkingui/OpenstackNetworkingUiMessageHandler.java b/apps/openstacknetworkingui/src/main/java/org/onosproject/openstacknetworkingui/OpenstackNetworkingUiMessageHandler.java
index 3982d22..1dfefb8 100644
--- a/apps/openstacknetworkingui/src/main/java/org/onosproject/openstacknetworkingui/OpenstackNetworkingUiMessageHandler.java
+++ b/apps/openstacknetworkingui/src/main/java/org/onosproject/openstacknetworkingui/OpenstackNetworkingUiMessageHandler.java
@@ -168,7 +168,7 @@
switch (mode) {
case MOUSE:
currentMode = Mode.MOUSE;
- eventExecutor.execute(() -> sendMouseData());
+ eventExecutor.execute(OpenstackNetworkingUiMessageHandler.this::sendMouseData);
break;
@@ -195,7 +195,7 @@
dstIp,
srcDeviceId,
dstDeviceId);
- eventExecutor.execute(() -> processFlowTraceRequest(srcIp, dstIp, srcDeviceId, dstDeviceId));
+ eventExecutor.execute(() -> processFlowTraceRequest(srcIp, dstIp, srcDeviceId));
}
}
@@ -211,7 +211,7 @@
if (!Strings.isNullOrEmpty(id)) {
eventExecutor.execute(() -> updateForMode(id));
} else {
- eventExecutor.execute(() -> clearForMode());
+ eventExecutor.execute(OpenstackNetworkingUiMessageHandler.this::clearForMode);
}
}
}
@@ -229,8 +229,6 @@
}
}
- // === ------------
-
private void clearState() {
currentMode = Mode.IDLE;
elementOfNote = null;
@@ -290,14 +288,13 @@
private void sendMouseData() {
Highlights highlights = new Highlights();
- if (elementOfNote != null && elementOfNote instanceof Device) {
+ if (elementOfNote instanceof Device) {
DeviceId deviceId = (DeviceId) elementOfNote.id();
List<OpenstackLink> edgeLinks = edgeLinks(deviceId);
- edgeLinks.forEach(edgeLink -> {
- highlights.add(edgeLink.highlight(OpenstackLink.RequestType.DEVICE_SELECTED));
- });
+ edgeLinks.forEach(edgeLink ->
+ highlights.add(edgeLink.highlight(OpenstackLink.RequestType.DEVICE_SELECTED)));
hostService.getConnectedHosts(deviceId).forEach(host -> {
HostHighlight hostHighlight = new HostHighlight(host.id().toString());
@@ -307,7 +304,7 @@
sendHighlights(highlights);
- } else if (elementOfNote != null && elementOfNote instanceof Host) {
+ } else if (elementOfNote instanceof Host) {
HostId hostId = HostId.hostId(elementOfNote.id().toString());
if (!hostMadeFromOpenstack(hostId)) {
@@ -316,9 +313,8 @@
List<OpenstackLink> openstackLinks = linksInSameNetwork(hostId);
- openstackLinks.forEach(openstackLink -> {
- highlights.add(openstackLink.highlight(OpenstackLink.RequestType.HOST_SELECTED));
- });
+ openstackLinks.forEach(openstackLink ->
+ highlights.add(openstackLink.highlight(OpenstackLink.RequestType.HOST_SELECTED)));
hostHighlightsInSameNetwork(hostId).forEach(highlights::add);
@@ -328,8 +324,7 @@
}
private boolean hostMadeFromOpenstack(HostId hostId) {
- return hostService.getHost(hostId).annotations()
- .value(ANNOTATION_NETWORK_ID) == null ? false : true;
+ return hostService.getHost(hostId).annotations().value(ANNOTATION_NETWORK_ID) != null;
}
private String networkId(HostId hostId) {
@@ -360,7 +355,7 @@
List<OpenstackLink> edgeLinks = Lists.newArrayList();
- openstackLinkMap.biLinks().forEach(edgeLinks::add);
+ edgeLinks.addAll(openstackLinkMap.biLinks());
return edgeLinks;
}
@@ -384,7 +379,7 @@
List<OpenstackLink> openstackLinks = Lists.newArrayList();
- linkMap.biLinks().forEach(openstackLinks::add);
+ openstackLinks.addAll(linkMap.biLinks());
return openstackLinks;
}
@@ -398,7 +393,7 @@
return NodeBadge.number(Status.INFO, n, "Openstack Node");
}
- private void processFlowTraceRequest(String srcIp, String dstIp, String srcDeviceId, String dstDeviceId) {
+ private void processFlowTraceRequest(String srcIp, String dstIp, String srcDeviceId) {
boolean traceSuccess = true;
ObjectMapper mapper = new ObjectMapper();
diff --git a/apps/openstacknode/app/src/main/java/org/onosproject/openstacknode/impl/OpenstackNodeManager.java b/apps/openstacknode/app/src/main/java/org/onosproject/openstacknode/impl/OpenstackNodeManager.java
index b6a4437..ff10a2c 100644
--- a/apps/openstacknode/app/src/main/java/org/onosproject/openstacknode/impl/OpenstackNodeManager.java
+++ b/apps/openstacknode/app/src/main/java/org/onosproject/openstacknode/impl/OpenstackNodeManager.java
@@ -264,16 +264,7 @@
public void addVfPort(OpenstackNode osNode, String portName) {
log.trace("addVfPort called");
- if (!isOvsdbConnected(osNode, ovsdbPort, ovsdbController, deviceService)) {
- log.warn("There's no ovsdb connection with the device {}. Try to connect the device...",
- osNode.ovsdb().toString());
- try {
- ovsdbController.connect(osNode.managementIp(), tpPort(ovsdbPort));
- } catch (Exception e) {
- log.error("Failed to connect to the openstackNode via ovsdb protocol because of exception {}",
- e.toString());
- }
- }
+ connectSwitch(osNode);
addOrRemoveSystemInterface(osNode, INTEGRATION_BRIDGE, portName, true);
}
@@ -282,6 +273,12 @@
public void removeVfPort(OpenstackNode osNode, String portName) {
log.trace("removeVfPort called");
+ connectSwitch(osNode);
+
+ addOrRemoveSystemInterface(osNode, INTEGRATION_BRIDGE, portName, false);
+ }
+
+ private void connectSwitch(OpenstackNode osNode) {
if (!isOvsdbConnected(osNode, ovsdbPort, ovsdbController, deviceService)) {
log.warn("There's no ovsdb connection with the device {}. Try to connect the device...",
osNode.ovsdb().toString());
@@ -292,8 +289,6 @@
e.toString());
}
}
-
- addOrRemoveSystemInterface(osNode, INTEGRATION_BRIDGE, portName, false);
}
private void addOrRemoveSystemInterface(OpenstackNode osNode, String bridgeName, String intfName,
diff --git a/apps/openstacktroubleshoot/app/src/main/java/org/onosproject/openstacktroubleshoot/impl/OpenstackTroubleshootManager.java b/apps/openstacktroubleshoot/app/src/main/java/org/onosproject/openstacktroubleshoot/impl/OpenstackTroubleshootManager.java
index 13e9579..1b1cdf8 100644
--- a/apps/openstacktroubleshoot/app/src/main/java/org/onosproject/openstacktroubleshoot/impl/OpenstackTroubleshootManager.java
+++ b/apps/openstacktroubleshoot/app/src/main/java/org/onosproject/openstacktroubleshoot/impl/OpenstackTroubleshootManager.java
@@ -44,6 +44,7 @@
import org.onosproject.net.flow.FlowRuleService;
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
+import org.onosproject.net.flow.criteria.Criterion;
import org.onosproject.net.flow.criteria.IPCriterion;
import org.onosproject.net.packet.DefaultOutboundPacket;
import org.onosproject.net.packet.InboundPacket;
@@ -290,45 +291,21 @@
/**
* Checks whether east-west ICMP reply rule is added or not.
*
- * @param ip IP address
+ * @param dstIp IP address
* @return true if ICMP reply rule is added, false otherwise
*/
- private boolean checkEastWestIcmpReplyRule(String ip) {
- for (FlowEntry entry : flowRuleService.getFlowEntriesById(appId)) {
- TrafficSelector selector = entry.selector();
-
- IPCriterion ipCriterion = (IPCriterion) selector.getCriterion(IPV4_DST);
-
- if (ipCriterion != null &&
- ip.equals(ipCriterion.ip().address().toString()) &&
- entry.state() == ADDED) {
- return true;
- }
- }
-
- return false;
+ private boolean checkEastWestIcmpReplyRule(String dstIp) {
+ return checkFlowRule(dstIp, IPV4_DST);
}
/**
* Checks whether north-south ICMP reply rule is added or not.
*
- * @param ip IP address
+ * @param srcIp IP address
* @return true if ICMP reply rule is added, false otherwise
*/
- private boolean checkNorthSouthIcmpReplyRule(String ip) {
- for (FlowEntry entry : flowRuleService.getFlowEntriesById(appId)) {
- TrafficSelector selector = entry.selector();
-
- IPCriterion ipCriterion = (IPCriterion) selector.getCriterion(IPV4_SRC);
-
- if (ipCriterion != null &&
- ip.equals(ipCriterion.ip().address().toString()) &&
- entry.state() == ADDED) {
- return true;
- }
- }
-
- return false;
+ private boolean checkNorthSouthIcmpReplyRule(String srcIp) {
+ return checkFlowRule(srcIp, IPV4_SRC);
}
/**
@@ -338,18 +315,28 @@
* @return true if the rule is added, false otherwise
*/
private boolean checkVidTagRule(String srcIp) {
+ return checkFlowRule(srcIp, IPV4_SRC);
+ }
+
+ /**
+ * Checks whether flow rules with the given IP and criterion are added or not.
+ *
+ * @param ip IP address
+ * @param ipType IP criterion type (IPV4_DST or IPV4_SRC)
+ * @return true if the flow rule is added, false otherwise
+ */
+ private boolean checkFlowRule(String ip, Criterion.Type ipType) {
for (FlowEntry entry : flowRuleService.getFlowEntriesById(appId)) {
TrafficSelector selector = entry.selector();
- IPCriterion srcIpCriterion = (IPCriterion) selector.getCriterion(IPV4_SRC);
+ IPCriterion ipCriterion = (IPCriterion) selector.getCriterion(ipType);
- if (srcIpCriterion != null &&
- srcIp.equals(srcIpCriterion.ip().address().toString()) &&
+ if (ipCriterion != null &&
+ ip.equals(ipCriterion.ip().address().toString()) &&
entry.state() == ADDED) {
return true;
}
}
-
return false;
}