From 6c71a0500a9f28d73bf9aa4b52db4cb4166e34fb Mon Sep 17 00:00:00 2001 From: Andrea Campanella Date: Fri, 22 Apr 2016 11:56:31 -0700 Subject: [PATCH] Deprecating PortDiscovery in favour of DeviceDescritpionDiscovery Change-Id: Ie9cff7937412c62c8a5a3b75b87a43952017f146 --- .../net/behaviour/PortDiscovery.java | 6 ++++ .../device/DeviceDescriptionDiscovery.java | 4 +-- .../common/net/AbstractDeviceProvider.java | 10 ++++-- ... => CienaWaveserverDeviceDescription.java} | 32 ++++++++++++++----- .../src/main/resources/ciena-drivers.xml | 2 ++ ...java => FujitsuT100DeviceDescription.java} | 24 ++++++++++---- .../src/main/resources/fujitsu-drivers.xml | 2 ++ .../LumentumRoadmDeviceDescription.java | 5 +-- .../device/impl/NetconfDeviceProvider.java | 10 +++++- .../rest/device/impl/RestDeviceProvider.java | 32 ++++++++++++------- .../snmp/device/impl/SnmpDeviceProvider.java | 10 ++++-- 11 files changed, 100 insertions(+), 37 deletions(-) rename drivers/ciena/src/main/java/org/onosproject/drivers/ciena/{PortDiscoveryCienaWaveserverImpl.java => CienaWaveserverDeviceDescription.java} (88%) rename drivers/fujitsu/src/main/java/org/onosproject/drivers/fujitsu/{PortGetterFujitsuImpl.java => FujitsuT100DeviceDescription.java} (91%) diff --git a/core/api/src/main/java/org/onosproject/net/behaviour/PortDiscovery.java b/core/api/src/main/java/org/onosproject/net/behaviour/PortDiscovery.java index 7847cd5a98..6e22e7f6bb 100644 --- a/core/api/src/main/java/org/onosproject/net/behaviour/PortDiscovery.java +++ b/core/api/src/main/java/org/onosproject/net/behaviour/PortDiscovery.java @@ -26,12 +26,18 @@ import java.util.List; /** * Discovers the set of ports from a device through a device specific protocol. * The returned ports are not retrieved from the information stored in ONOS. + * + * @deprecated 1.6.0 Goldeneye. Use DeviceDescriptionDiscovery instead */ +@Deprecated public interface PortDiscovery extends HandlerBehaviour { /** * Retrieves the set of ports from a device. + * * @return a set of port descriptions. + * @deprecated 1.6.0 Goldeneye. Use DeviceDescriptionDiscovery instead */ + @Deprecated List getPorts(); } \ No newline at end of file diff --git a/core/api/src/main/java/org/onosproject/net/device/DeviceDescriptionDiscovery.java b/core/api/src/main/java/org/onosproject/net/device/DeviceDescriptionDiscovery.java index 4bc624b810..f7649b0a67 100644 --- a/core/api/src/main/java/org/onosproject/net/device/DeviceDescriptionDiscovery.java +++ b/core/api/src/main/java/org/onosproject/net/device/DeviceDescriptionDiscovery.java @@ -28,7 +28,7 @@ import java.util.List; public interface DeviceDescriptionDiscovery extends HandlerBehaviour { /** - * Returns device description appropriately annotated to support + * Returns a device description appropriately annotated to support * downstream model extension via projections of the resulting device, * as in the following example. * 
@@ -40,7 +40,7 @@ public interface DeviceDescriptionDiscovery extends HandlerBehaviour {
     DeviceDescription discoverDeviceDetails();
 
     /**
-     * Returns list of port descriptions appropriately annotated to support
+     * Returns a list of port descriptions appropriately annotated to support
      * downstream model extension via projections of their parent device,
      * as in the following example.
      * 
diff --git a/core/common/src/main/java/org/onosproject/common/net/AbstractDeviceProvider.java b/core/common/src/main/java/org/onosproject/common/net/AbstractDeviceProvider.java
index 9d7e35a547..856062544b 100644
--- a/core/common/src/main/java/org/onosproject/common/net/AbstractDeviceProvider.java
+++ b/core/common/src/main/java/org/onosproject/common/net/AbstractDeviceProvider.java
@@ -22,6 +22,7 @@ import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.device.DeviceDescription;
 import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.DeviceProvider;
 import org.onosproject.net.device.DeviceProviderRegistry;
@@ -93,10 +94,15 @@ public abstract class AbstractDeviceProvider extends AbstractProvider
     protected void discoverDevice(DriverHandler handler) {
         DeviceId deviceId = handler.data().deviceId();
         DeviceDescriptionDiscovery discovery = handler.behaviour(DeviceDescriptionDiscovery.class);
-        providerService.deviceConnected(deviceId, discovery.discoverDeviceDetails());
+        DeviceDescription description = discovery.discoverDeviceDetails();
+        if (description != null) {
+            providerService.deviceConnected(deviceId, description);
+        } else {
+            log.info("No other description given for device {}", deviceId);
+        }
         providerService.updatePorts(deviceId, discovery.discoverPortDetails());
-    }
 
+    }
     // TODO: inspect NETCONF, SNMP, RESTSB device providers for additional common patterns
     // TODO: provide base for port status update
     // TODO: integrate with network config for learning about management addresses to probe
diff --git a/drivers/ciena/src/main/java/org/onosproject/drivers/ciena/PortDiscoveryCienaWaveserverImpl.java b/drivers/ciena/src/main/java/org/onosproject/drivers/ciena/CienaWaveserverDeviceDescription.java
similarity index 88%
rename from drivers/ciena/src/main/java/org/onosproject/drivers/ciena/PortDiscoveryCienaWaveserverImpl.java
rename to drivers/ciena/src/main/java/org/onosproject/drivers/ciena/CienaWaveserverDeviceDescription.java
index 74df64cbaf..69bbe9c49c 100644
--- a/drivers/ciena/src/main/java/org/onosproject/drivers/ciena/PortDiscoveryCienaWaveserverImpl.java
+++ b/drivers/ciena/src/main/java/org/onosproject/drivers/ciena/CienaWaveserverDeviceDescription.java
@@ -18,6 +18,7 @@
 
 package org.onosproject.drivers.ciena;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.apache.commons.configuration.HierarchicalConfiguration;
 import org.onosproject.drivers.utilities.XmlConfigParser;
@@ -31,11 +32,13 @@ import org.onosproject.net.OchSignal;
 import org.onosproject.net.OduSignalType;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.SparseAnnotations;
-import org.onosproject.net.behaviour.PortDiscovery;
+import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.PortDescription;
 import org.onosproject.net.driver.AbstractHandlerBehaviour;
 import org.onosproject.net.driver.DriverHandler;
 import org.onosproject.protocol.rest.RestSBController;
+import org.slf4j.Logger;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -43,12 +46,15 @@ import java.util.List;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.onosproject.net.optical.device.OchPortHelper.ochPortDescription;
 import static org.onosproject.net.optical.device.OduCltPortHelper.oduCltPortDescription;
+import static org.slf4j.LoggerFactory.getLogger;
 
 /**
  * Discovers the ports from a Ciena WaveServer Rest device.
  */
-public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
-        implements PortDiscovery {
+public class CienaWaveserverDeviceDescription extends AbstractHandlerBehaviour
+        implements DeviceDescriptionDiscovery {
+
+    private final Logger log = getLogger(getClass());
 
     private static final String SPEED = "speed";
     private static final String GBPS = "Gbps";
@@ -74,9 +80,19 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
 //    private static final String SPECIFIC_PORT_CONFIG =
 //            "/ptp-config?config=true&format=xml&depth=unbounded";
 
+    @Override
+    public DeviceDescription discoverDeviceDetails() {
+        log.info("No description to be added for device");
+        //TODO to be implemented if needed.
+        return null;
+    }
 
     @Override
-    public List getPorts() {
+    public List discoverPortDetails() {
+        return getPorts();
+    }
+
+    private List getPorts() {
         List ports = Lists.newArrayList();
         DriverHandler handler = handler();
         RestSBController controller = checkNotNull(handler.get(RestSBController.class));
@@ -121,11 +137,11 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
                                 .replace(" ", EMPTY_STRING))) == speed100GbpsinMbps ?
                         CltSignalType.CLT_100GBE : null;
                 ports.add(oduCltPortDescription(PortNumber.portNumber(sub.getLong(PORT_ID)),
-                                                    sub.getString(ADMIN_STATE).equals(ENABLED),
-                                                    cltType, annotations.build()));
+                                                sub.getString(ADMIN_STATE).equals(ENABLED),
+                                                cltType, annotations.build()));
             }
         });
