Allow reading PortDescription from DeviceStore
DeviceManager had to reverse-engineer PortDescription
in order to mark port off-line, etc.
which required domain specific knowledge.
(e.g., OpticalPortOperatorr#descriptionOf)
required work for ONOS-4691
Change-Id: I954f5f2db2cb7db938f498ead4c8e3f84212a53f
diff --git a/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java b/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
index f3bc222..cfc9626 100644
--- a/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
+++ b/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
@@ -24,6 +24,7 @@
import java.util.Collection;
import java.util.List;
+import java.util.stream.Stream;
/**
* Manages inventory of infrastructure devices; not intended for direct use.
@@ -115,6 +116,15 @@
List<Port> getPorts(DeviceId deviceId);
/**
+ * Returns the stream of port descriptions that belong to the specified device.
+ *
+ * @param providerId provider identifier
+ * @param deviceId device identifier
+ * @return stream of device portdescriptions
+ */
+ Stream<PortDescription> getPortDescriptions(ProviderId providerId, DeviceId deviceId);
+
+ /**
* Updates the port statistics of the specified device using the give port
* statistics.
*
@@ -152,6 +162,16 @@
Port getPort(DeviceId deviceId, PortNumber portNumber);
/**
+ * Returns the specified device port description.
+ *
+ * @param providerId provider identifier
+ * @param deviceId device identifier
+ * @param portNumber port number
+ * @return device port description
+ */
+ PortDescription getPortDescription(ProviderId providerId, DeviceId deviceId, PortNumber portNumber);
+
+ /**
* Indicates whether the specified device is available/online.
*
* @param deviceId device identifier
diff --git a/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java b/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
index 9208ee0..d13da6a 100644
--- a/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
+++ b/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
@@ -24,6 +24,7 @@
import java.util.Collection;
import java.util.List;
+import java.util.stream.Stream;
/**
* Test adapter for the DeviceStore API.
@@ -78,6 +79,12 @@
}
@Override
+ public Stream<PortDescription> getPortDescriptions(ProviderId providerId,
+ DeviceId deviceId) {
+ return Stream.empty();
+ }
+
+ @Override
public DeviceEvent updatePortStatistics(ProviderId providerId, DeviceId deviceId,
Collection<PortStatistics> portStats) {
return null;
@@ -99,6 +106,13 @@
}
@Override
+ public PortDescription getPortDescription(ProviderId providerId,
+ DeviceId deviceId,
+ PortNumber portNumber) {
+ return null;
+ }
+
+ @Override
public boolean isAvailable(DeviceId deviceId) {
return false;
}
diff --git a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
index 4b23e21..31b4292 100644
--- a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
+++ b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
@@ -59,11 +59,13 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -416,6 +418,15 @@
}
@Override
+ public Stream<PortDescription> getPortDescriptions(ProviderId providerId,
+ DeviceId deviceId) {
+ return Optional.ofNullable(deviceDescs.get(deviceId))
+ .map(m -> m.get(providerId))
+ .map(descs -> descs.portDescs.values().stream())
+ .orElse(Stream.empty());
+ }
+
+ @Override
public DeviceEvent updatePortStatistics(ProviderId providerId, DeviceId deviceId,
Collection<PortStatistics> newStatsCollection) {
@@ -480,6 +491,16 @@
}
@Override
+ public PortDescription getPortDescription(ProviderId providerId,
+ DeviceId deviceId,
+ PortNumber portNumber) {
+ return Optional.ofNullable(deviceDescs.get(deviceId))
+ .map(m -> m.get(providerId))
+ .map(descs -> descs.getPortDesc(portNumber))
+ .orElse(null);
+ }
+
+ @Override
public List<PortStatistics> getPortStatistics(DeviceId deviceId) {
Map<PortNumber, PortStatistics> portStats = devicePortStats.get(deviceId);
if (portStats == null) {
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
index 3c9b71d..2bf4ee8 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
@@ -86,6 +86,7 @@
import org.onosproject.net.optical.device.OtuPortHelper;
import org.onosproject.net.provider.AbstractListenerProviderRegistry;
import org.onosproject.net.provider.AbstractProviderService;
+import org.onosproject.net.provider.Provider;
import org.slf4j.Logger;
import com.google.common.util.concurrent.Futures;
@@ -367,6 +368,17 @@
}
+ private PortDescription ensurePortEnabledState(PortDescription desc, boolean enabled) {
+ if (desc.isEnabled() != enabled) {
+ return new DefaultPortDescription(desc.portNumber(),
+ enabled,
+ desc.type(),
+ desc.portSpeed(),
+ desc.annotations());
+ }
+ return desc;
+ }
+
@Override
public void deviceDisconnected(DeviceId deviceId) {
checkNotNull(deviceId, DEVICE_ID_NULL);
@@ -374,17 +386,9 @@
log.info("Device {} disconnected from this node", deviceId);
- List<Port> ports = store.getPorts(deviceId);
- final Device device = getDevice(deviceId);
-
- List<PortDescription> descs = ports.stream().map(
- port -> (!(Device.Type.ROADM.equals(device.type()) ||
- (Device.Type.OTN.equals(device.type())) ||
- (Device.Type.FIBER_SWITCH.equals(device.type())))) ?
- new DefaultPortDescription(port.number(), false,
- port.type(), port.portSpeed()) :
- OpticalPortOperator.descriptionOf(port, false)
- ).collect(Collectors.toList());
+ List<PortDescription> descs = store.getPortDescriptions(provider().id(), deviceId)
+ .map(desc -> ensurePortEnabledState(desc, false))
+ .collect(Collectors.toList());
store.updatePorts(this.provider().id(), deviceId, descs);
try {
@@ -524,9 +528,12 @@
Device device = nullIsNotFound(getDevice(deviceId), "Device not found");
if ((Device.Type.ROADM.equals(device.type())) ||
(Device.Type.OTN.equals(device.type()))) {
- Port port = getPort(deviceId, portDescription.portNumber());
// FIXME This is ignoring all other info in portDescription given as input??
- portDescription = OpticalPortOperator.descriptionOf(port, portDescription.isEnabled());
+ PortDescription storedPortDesc = store.getPortDescription(provider().id(),
+ deviceId,
+ portDescription.portNumber());
+ portDescription = ensurePortEnabledState(storedPortDesc,
+ portDescription.isEnabled());
}
portDescription = consolidate(deviceId, portDescription);
@@ -847,13 +854,14 @@
if (event.configClass().equals(OpticalPortConfig.class)) {
ConnectPoint cpt = (ConnectPoint) event.subject();
DeviceId did = cpt.deviceId();
+ Provider provider = getProvider(did);
Port dpt = getPort(did, cpt.port());
- if (dpt != null) {
+ if (dpt != null && provider != null) {
OpticalPortConfig opc = networkConfigService.getConfig(cpt, OpticalPortConfig.class);
- PortDescription desc = OpticalPortOperator.descriptionOf(dpt);
+ PortDescription desc = store.getPortDescription(provider.id(), did, cpt.port());
desc = OpticalPortOperator.combine(opc, desc);
- if (desc != null && getProvider(did) != null) {
+ if (desc != null) {
de = store.updatePortStatus(getProvider(did).id(), did, desc);
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java b/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
index bd56751..b07780b 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
@@ -20,13 +20,10 @@
import static org.onosproject.net.optical.device.OmsPortHelper.omsPortDescription;
import static org.onosproject.net.optical.device.OtuPortHelper.otuPortDescription;
import static org.slf4j.LoggerFactory.getLogger;
-import static com.google.common.base.Preconditions.checkNotNull;
-
import org.onosproject.net.config.ConfigOperator;
import org.onosproject.net.config.basics.OpticalPortConfig;
import org.onosproject.net.AnnotationKeys;
import org.onosproject.net.DefaultAnnotations;
-import org.onosproject.net.Port;
import org.onosproject.net.PortNumber;
import org.onosproject.net.SparseAnnotations;
import org.onosproject.net.device.DefaultPortDescription;
@@ -35,11 +32,6 @@
import org.onosproject.net.device.OmsPortDescription;
import org.onosproject.net.device.OtuPortDescription;
import org.onosproject.net.device.PortDescription;
-import org.onosproject.net.optical.OchPort;
-import org.onosproject.net.optical.OduCltPort;
-import org.onosproject.net.optical.OmsPort;
-import org.onosproject.net.optical.OpticalDevice;
-import org.onosproject.net.optical.OtuPort;
import org.slf4j.Logger;
/**
@@ -184,98 +176,4 @@
return DefaultAnnotations.union(an, b.build());
}
- /**
- * Returns a description built from an existing port.
- *
- * @param port the device port
- * @return a PortDescription based on the port
- */
- public static PortDescription descriptionOf(Port port) {
- checkNotNull(port, "Must supply non-null Port");
- final boolean isUp = port.isEnabled();
- return descriptionOfPort(port, isUp);
- }
-
- /**
- * Returns a description built from an existing port and reported status.
- *
- * @param port port
- * @param isEnabled true if enabled
- * @return a PortDescription based on the port
- */
- static PortDescription descriptionOf(Port port, boolean isEnabled) {
- checkNotNull(port, "Must supply non-null Port");
- final boolean isup = isEnabled;
- return descriptionOfPort(port, isup);
- }
-
- private static PortDescription descriptionOfPort(Port port, final boolean isup) {
- final PortNumber ptn = port.number();
- final SparseAnnotations an = (SparseAnnotations) port.annotations();
- switch (port.type()) {
- case OMS:
- if (port instanceof org.onosproject.net.OmsPort) {
- // remove if-block once deprecation is complete
- org.onosproject.net.OmsPort oms = (org.onosproject.net.OmsPort) port;
- return omsPortDescription(ptn, isup, oms.minFrequency(),
- oms.maxFrequency(), oms.grid(), an);
- }
- if (port.element().is(OpticalDevice.class)) {
- OpticalDevice optDevice = port.element().as(OpticalDevice.class);
- if (optDevice.portIs(port, OmsPort.class)) {
- OmsPort oms = optDevice.portAs(port, OmsPort.class).get();
- return omsPortDescription(ptn, isup, oms.minFrequency(),
- oms.maxFrequency(), oms.grid(), an);
- }
- }
- return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
- case OCH:
- if (port instanceof org.onosproject.net.OchPort) {
- // remove if-block once old OchPort deprecation is complete
- org.onosproject.net.OchPort och = (org.onosproject.net.OchPort) port;
- return ochPortDescription(ptn, isup, och.signalType(),
- och.isTunable(), och.lambda(), an);
- }
- if (port.element().is(OpticalDevice.class)) {
- OpticalDevice optDevice = port.element().as(OpticalDevice.class);
- if (optDevice.portIs(port, OchPort.class)) {
- OchPort och = optDevice.portAs(port, OchPort.class).get();
- return ochPortDescription(ptn, isup, och.signalType(),
- och.isTunable(), och.lambda(), an);
- }
- }
- return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
-
- case ODUCLT:
- if (port instanceof org.onosproject.net.OduCltPort) {
- // remove if-block once deprecation is complete
- org.onosproject.net.OduCltPort odu = (org.onosproject.net.OduCltPort) port;
- return oduCltPortDescription(ptn, isup, odu.signalType(), an);
- }
- if (port.element().is(OpticalDevice.class)) {
- OpticalDevice optDevice = port.element().as(OpticalDevice.class);
- if (optDevice.portIs(port, OduCltPort.class)) {
- OduCltPort odu = (OduCltPort) port;
- return oduCltPortDescription(ptn, isup, odu.signalType(), an);
- }
- }
- return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
- case OTU:
- if (port instanceof org.onosproject.net.OtuPort) {
- // remove if-block once deprecation is complete
- org.onosproject.net.OtuPort otu = (org.onosproject.net.OtuPort) port;
- return otuPortDescription(ptn, isup, otu.signalType(), an);
- }
- if (port.element().is(OpticalDevice.class)) {
- OpticalDevice optDevice = port.element().as(OpticalDevice.class);
- if (optDevice.portIs(port, OtuPort.class)) {
- OtuPort otu = (OtuPort) port;
- return otuPortDescription(ptn, isup, otu.signalType(), an);
- }
- }
- return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
- default:
- return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
- }
- }
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
index a407d87..de4123b 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
@@ -87,6 +87,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
+import java.util.stream.Stream;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Verify.verify;
@@ -575,11 +576,27 @@
}
@Override
+ public Stream<PortDescription> getPortDescriptions(ProviderId pid,
+ DeviceId deviceId) {
+
+ return portDescriptions.entrySet().stream()
+ .filter(e -> e.getKey().providerId().equals(pid))
+ .map(Map.Entry::getValue);
+ }
+
+ @Override
public Port getPort(DeviceId deviceId, PortNumber portNumber) {
return devicePorts.getOrDefault(deviceId, Maps.newHashMap()).get(portNumber);
}
@Override
+ public PortDescription getPortDescription(ProviderId pid,
+ DeviceId deviceId,
+ PortNumber portNumber) {
+ return portDescriptions.get(new PortKey(pid, deviceId, portNumber));
+ }
+
+ @Override
public DeviceEvent updatePortStatistics(ProviderId providerId,
DeviceId deviceId,
Collection<PortStatistics> newStatsCollection) {
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
index d2d450e..acc8ff4 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
@@ -93,11 +93,13 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Predicates.notNull;
@@ -835,6 +837,25 @@
}
@Override
+ public Stream<PortDescription> getPortDescriptions(ProviderId pid,
+ DeviceId deviceId) {
+ Map<ProviderId, DeviceDescriptions> descs = this.deviceDescs.get(deviceId);
+ if (descs == null) {
+ return null;
+ }
+ // inner-Map(=descs) is HashMap, thus requires synchronization even for reads
+ final Optional<DeviceDescriptions> devDescs;
+ synchronized (descs) {
+ devDescs = Optional.ofNullable(descs.get(pid));
+ }
+ // DeviceDescriptions is concurrent access-safe
+ return devDescs
+ .map(dd -> dd.getPortDescs().values().stream()
+ .map(Timestamped::value))
+ .orElse(Stream.empty());
+ }
+
+ @Override
public DeviceEvent updatePortStatistics(ProviderId providerId, DeviceId deviceId,
Collection<PortStatistics> newStatsCollection) {
@@ -926,6 +947,26 @@
}
@Override
+ public PortDescription getPortDescription(ProviderId pid,
+ DeviceId deviceId,
+ PortNumber portNumber) {
+ Map<ProviderId, DeviceDescriptions> descs = this.deviceDescs.get(deviceId);
+ if (descs == null) {
+ return null;
+ }
+ // inner-Map(=descs) is HashMap, thus requires synchronization even for reads
+ final Optional<DeviceDescriptions> devDescs;
+ synchronized (descs) {
+ devDescs = Optional.ofNullable(descs.get(pid));
+ }
+ // DeviceDescriptions is concurrent access-safe
+ return devDescs
+ .map(deviceDescriptions -> deviceDescriptions.getPortDesc(portNumber))
+ .map(Timestamped::value)
+ .orElse(null);
+ }
+
+ @Override
public boolean isAvailable(DeviceId deviceId) {
return availableDevices.contains(deviceId);
}