Proper device state management in server driver

This patch complements Change 19229 to fix the issues
caused by multi-server deployments. Previously, only one
server device could be detected by ONOS due to the proxy field.
Also, when that device went offline, the driver did not notify the
RestController.
With this patch, the server device driver cooperates with the
RestController to provide a consistent server activity state for
multiple servers.

Addressed comments made by ONOS reviewers

Change-Id: Ifc0c556a2a5322fd2ee8b02065a2a507cf6b92fc
Signed-off-by: Georgios Katsikas <katsikas.gp@gmail.com>
diff --git a/drivers/server/src/main/java/org/onosproject/drivers/server/BasicServerDriver.java b/drivers/server/src/main/java/org/onosproject/drivers/server/BasicServerDriver.java
index 046578a..4fffe6e 100644
--- a/drivers/server/src/main/java/org/onosproject/drivers/server/BasicServerDriver.java
+++ b/drivers/server/src/main/java/org/onosproject/drivers/server/BasicServerDriver.java
@@ -21,8 +21,7 @@
 import org.onosproject.net.driver.AbstractHandlerBehaviour;
 import org.onosproject.net.driver.DriverHandler;
 import org.onosproject.protocol.rest.RestSBController;
-
-import org.onlab.osgi.ServiceNotFoundException;
+import org.onosproject.protocol.rest.RestSBDevice;
 
 import org.slf4j.Logger;
 
@@ -47,7 +46,8 @@
      */
     public    static final MediaType  JSON = MediaType.valueOf(MediaType.APPLICATION_JSON);
     protected static final String ROOT_URL = "";
-    public    static final String BASE_URL = ROOT_URL + "/metron";
+    protected static final String SLASH = "/";
+    public    static final String BASE_URL = ROOT_URL + SLASH + "metron";
 
     /**
      * Common parameters to be exchanged with the server's agent.
@@ -85,61 +85,36 @@
     public BasicServerDriver() {};
 
     /**
-     * Initialize the REST SB controller (if not already).
-     * Creates a handler and a controller only once, and
-     * then re-uses these objects.
+     * Retrieve an instance of the driver handler.
      *
-     * @throws ServiceNotFoundException when either the handler
-     *         or the controller cannot be retrieved.
+     * @return DriverHandler instance
      */