-        return ports;
+        return ImmutableList.copyOf(ports);
     }
 
     public static List parseWaveServerCienaPorts(HierarchicalConfiguration cfg) {
@@ -156,7 +172,7 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
                 baseFrequency)) / toGbpsFromHz(chSpacing.frequency().asHz())); //FIXME is there a better way ?
 
         return ochPortDescription(PortNumber.portNumber(portNumber), isEnabled, oduSignalType, isTunable,
-                                      new OchSignal(gridType, chSpacing, spacingMult, 1), annotations);
+                                  new OchSignal(gridType, chSpacing, spacingMult, 1), annotations);
     }
 
     //FIXME remove when all optical types have two way information methods, see jira tickets
diff --git a/drivers/ciena/src/main/resources/ciena-drivers.xml b/drivers/ciena/src/main/resources/ciena-drivers.xml
index eb08b25a21..be9e7cb36c 100644
--- a/drivers/ciena/src/main/resources/ciena-drivers.xml
+++ b/drivers/ciena/src/main/resources/ciena-drivers.xml
@@ -20,6 +20,8 @@
                    impl="org.onosproject.drivers.ciena.PortDiscoveryCienaWaveserverImpl"/>
         
+        
     
 
 
diff --git a/drivers/fujitsu/src/main/java/org/onosproject/drivers/fujitsu/PortGetterFujitsuImpl.java b/drivers/fujitsu/src/main/java/org/onosproject/drivers/fujitsu/FujitsuT100DeviceDescription.java
similarity index 91%
rename from drivers/fujitsu/src/main/java/org/onosproject/drivers/fujitsu/PortGetterFujitsuImpl.java
rename to drivers/fujitsu/src/main/java/org/onosproject/drivers/fujitsu/FujitsuT100DeviceDescription.java
index a65247f753..8b8d1707e8 100644
--- a/drivers/fujitsu/src/main/java/org/onosproject/drivers/fujitsu/PortGetterFujitsuImpl.java
+++ b/drivers/fujitsu/src/main/java/org/onosproject/drivers/fujitsu/FujitsuT100DeviceDescription.java
@@ -16,6 +16,8 @@
 
 package org.onosproject.drivers.fujitsu;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 import org.apache.commons.configuration.HierarchicalConfiguration;
 import org.onosproject.drivers.utilities.XmlConfigParser;
 import org.onosproject.net.AnnotationKeys;
