Move EdgeService back to Link/Device services rather than TopologyService.
I think the EdgeService has to use one or the other, because the
TopologyService is not in sync with the Link/Device services. The problem
with using the TopologyService is that it does not handle Port events,
only Device and Link, so it is not suitable for building an inventory of
edge ports.
Change-Id: If31d6d7013985da3e03f8c83f2f2eb2957deffe1
diff --git a/cli/src/main/java/org/onosproject/cli/net/EdgePortsListCommand.java b/cli/src/main/java/org/onosproject/cli/net/EdgePortsListCommand.java
index 6a9a23b..2b03637 100644
--- a/cli/src/main/java/org/onosproject/cli/net/EdgePortsListCommand.java
+++ b/cli/src/main/java/org/onosproject/cli/net/EdgePortsListCommand.java
@@ -18,8 +18,14 @@
import org.apache.karaf.shell.commands.Argument;
import org.apache.karaf.shell.commands.Command;
import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.net.ConnectPoint;
import org.onosproject.net.edge.EdgePortService;
+import org.onosproject.utils.Comparators;
+import java.util.Collections;
+import java.util.List;
+
+import static com.google.common.collect.Lists.newArrayList;
import static org.onosproject.net.DeviceId.deviceId;
/**
@@ -35,16 +41,24 @@
required = false, multiValued = false)
String uri = null;
-
-
@Override
protected void execute() {
EdgePortService service = get(EdgePortService.class);
if (uri == null) {
- service.getEdgePoints().forEach(e -> print(FMT, e.deviceId(), e.port()));
+ printEdgePoints(service.getEdgePoints());
} else {
- service.getEdgePoints(deviceId(uri)).forEach(e -> print(FMT, e.deviceId(), e.port()));
+ printEdgePoints(service.getEdgePoints(deviceId(uri)));
}
}
+ private void printEdgePoints(Iterable<ConnectPoint> edgePoints) {
+ sort(edgePoints).forEach(e -> print(FMT, e.deviceId(), e.port()));
+ }
+
+ private static List<ConnectPoint> sort(Iterable<ConnectPoint> connectPoints) {
+ List<ConnectPoint> edgePoints = newArrayList(connectPoints);
+ Collections.sort(edgePoints, Comparators.CONNECT_POINT_COMPARATOR);
+ return edgePoints;
+ }
+
}
diff --git a/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml b/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
index 4fbae87..5664cc3 100644
--- a/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
+++ b/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
@@ -229,6 +229,7 @@
<action class="org.onosproject.cli.net.EdgePortsListCommand"/>
<completers>
<ref component-id="deviceIdCompleter"/>
+ <null/>
</completers>
</command>
diff --git a/core/net/src/main/java/org/onosproject/net/edgeservice/impl/EdgeManager.java b/core/net/src/main/java/org/onosproject/net/edgeservice/impl/EdgeManager.java
index c5d0928..68a58c2 100644
--- a/core/net/src/main/java/org/onosproject/net/edgeservice/impl/EdgeManager.java
+++ b/core/net/src/main/java/org/onosproject/net/edgeservice/impl/EdgeManager.java
@@ -31,6 +31,7 @@
import org.onosproject.net.DeviceId;
import org.onosproject.net.Link.Type;
import org.onosproject.net.device.DeviceEvent;
+import org.onosproject.net.device.DeviceListener;
import org.onosproject.net.device.DeviceService;
import org.onosproject.net.edge.EdgePortEvent;
import org.onosproject.net.edge.EdgePortListener;
@@ -38,25 +39,29 @@
import org.onosproject.net.flow.DefaultTrafficTreatment;
import org.onosproject.net.flow.TrafficTreatment;
import org.onosproject.net.link.LinkEvent;
+import org.onosproject.net.link.LinkListener;
+import org.onosproject.net.link.LinkService;
import org.onosproject.net.packet.DefaultOutboundPacket;
import org.onosproject.net.packet.OutboundPacket;
import org.onosproject.net.packet.PacketService;
-import org.onosproject.net.topology.Topology;
-import org.onosproject.net.topology.TopologyEvent;
-import org.onosproject.net.topology.TopologyListener;
-import org.onosproject.net.topology.TopologyService;
import org.slf4j.Logger;
import java.nio.ByteBuffer;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import static org.onosproject.net.device.DeviceEvent.Type.*;
+
+import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_ADDED;
+import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_AVAILABILITY_CHANGED;
+import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_REMOVED;
+import static org.onosproject.net.device.DeviceEvent.Type.PORT_STATS_UPDATED;
+import static org.onosproject.net.device.DeviceEvent.Type.PORT_UPDATED;
import static org.onosproject.net.edge.EdgePortEvent.Type.EDGE_PORT_ADDED;
import static org.onosproject.net.edge.EdgePortEvent.Type.EDGE_PORT_REMOVED;
-import static org.slf4j.LoggerFactory.getLogger;
import static org.onosproject.security.AppGuard.checkPermission;
-import static org.onosproject.security.AppPermission.Type.*;
+import static org.onosproject.security.AppPermission.Type.PACKET_WRITE;
+import static org.onosproject.security.AppPermission.Type.TOPOLOGY_READ;
+import static org.slf4j.LoggerFactory.getLogger;
/**
* This is an implementation of the edge net service.
@@ -69,14 +74,11 @@
private final Logger log = getLogger(getClass());
- private Topology topology;
-
- /**
- * Set of edge ConnectPoints per Device.
- */
+ // Set of edge ConnectPoints per Device.
private final Map<DeviceId, Set<ConnectPoint>> connectionPoints = Maps.newConcurrentMap();
- private final TopologyListener topologyListener = new InnerTopologyListener();
+ private final DeviceListener deviceListener = new InnerDeviceListener();
+ private final LinkListener linkListener = new InnerLinkListener();
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected PacketService packetService;
@@ -85,19 +87,21 @@
protected DeviceService deviceService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- protected TopologyService topologyService;
+ protected LinkService linkService;
@Activate
public void activate() {
eventDispatcher.addSink(EdgePortEvent.class, listenerRegistry);
- topologyService.addListener(topologyListener);
+ deviceService.addListener(deviceListener);
+ linkService.addListener(linkListener);
loadAllEdgePorts();
log.info("Started");
}
@Deactivate
public void deactivate() {
- topologyService.removeListener(topologyListener);
+ deviceService.removeListener(deviceListener);
+ linkService.removeListener(linkListener);
eventDispatcher.removeSink(EdgePortEvent.class);
log.info("Stopped");
}
@@ -105,7 +109,8 @@
@Override
public boolean isEdgePoint(ConnectPoint point) {
checkPermission(TOPOLOGY_READ);
- return !topologyService.isInfrastructure(topologyService.currentTopology(), point);
+ Set<ConnectPoint> connectPoints = connectionPoints.get(point.deviceId());
+ return connectPoints != null && connectPoints.contains(point);
}
@Override
@@ -148,26 +153,25 @@
return new DefaultOutboundPacket(point.deviceId(), builder.build(), data);
}
- private class InnerTopologyListener implements TopologyListener {
-
+ private class InnerLinkListener implements LinkListener {
@Override
- public void event(TopologyEvent event) {
- log.trace("Processing TopologyEvent {} caused by {}",
- event.subject(), event.reasons());
- topology = event.subject();
- event.reasons().forEach(reason -> {
- if (reason instanceof DeviceEvent) {
- processDeviceEvent((DeviceEvent) reason);
- } else if (reason instanceof LinkEvent) {
- processLinkEvent((LinkEvent) reason);
- }
- });
+ public void event(LinkEvent event) {
+ processLinkEvent(event);
+ }
+ }
+
+ private class InnerDeviceListener implements DeviceListener {
+ @Override
+ public void event(DeviceEvent event) {
+ if (event.type() == PORT_STATS_UPDATED) {
+ return;
+ }
+ processDeviceEvent(event);
}
}
// Initial loading of the edge port cache.
private void loadAllEdgePorts() {
- topology = topologyService.currentTopology();
deviceService.getAvailableDevices().forEach(d -> deviceService.getPorts(d.id())
.forEach(p -> addEdgePort(new ConnectPoint(d.id(), p.number()))));
}
@@ -177,8 +181,7 @@
// negative Link event can result in increase of edge ports
boolean addEdgePort = event.type() == LinkEvent.Type.LINK_REMOVED;
- // but if the Link is an Edge type,
- // it will be the opposite
+ // but if the Link is an Edge type, it will be the opposite
if (event.subject().type() == Type.EDGE) {
addEdgePort = !addEdgePort;
}
@@ -198,8 +201,6 @@
DeviceEvent.Type type = event.type();
DeviceId id = event.subject().id();
- // FIXME there's still chance that Topology and Device Service
- // view is out-of-sync
if (type == DEVICE_ADDED ||
type == DEVICE_AVAILABILITY_CHANGED && deviceService.isAvailable(id)) {
// When device is added or becomes available, add all its ports
@@ -223,8 +224,11 @@
}
private boolean isEdgePort(ConnectPoint point) {
- return !topologyService.isInfrastructure(topology, point) &&
- !point.port().isLogical();
+ // Logical ports are not counted as edge ports nor are infrastructure
+ // ports. Ports that have only edge links are considered edge ports.
+ return !point.port().isLogical() &&
+ linkService.getLinks(point).stream()
+ .allMatch(link -> link.type() == Type.EDGE);
}
// Adds the specified connection point to the edge points if needed.
diff --git a/core/net/src/test/java/org/onosproject/net/edgeservice/impl/EdgeManagerTest.java b/core/net/src/test/java/org/onosproject/net/edgeservice/impl/EdgeManagerTest.java
index 67c82e1..971ce4b 100644
--- a/core/net/src/test/java/org/onosproject/net/edgeservice/impl/EdgeManagerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/edgeservice/impl/EdgeManagerTest.java
@@ -19,7 +19,6 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
-
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -29,6 +28,7 @@
import org.onosproject.net.DefaultPort;
import org.onosproject.net.Device;
import org.onosproject.net.DeviceId;
+import org.onosproject.net.Link;
import org.onosproject.net.NetTestTools;
import org.onosproject.net.Port;
import org.onosproject.net.PortNumber;
@@ -38,26 +38,32 @@
import org.onosproject.net.edge.EdgePortEvent;
import org.onosproject.net.edge.EdgePortListener;
import org.onosproject.net.link.LinkEvent;
+import org.onosproject.net.link.LinkListener;
+import org.onosproject.net.link.LinkServiceAdapter;
import org.onosproject.net.packet.OutboundPacket;
import org.onosproject.net.packet.PacketServiceAdapter;
-import org.onosproject.net.topology.Topology;
import org.onosproject.net.topology.TopologyEvent;
import org.onosproject.net.topology.TopologyEvent.Type;
-import org.onosproject.net.topology.TopologyListener;
-import org.onosproject.net.topology.TopologyServiceAdapter;
import java.nio.ByteBuffer;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
import static org.onosproject.net.NetTestTools.injectEventDispatcher;
-import static org.onosproject.net.device.DeviceEvent.Type.*;
+import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_ADDED;
+import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_AVAILABILITY_CHANGED;
+import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_REMOVED;
import static org.onosproject.net.edge.EdgePortEvent.Type.EDGE_PORT_ADDED;
import static org.onosproject.net.edge.EdgePortEvent.Type.EDGE_PORT_REMOVED;
import static org.onosproject.net.link.LinkEvent.Type.LINK_ADDED;
@@ -78,23 +84,23 @@
private Set<OutboundPacket> packets = Sets.newConcurrentHashSet();
private final EdgePortListener testListener = new TestListener(events);
private TestDeviceManager testDeviceManager;
- private TestTopologyManager testTopologyManager;
+ private TestLinkService testLinkService;
@Before
public void setUp() {
mgr = new EdgeManager();
injectEventDispatcher(mgr, new TestEventDispatcher());
- testTopologyManager = new TestTopologyManager(infrastructurePorts);
- mgr.topologyService = testTopologyManager;
testDeviceManager = new TestDeviceManager(devices);
mgr.deviceService = testDeviceManager;
+
+ testLinkService = new TestLinkService();
+ mgr.linkService = testLinkService;
+
mgr.packetService = new TestPacketManager();
mgr.activate();
mgr.addListener(testListener);
-
}
-
@After
public void tearDown() {
mgr.removeListener(testListener);
@@ -110,13 +116,12 @@
assertEquals("Unexpected number of ports", numDevices * numPorts, infrastructurePorts.size());
- assertFalse("no ports expected", mgr.getEdgePoints().iterator().hasNext());
-
assertFalse("Expected isEdge to return false",
mgr.isEdgePoint(NetTestTools.connectPoint(Integer.toString(1), 1)));
removeInfraPort(NetTestTools.connectPoint(Integer.toString(1), 1));
- assertTrue("Expected isEdge to return false",
+ postTopologyEvent(new LinkEvent(LINK_REMOVED, NetTestTools.link(Integer.toString(1), 1, "b", 2)));
+ assertTrue("Expected isEdge to return true",
mgr.isEdgePoint(NetTestTools.connectPoint(Integer.toString(1), 1)));
}
@@ -190,17 +195,25 @@
int numDevices = 10;
int numInfraPorts = 5;
totalPorts = 10;
+
defaultPopulator(numDevices, numInfraPorts);
+ events.clear();
+
//Test response to device added events
- referenceDevice = NetTestTools.device("1");
+
+ referenceDevice = NetTestTools.device("11");
+ devices.put(referenceDevice.id(), referenceDevice);
+ for (int port = 1; port <= numInfraPorts; port++) {
+ infrastructurePorts.add(NetTestTools.connectPoint("11", port));
+ }
event = new DeviceEvent(DEVICE_ADDED, referenceDevice,
new DefaultPort(referenceDevice, PortNumber.portNumber(1), true));
postTopologyEvent(event);
//Check that ports were populated correctly
assertTrue("Unexpected number of new ports added",
- mgr.deviceService.getPorts(NetTestTools.did("1")).size() == 10);
+ mgr.deviceService.getPorts(NetTestTools.did("11")).size() == 10);
//Check that of the ten ports the half that are infrastructure ports aren't added
assertEquals("Unexpected number of new edge ports added", (totalPorts - numInfraPorts), events.size());
@@ -227,7 +240,7 @@
for (; pointIterator.hasNext(); count++) {
pointIterator.next();
}
- assertEquals("Unexpected number of edge points", totalPorts - numInfraPorts, count);
+ assertEquals("Unexpected number of edge points", (numDevices + 1) * numInfraPorts, count);
//Testing device removal
events.clear();
event = (new DeviceEvent(DEVICE_REMOVED, referenceDevice,
@@ -271,10 +284,11 @@
//Ensure that the deviceManager shows the device as unavailable
removeDevice(referenceDevice);
- /*This variable copies the behavior of the topology by returning ports attached to an unavailable device
- //this behavior is necessary for the following event to execute properly, if these statements are removed
- no events will be generated since no ports will be provided in getPorts() to EdgeManager.
- */
+ // This variable copies the behavior of the topology by returning ports
+ // attached to an unavailable device this behavior is necessary for the
+ // following event to execute properly, if these statements are removed
+ // no events will be generated since no ports will be provided in
+ // getPorts() to EdgeManager.
alwaysReturnPorts = true;
postTopologyEvent(event);
alwaysReturnPorts = false;
@@ -330,7 +344,6 @@
}
}
-
@Test
public void testEmit() {
byte[] arr = new byte[10];
@@ -345,7 +358,7 @@
}
for (int i = 0; i < numDevices; i++) {
referenceDevice = NetTestTools.device(Integer.toString(i));
- postTopologyEvent(new DeviceEvent(DEVICE_ADDED, referenceDevice,
+ testDeviceManager.listener.event(new DeviceEvent(DEVICE_ADDED, referenceDevice,
new DefaultPort(referenceDevice,
PortNumber.portNumber(1),
true)));
@@ -392,7 +405,13 @@
* @param event Event
*/
private void postTopologyEvent(Event event) {
- testTopologyManager.listener.event(topologyEventOf(event));
+ if (event instanceof DeviceEvent) {
+ testDeviceManager.listener.event((DeviceEvent) event);
+ }
+ if (event instanceof LinkEvent) {
+ testLinkService.listener.event((LinkEvent) event);
+ }
+ //testTopologyManager.listener.event(topologyEventOf(event));
}
@@ -406,7 +425,9 @@
String str = Integer.toString(device);
Device deviceToAdd = NetTestTools.device(str);
devices.put(deviceToAdd.id(), deviceToAdd);
+ testDeviceManager.listener.event(new DeviceEvent(DEVICE_ADDED, deviceToAdd));
for (int port = 1; port <= numInfraPorts; port++) {
+ testLinkService.listener.event(new LinkEvent(LINK_ADDED, NetTestTools.link(str, port, "other", 1)));
infrastructurePorts.add(NetTestTools.connectPoint(str, port));
}
}
@@ -436,30 +457,6 @@
infrastructurePorts.remove(port);
}
- private class TestTopologyManager extends TopologyServiceAdapter {
- private TopologyListener listener;
- private Set<ConnectPoint> infrastructurePorts;
-
- public TestTopologyManager(Set<ConnectPoint> infrastructurePorts) {
- this.infrastructurePorts = infrastructurePorts;
- }
-
- @Override
- public boolean isInfrastructure(Topology topology, ConnectPoint connectPoint) {
- return infrastructurePorts.contains(connectPoint);
- }
-
- @Override
- public void addListener(TopologyListener listener) {
- this.listener = listener;
- }
-
- @Override
- public void removeListener(TopologyListener listener) {
- this.listener = null;
- }
- }
-
private class TestDeviceManager extends DeviceServiceAdapter {
private DeviceListener listener;
@@ -510,6 +507,25 @@
}
}
+ private class TestLinkService extends LinkServiceAdapter {
+
+ private LinkListener listener;
+
+ @Override
+ public Set<Link> getLinks(ConnectPoint connectPoint) {
+ if (infrastructurePorts.contains(connectPoint)) {
+ return Collections.singleton(NetTestTools.link("1", 1, "2", 1));
+ } else {
+ return Collections.emptySet();
+ }
+ }
+
+ @Override
+ public void addListener(LinkListener listener) {
+ this.listener = listener;
+ }
+ }
+
private class TestPacketManager extends PacketServiceAdapter {
@Override
public void emit(OutboundPacket packet) {