-    private void init() {
+    protected DriverHandler getHandler() {
         synchronized (CONTROLLER_LOCK) {
-            // Already done
-            if ((handler != null) && (controller != null)) {
-                return;
-            }
-
             handler = handler();
             checkNotNull(handler, HANDLER_NULL);
-            controller = handler.get(RestSBController.class);
-            checkNotNull(controller, CONTROLLER_NULL);
         }
+
+        return handler;
     }
 
     /**
      * Retrieve an instance of the REST SB controller.
-     * Method init will only be called the first time
-     * this method (or getHandler) is invoked.
      *
      * @return RestSBController instance
      */
     public RestSBController getController() {
         synchronized (CONTROLLER_LOCK) {
             if (controller == null) {
-                init();
+                controller = getHandler().get(RestSBController.class);
+                checkNotNull(controller, CONTROLLER_NULL);
             }
         }
+
         return controller;
     }
 
     /**
-     * Retrieve an instance of the driver handler.
-     * Method init will only be called the first time
-     * this method (or getController) is invoked.
-     *
-     * @return DriverHandler instance
-     */
-    protected DriverHandler getHandler() {
-        synchronized (CONTROLLER_LOCK) {
-            if (handler == null) {
-                init();
-            }
-        }
-
-        return handler;
-    }
-
-    /**
      * Finds the NIC name that corresponds to a device's port number.
      *
      * @param deviceId a device ID
@@ -147,6 +122,10 @@
      * @return device's NIC name
      */
     public static String findNicInterfaceWithPort(DeviceId deviceId, long port) {
+        if (controller == null) {
+            return null;
+        }
+
         RestServerSBDevice device = null;
         try {
             device = (RestServerSBDevice) controller.getDevice(deviceId);
@@ -165,8 +144,7 @@
      * @param enumType the enum class to get its types
      * @return String with all enumeration types
      */
-    public static <E extends Enum<E>> String enumTypesToString(
-            Class<E> enumType) {
+    public static <E extends Enum<E>> String enumTypesToString(Class<E> enumType) {
         String allTypes = "";
         for (E en : EnumSet.allOf(enumType)) {
             allTypes += en.toString() + " ";
@@ -215,4 +193,21 @@
         return false;
     }
 
+    /**
+     * Upon a failure to contact a device, the driver
+     * raises a disconnect event by resetting the
+     * activity flag of this device.
+     *
+     * @param device a device to disconnect
+     */
+    protected void raiseDeviceDisconnect(RestSBDevice device) {
+        // Already done!
+        if (!device.isActive()) {
+            return;
+        }
+
+        log.info("Setting device {} inactive", device.deviceId());
+        device.setActive(false);
+    }
+
 }
diff --git a/drivers/server/src/main/java/org/onosproject/drivers/server/FlowRuleProgrammableServerImpl.java b/drivers/server/src/main/java/org/onosproject/drivers/server/FlowRuleProgrammableServerImpl.java
index efc7bd6..9c38fe8 100644
--- a/drivers/server/src/main/java/org/onosproject/drivers/server/FlowRuleProgrammableServerImpl.java
+++ b/drivers/server/src/main/java/org/onosproject/drivers/server/FlowRuleProgrammableServerImpl.java
@@ -62,7 +62,7 @@
     /**
      * Resource endpoints of the server agent (REST server-side).
      */
-    private static final String RULE_MANAGEMENT_URL = BASE_URL + "/rules";
+    private static final String RULE_MANAGEMENT_URL = BASE_URL + SLASH + "rules";
 
     /**
      * Parameters to be exchanged with the server's agent.
@@ -360,19 +360,23 @@
      * @return boolean removal status
      */
     private boolean removeNicFlowRule(DeviceId deviceId, long ruleId) {
-        // Remove rule with ID from this server
-        int response = getController().delete(deviceId,
-            RULE_MANAGEMENT_URL + "/" + Long.toString(ruleId), null, JSON);
+        int response = -1;
 
-        if (!checkStatusCode(response)) {
-            log.error("Failed to remove flow rule {} from device {}",
-                ruleId, deviceId);
+        // Try to remove the rule, although server might be unreachable
+        try {
+            response = getController().delete(deviceId,
+                RULE_MANAGEMENT_URL + SLASH + Long.toString(ruleId), null, JSON);
+        } catch (Exception ex) {
+            log.error("Failed to remove flow rule {} from device {}", ruleId, deviceId);
             return false;
         }
 
-        log.info("Successfully removed flow rule {} from device {}",
-            ruleId, deviceId);
+        if (!checkStatusCode(response)) {
+            log.error("Failed to remove flow rule {} from device {}", ruleId, deviceId);
+            return false;
+        }
 
+        log.info("Successfully removed flow rule {} from device {}", ruleId, deviceId);
         return true;
     }
 
diff --git a/drivers/server/src/main/java/org/onosproject/drivers/server/ServerControllerConfig.java b/drivers/server/src/main/java/org/onosproject/drivers/server/ServerControllerConfig.java
index db62a6b..3ffbc91 100644
--- a/drivers/server/src/main/java/org/onosproject/drivers/server/ServerControllerConfig.java
+++ b/drivers/server/src/main/java/org/onosproject/drivers/server/ServerControllerConfig.java
@@ -53,7 +53,7 @@
     /**
      * Resource endpoints of the server agent (REST server-side).
      */
-    private static final String CONTROLLERS_CONF_URL = BASE_URL + "/controllers";
+    private static final String CONTROLLERS_CONF_URL = BASE_URL + SLASH + "controllers";
 
     /**
      * Parameters to be exchanged with the server's agent.
@@ -219,7 +219,7 @@
             log.info("Remove controller with {}:{}:{}",
                 ctrl.type(), ctrl.ip().toString(), ctrl.port());
 
-            String remCtrlUrl = CONTROLLERS_CONF_URL + "/" + ctrl.ip().toString();
+            String remCtrlUrl = CONTROLLERS_CONF_URL + SLASH + ctrl.ip().toString();
 
             // Remove this controller
             int response = getController().delete(deviceId, remCtrlUrl, null, JSON);
diff --git a/drivers/server/src/main/java/org/onosproject/drivers/server/ServerDevicesDiscovery.java b/drivers/server/src/main/java/org/onosproject/drivers/server/ServerDevicesDiscovery.java
index 0755bcd..10203a3 100644
--- a/drivers/server/src/main/java/org/onosproject/drivers/server/ServerDevicesDiscovery.java
+++ b/drivers/server/src/main/java/org/onosproject/drivers/server/ServerDevicesDiscovery.java
@@ -99,9 +99,9 @@
     /**
      * Resource endpoints of the server agent (REST server-side).
      */
-    private static final String RESOURCE_DISCOVERY_URL   = BASE_URL + "/resources";
-    private static final String GLOBAL_STATS_URL         = BASE_URL + "/stats";
-    private static final String SERVICE_CHAINS_STATS_URL = BASE_URL + "/chains_stats";  // + /ID
+    private static final String RESOURCE_DISCOVERY_URL   = BASE_URL + SLASH + "resources";
+    private static final String GLOBAL_STATS_URL         = BASE_URL + SLASH + "stats";
+    private static final String SERVICE_CHAINS_STATS_URL = BASE_URL + SLASH + "chains_stats";  // + /ID
 
     /**
      * Parameters to be exchanged with the server's agent.
@@ -211,9 +211,6 @@
      * @return a DeviceDescription with the device's features
      */
     private DeviceDescription getDeviceDetails(DeviceId deviceId) {
-        // Create a description for this server device
-        ServerDeviceDescription desc = null;
-
         // Retrieve the device ID, if null given
         if (deviceId == null) {
             deviceId = getHandler().data().deviceId();
@@ -230,7 +227,7 @@
             response = getController().get(deviceId, RESOURCE_DISCOVERY_URL, JSON);
         } catch (ProcessingException pEx) {
             log.error("Failed to discover the device details of: {}", deviceId);
-            return desc;
+            return null;
         }
 
         // Load the JSON into objects
@@ -244,12 +241,12 @@
             objNode = (ObjectNode) jsonNode;
         } catch (IOException ioEx) {
             log.error("Failed to discover the device details of: {}", deviceId);
-            return desc;
+            return null;
         }
 
         if (jsonMap == null) {
             log.error("Failed to discover the device details of: {}", deviceId);
-            return desc;
+            return null;
         }
 
         // Get all the attributes
@@ -357,6 +354,9 @@
         getController().removeDevice(deviceId);
         getController().addDevice((RestSBDevice) dev);
 
+        // Create a description for this server device
+        ServerDeviceDescription desc = null;
+
         try {
             desc = new DefaultServerDeviceDescription(
                 new URI(id), Device.Type.SERVER, vendor,
@@ -376,44 +376,31 @@
 
     @Override
     public List<PortDescription> discoverPortDetails() {
-        // List of port descriptions to return
-        List<PortDescription> portDescriptions = Lists.newArrayList();
-
         // Retrieve the device ID
         DeviceId deviceId = getHandler().data().deviceId();
         checkNotNull(deviceId, DEVICE_ID_NULL);
+
         // .. and object
         RestServerSBDevice device = null;
-
-        // In case this method is called before discoverDeviceDetails,
-        // there is missing information to be gathered.
-        short i = 0;
-        while ((device == null) && (i < DISCOVERY_RETRIES)) {
-            i++;
-
-            try {
-                device = (RestServerSBDevice) getController().getDevice(deviceId);
-            } catch (ClassCastException ccEx) {
-                try {
-                    Thread.sleep(1);
-                } catch (InterruptedException intEx) {
-                    // Just retry
-                    continue;
-                }
-            }
-
-            // No device
-            if (device == null) {
-                // This method will add the device to the RestSBController
-                this.getDeviceDetails(deviceId);
-            }
+        try {
+            device = (RestServerSBDevice) getController().getDevice(deviceId);
+        } catch (ClassCastException ccEx) {
+            log.error("Failed to discover ports for device {}", deviceId);
+            return Collections.EMPTY_LIST;
         }
 
-        if ((device == null) || (device.nics() == null)) {
+        if (device == null) {
+            log.error("No device with ID {} is available for port discovery", deviceId);
+            return Collections.EMPTY_LIST;
+        }
+        if ((device.nics() == null) || (device.nics().size() == 0)) {
             log.error("No ports available on {}", deviceId);
             return Collections.EMPTY_LIST;
         }
 
+        // List of port descriptions to return
+        List<PortDescription> portDescriptions = Lists.newArrayList();
+
         // Sorted list of NIC ports
         Set<NicDevice> nics = new TreeSet(device.nics());
 
@@ -537,7 +524,9 @@
                 deviceId);
             return monStats;
         }
-        checkNotNull(device, DEVICE_NULL);
+        if ((device == null) || (!device.isActive())) {
+            return monStats;
+        }
 
         // Hit the path that provides the server's global resources
         InputStream response = null;
@@ -546,6 +535,7 @@
         } catch (ProcessingException pEx) {
             log.error("Failed to retrieve global monitoring statistics from device {}",
                 deviceId);
+            raiseDeviceDisconnect(device);
             return monStats;
         }
 
@@ -560,12 +550,14 @@
         } catch (IOException ioEx) {
             log.error("Failed to retrieve global monitoring statistics from device {}",
                 deviceId);
+            raiseDeviceDisconnect(device);
             return monStats;
         }
 
         if (jsonMap == null) {
             log.error("Failed to retrieve global monitoring statistics from device {}",
                 deviceId);
+            raiseDeviceDisconnect(device);
             return monStats;
         }
 
@@ -629,10 +621,12 @@
                 deviceId);
             return monStats;
         }
-        checkNotNull(device, DEVICE_NULL);
+        if (device == null) {
+            return monStats;
+        }
 
         // Create a resource-specific URL
-        String scUrl = SERVICE_CHAINS_STATS_URL + "/" + tcId.toString();
+        String scUrl = SERVICE_CHAINS_STATS_URL + SLASH + tcId.toString();
 
         // Hit the path that provides the server's specific resources
         InputStream response = null;
@@ -641,6 +635,7 @@
         } catch (ProcessingException pEx) {
             log.error("Failed to retrieve monitoring statistics from device {}",
                 deviceId);
+            raiseDeviceDisconnect(device);
             return monStats;
         }
 
@@ -656,12 +651,14 @@
         } catch (IOException ioEx) {
             log.error("Failed to retrieve monitoring statistics from device {}",
                 deviceId);
+            raiseDeviceDisconnect(device);
             return monStats;
         }
 
         if (jsonMap == null) {
             log.error("Failed to retrieve monitoring statistics from device {}",
                 deviceId);
+            raiseDeviceDisconnect(device);
             return monStats;
         }
 
@@ -711,9 +708,8 @@
      * @param objNode input JSON node with CPU statistics information
      * @return list of (per core) CpuStatistics
      */
-    private Collection<CpuStatistics> parseCpuStatistics(
-            DeviceId deviceId, JsonNode objNode) {
-        if (objNode == null) {
+    private Collection<CpuStatistics> parseCpuStatistics(DeviceId deviceId, JsonNode objNode) {
+        if ((deviceId == null) || (objNode == null)) {
             return Collections.EMPTY_LIST;
         }
 
@@ -755,9 +751,8 @@
      * @param objNode input JSON node with NIC statistics information
      * @return list of (per port) PortStatistics
      */
-    private Collection<PortStatistics> parseNicStatistics(
-            DeviceId deviceId, JsonNode objNode) {
-        if (objNode == null) {
+    private Collection<PortStatistics> parseNicStatistics(DeviceId deviceId, JsonNode objNode) {
+        if ((deviceId == null) || (objNode == null)) {
             return Collections.EMPTY_LIST;
         }
 
@@ -767,7 +762,9 @@
         } catch (ClassCastException ccEx) {
             return Collections.EMPTY_LIST;
         }
-        checkNotNull(device, DEVICE_NULL);
+        if (device == null) {
+            return Collections.EMPTY_LIST;
+        }
 
         Collection<PortStatistics> nicStats = Lists.newArrayList();