@@ -26,7 +28,8 @@ import org.onosproject.net.GridType;
 import org.onosproject.net.OchSignal;
 import org.onosproject.net.OduSignalType;
 import org.onosproject.net.PortNumber;
-import org.onosproject.net.behaviour.PortDiscovery;
+import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.PortDescription;
 import org.onosproject.net.driver.AbstractHandlerBehaviour;
 import org.onosproject.netconf.NetconfController;
@@ -34,8 +37,6 @@ import org.onosproject.netconf.NetconfException;
 import org.onosproject.netconf.NetconfSession;
 import org.slf4j.Logger;
 
-import com.google.common.collect.Lists;
-
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.util.List;
@@ -49,13 +50,20 @@ import static org.slf4j.LoggerFactory.getLogger;
 /**
  * Retrieves the ports from a Fujitsu T100 device via netconf.
  */
-public class PortGetterFujitsuImpl extends AbstractHandlerBehaviour
-        implements PortDiscovery {
+public class FujitsuT100DeviceDescription extends AbstractHandlerBehaviour
+        implements DeviceDescriptionDiscovery {
 
     private final Logger log = getLogger(getClass());
 
     @Override
-    public List getPorts() {
+    public DeviceDescription discoverDeviceDetails() {
+        log.info("No description to be added for device");
+        //TODO to be implemented if needed.
+        return null;
+    }
+
+    @Override
+    public List discoverPortDetails() {
         NetconfController controller = checkNotNull(handler().get(NetconfController.class));
         NetconfSession session = controller.getDevicesMap().get(handler().data().deviceId()).getSession();
         String reply;
@@ -67,12 +75,13 @@ public class PortGetterFujitsuImpl extends AbstractHandlerBehaviour
         List descriptions =
                 parseFujitsuT100Ports(XmlConfigParser.
                         loadXml(new ByteArrayInputStream(reply.getBytes())));
-        return descriptions;
+        return ImmutableList.copyOf(descriptions);
     }
 
     /**
      * Builds a request crafted to get the configuration required to create port
      * descriptions for the device.
+     *
      * @return The request string.
      */
     private String requestBuilder() {
@@ -91,6 +100,7 @@ public class PortGetterFujitsuImpl extends AbstractHandlerBehaviour
 
     /**
      * Parses a configuration and returns a set of ports for the fujitsu T100.
+     *
      * @param cfg a hierarchical configuration
      * @return a list of port descriptions
      */
diff --git a/drivers/fujitsu/src/main/resources/fujitsu-drivers.xml b/drivers/fujitsu/src/main/resources/fujitsu-drivers.xml
index 6fa70953c2..d545eda042 100644
--- a/drivers/fujitsu/src/main/resources/fujitsu-drivers.xml
+++ b/drivers/fujitsu/src/main/resources/fujitsu-drivers.xml
@@ -20,6 +20,8 @@
                    impl="org.onosproject.drivers.fujitsu.PortGetterFujitsuImpl"/>
         
+        
     
 
 
diff --git a/drivers/lumentum/src/main/java/org/onosproject/drivers/lumentum/LumentumRoadmDeviceDescription.java b/drivers/lumentum/src/main/java/org/onosproject/drivers/lumentum/LumentumRoadmDeviceDescription.java
index 0bf9b1301d..46e0887ba0 100644
--- a/drivers/lumentum/src/main/java/org/onosproject/drivers/lumentum/LumentumRoadmDeviceDescription.java
+++ b/drivers/lumentum/src/main/java/org/onosproject/drivers/lumentum/LumentumRoadmDeviceDescription.java
@@ -16,6 +16,7 @@
 
 package org.onosproject.drivers.lumentum;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.onosproject.net.AnnotationKeys;
 import org.onosproject.net.DefaultAnnotations;
@@ -45,7 +46,7 @@ import static org.slf4j.LoggerFactory.getLogger;
 /**
  * Device description behaviour for Lumentum Snmp devices.
  */
-public class LumentumRoadmDeviceDescription extends AbstractHandlerBehaviour implements DeviceDescriptionDiscovery  {
+public class LumentumRoadmDeviceDescription extends AbstractHandlerBehaviour implements DeviceDescriptionDiscovery {
 
     private final Logger log = getLogger(getClass());
 
@@ -66,7 +67,7 @@ public class LumentumRoadmDeviceDescription extends AbstractHandlerBehaviour imp
 
     @Override
     public List discoverPortDetails() {
-        return this.getPorts();
+        return ImmutableList.copyOf(this.getPorts());
     }
 
     private List getPorts() {
diff --git a/providers/netconf/device/src/main/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProvider.java b/providers/netconf/device/src/main/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProvider.java
index 32e35b0a65..4fca42f84d 100644
--- a/providers/netconf/device/src/main/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProvider.java
+++ b/providers/netconf/device/src/main/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProvider.java
@@ -43,6 +43,7 @@ import org.onosproject.net.config.NetworkConfigListener;
 import org.onosproject.net.config.NetworkConfigRegistry;
 import org.onosproject.net.device.DefaultDeviceDescription;
 import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.DeviceEvent;
 import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceProvider;
@@ -331,10 +332,17 @@ public class NetconfDeviceProvider extends AbstractProvider
 
     private void discoverPorts(DeviceId deviceId) {
         Device device = deviceService.getDevice(deviceId);
+        //TODO remove when PortDiscovery is removed from master
         if (device.is(PortDiscovery.class)) {
             PortDiscovery portConfig = device.as(PortDiscovery.class);
             providerService.updatePorts(deviceId,
                                         portConfig.getPorts());
+        } else if (device.is(DeviceDescriptionDiscovery.class)) {
+            DeviceDescriptionDiscovery deviceDescriptionDiscovery =
+                    device.as(DeviceDescriptionDiscovery.class);
+            providerService.updatePorts(deviceId,
+                                        deviceDescriptionDiscovery.discoverPortDetails());
+
         } else {
             log.warn("No portGetter behaviour for device {}", deviceId);
         }
@@ -343,7 +351,7 @@ public class NetconfDeviceProvider extends AbstractProvider
     /**
      * Return the DeviceId about the device containing the URI.
      *
-     * @param ip IP address
+     * @param ip   IP address
      * @param port port number
      * @return DeviceId
      */
diff --git a/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java b/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
index 5b60fb9ee6..eaafc30c09 100644
--- a/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
+++ b/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
@@ -40,11 +40,11 @@ import org.onosproject.net.config.NetworkConfigListener;
 import org.onosproject.net.config.NetworkConfigRegistry;
 import org.onosproject.net.device.DefaultDeviceDescription;
 import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceDescriptionDiscovery;
 import org.onosproject.net.device.DeviceProvider;
 import org.onosproject.net.device.DeviceProviderRegistry;
 import org.onosproject.net.device.DeviceProviderService;
-import org.onosproject.net.driver.DriverHandler;
-import org.onosproject.net.driver.DriverService;
+import org.onosproject.net.device.DeviceService;
 import org.onosproject.net.provider.AbstractProvider;
 import org.onosproject.net.provider.ProviderId;
 import org.onosproject.protocol.rest.RestSBController;
@@ -93,7 +93,7 @@ public class RestDeviceProvider extends AbstractProvider
     protected CoreService coreService;
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected DriverService driverService;
+    protected DeviceService deviceService;
 
 
     private DeviceProviderService providerService;
@@ -219,19 +219,27 @@ public class RestDeviceProvider extends AbstractProvider
             log.error("Configuration error {}", e);
         }
         log.debug("REST Devices {}", controller.getDevices());
-        addedDevices.forEach(deviceId -> {
-            DriverHandler h = driverService.createHandler(deviceId);
-            PortDiscovery portConfig = h.behaviour(PortDiscovery.class);
-            if (portConfig != null) {
-                providerService.updatePorts(deviceId, portConfig.getPorts());
-            } else {
-                log.warn("No portGetter behaviour for device {}", deviceId);
-            }
-        });
+        addedDevices.forEach(this::discoverPorts);
         addedDevices.clear();
 
     }
 
+    private void discoverPorts(DeviceId deviceId) {
+        Device device = deviceService.getDevice(deviceId);
+        //TODO remove when PortDiscovery is removed from master
+        if (device.is(PortDiscovery.class)) {
+            PortDiscovery portConfig = device.as(PortDiscovery.class);
+            providerService.updatePorts(deviceId,
+                                        portConfig.getPorts());
+        } else if (device.is(DeviceDescriptionDiscovery.class)) {
+            DeviceDescriptionDiscovery deviceDescriptionDiscovery =
+                    device.as(DeviceDescriptionDiscovery.class);
+            providerService.updatePorts(deviceId, deviceDescriptionDiscovery.discoverPortDetails());
+        } else {
+            log.warn("No portGetter behaviour for device {}", deviceId);
+        }
+    }
+
     private boolean testDeviceConnection(RestSBDevice device) {
         try {
             return controller.get(device.deviceId(), "", "json") != null;
diff --git a/providers/snmp/device/src/main/java/org/onosproject/provider/snmp/device/impl/SnmpDeviceProvider.java b/providers/snmp/device/src/main/java/org/onosproject/provider/snmp/device/impl/SnmpDeviceProvider.java
index 01fb738cfb..280d3ffe42 100644
--- a/providers/snmp/device/src/main/java/org/onosproject/provider/snmp/device/impl/SnmpDeviceProvider.java
+++ b/providers/snmp/device/src/main/java/org/onosproject/provider/snmp/device/impl/SnmpDeviceProvider.java
@@ -287,9 +287,13 @@ public class SnmpDeviceProvider extends AbstractProvider
                 if (d.is(DeviceDescriptionDiscovery.class)) {
                     DeviceDescriptionDiscovery descriptionDiscovery = d.as(DeviceDescriptionDiscovery.class);
                     DeviceDescription description = descriptionDiscovery.discoverDeviceDetails();
-                    deviceStore.createOrUpdateDevice(
-                            new ProviderId("snmp", "org.onosproject.provider.device"),
-                            did, description);
+                    if (description != null) {
+                        deviceStore.createOrUpdateDevice(
+                                new ProviderId("snmp", "org.onosproject.provider.device"),
+                                did, description);
+                    } else {
+                        log.info("No other description given for device {}", d.id());
+                    }
                     providerService.updatePorts(did, descriptionDiscovery.discoverPortDetails());
                 } else {
                     log.warn("No populate description and ports behaviour for device {}", did);