ONOS-3941 Adding Executor pool and support for username but passwordless https login in Rest SB

Change-Id: Ia3da59dbffcabf233a27931aa756488629e8dd63

ONOS-3941 Adding Executor pool and support for username but passwordless https login in Rest SB
Change-Id: Ia3da59dbffcabf233a27931aa756488629e8dd63
Andrea Campanella · Gerrit Code Review
Commit 784ee0ff976a6e2d598a1c955f7771aef1885ec1 784ee0ff 1 parent 2464dc3d
Showing 6 changed files with 47 additions and 31 deletions
drivers/ciena/src/main/java/org/onosproject/drivers/ciena/PortDiscoveryCienaWaveserverImpl.java
protocols/rest/api/src/main/java/org/onosproject/protocol/rest/DefaultRestSBDevice.java
protocols/rest/api/src/main/java/org/onosproject/protocol/rest/RestSBDevice.java
protocols/rest/ctl/src/main/java/org/onosproject/protocol/rest/ctl/RestSBControllerImpl.java
providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestProviderConfig.java
--- a/drivers/ciena/src/main/java/org/onosproject/drivers/ciena/PortDiscoveryCienaWaveserverImpl.java
View file @784ee0f
+++ b/drivers/ciena/src/main/java/org/onosproject/drivers/ciena/PortDiscoveryCienaWaveserverImpl.java
View file @784ee0f
@@ -26,7 +26,6 @@ import org.onosproject.net.CltSignalType;
 import org.onosproject.net.DefaultAnnotations;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.PortNumber;
-import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.behaviour.PortDiscovery;
 import org.onosproject.net.device.OduCltPortDescription;
 import org.onosproject.net.device.PortDescription;
@@ -53,6 +52,8 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
     private static final String EMPTY_STRING = "";
     private static final String NAME = "name";
     private static final String ADMIN_STATE = "admin-state";
+    private static final String LINESIDE_NAME = "lineside";
+    private static final String CLIENTSIDE_NAME = "clientside";
     private static final ArrayList<String> LINESIDE_PORT_ID = Lists.newArrayList(
             "4", "48");
@@ -85,9 +86,10 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
         portsConfig.stream().forEach(sub -> {
             String portId = sub.getString(PORT_ID);
             String name = sub.getString(NAME);
-            SparseAnnotations annotations = DefaultAnnotations.builder()
+            DefaultAnnotations.Builder annotations = DefaultAnnotations.builder()
-                    .set(AnnotationKeys.NAME, name).build();
+                    .set(AnnotationKeys.NAME, name);
             if (LINESIDE_PORT_ID.contains(portId)) {
+                annotations.set(AnnotationKeys.PORT_NAME, LINESIDE_NAME);
                 String wsportInfoRequest = SPECIFIC_PORT_PATH + portId +
                         SPECIFIC_PORT_CONFIG;
                 ports.add(XmlConfigParser.parseWaveServerCienaOchPorts(
@@ -95,17 +97,17 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
                         toGbps(Long.parseLong(sub.getString(SPEED).replace(GBPS, EMPTY_STRING)
                                                       .replace(" ", EMPTY_STRING))),
                         XmlConfigParser.loadXml(controller.get(deviceId, wsportInfoRequest, XML)),
-                        annotations));
+                        annotations.build()));
                 //adding corresponding opposite side port
                 ports.add(XmlConfigParser.parseWaveServerCienaOchPorts(
                         sub.getLong(PORT_ID) + 1,
                         toGbps(Long.parseLong(sub.getString(SPEED).replace(GBPS, EMPTY_STRING)
                                                       .replace(" ", EMPTY_STRING))),
                         XmlConfigParser.loadXml(controller.get(deviceId, wsportInfoRequest, XML)),
-                        DefaultAnnotations.builder()
+                        annotations.set(AnnotationKeys.NAME, name.replace(".1", ".2"))
-                                .set(AnnotationKeys.NAME, name.replace(".1", ".2"))
                                 .build()));
             } else if (!portId.equals("5") && !portId.equals("49")) {
+                annotations.set(AnnotationKeys.PORT_NAME, CLIENTSIDE_NAME);
                 //FIXME change when all optical types have two way information methods, see jira tickets
                 final int speed100GbpsinMbps = 100000;
                 CltSignalType cltType = toGbps(Long.parseLong(
@@ -114,7 +116,7 @@ public class PortDiscoveryCienaWaveserverImpl extends AbstractHandlerBehaviour
                         CltSignalType.CLT_100GBE : null;
                 ports.add(new OduCltPortDescription(PortNumber.portNumber(sub.getLong(PORT_ID)),
                                                     sub.getString(ADMIN_STATE).equals(ENABLED),
-                                                    cltType, annotations));
+                                                    cltType, annotations.build()));
             }
         });
         return ports;
--- a/protocols/rest/api/src/main/java/org/onosproject/protocol/rest/DefaultRestSBDevice.java
View file @784ee0f
+++ b/protocols/rest/api/src/main/java/org/onosproject/protocol/rest/DefaultRestSBDevice.java
View file @784ee0f
@@ -34,7 +34,7 @@ public class DefaultRestSBDevice implements RestSBDevice {
     private static final String COLON = ":";
     private final IpAddress ip;
     private final int port;
-    private final String name;
+    private final String username;
     private final String password;
     private boolean isActive;
     private String protocol;
@@ -47,7 +47,7 @@ public class DefaultRestSBDevice implements RestSBDevice {
         Preconditions.checkNotNull(protocol, "protocol address cannot be null");
         this.ip = ip;
         this.port = port;
-        this.name = name;
+        this.username = name;
         this.password = StringUtils.isEmpty(password) ? null : password;
         this.isActive = isActive;
         this.protocol = protocol;
@@ -65,8 +65,8 @@ public class DefaultRestSBDevice implements RestSBDevice {
     }
     @Override
-    public String name() {
+    public String username() {
-        return name;
+        return username;
     }
     @Override
@@ -110,7 +110,7 @@ public class DefaultRestSBDevice implements RestSBDevice {
         return MoreObjects.toStringHelper(this)
                 .add("url", url)
                 .add("protocol", protocol)
-                .add("name", name)
+                .add("username", username)
                 .add("port", port)
                 .add("ip", ip)
                 .toString();
@@ -125,7 +125,7 @@ public class DefaultRestSBDevice implements RestSBDevice {
             return false;
         }
         RestSBDevice device = (RestSBDevice) obj;
-        return this.name.equals(device.name()) && this.ip.equals(device.ip()) &&
+        return this.username.equals(device.username()) && this.ip.equals(device.ip()) &&
                 this.port == device.port();
     }
--- a/protocols/rest/api/src/main/java/org/onosproject/protocol/rest/RestSBDevice.java
View file @784ee0f
+++ b/protocols/rest/api/src/main/java/org/onosproject/protocol/rest/RestSBDevice.java
View file @784ee0f
@@ -38,11 +38,11 @@ public interface RestSBDevice {
     int port();
     /**
-     * Returns the name of this device.
+     * Returns the username of this device.
      *
-     * @return name
+     * @return username
      */
-    String name();
+    String username();
     /**
      * Returns the password of this device.
--- a/protocols/rest/ctl/src/main/java/org/onosproject/protocol/rest/ctl/RestSBControllerImpl.java
View file @784ee0f
+++ b/protocols/rest/ctl/src/main/java/org/onosproject/protocol/rest/ctl/RestSBControllerImpl.java
View file @784ee0f
@@ -182,8 +182,9 @@ public class RestSBControllerImpl implements RestSBController {
         try {
             log.debug("Url request {} ", getUrlString(device, request));
             HttpPatch httprequest = new HttpPatch(getUrlString(device, request));
-            if (deviceMap.get(device).password() != null) {
+            if (deviceMap.get(device).username() != null) {
-                String userPassword = deviceMap.get(device).name() + COLON + deviceMap.get(device).password();
+                String pwd = deviceMap.get(device).password() == null ? "" : COLON + deviceMap.get(device).password();
+                String userPassword = deviceMap.get(device).username() + pwd;
                 String base64string = Base64.getEncoder().encodeToString(userPassword.getBytes(StandardCharsets.UTF_8));
                 httprequest.addHeader(AUTHORIZATION_PROPERTY, BASIC_AUTH_PREFIX + base64string);
             }
@@ -232,9 +233,10 @@ public class RestSBControllerImpl implements RestSBController {
     private WebResource getWebResource(DeviceId device, String request) {
         log.debug("Sending request to URL {} ", getUrlString(device, request));
         WebResource webResource = client.resource(getUrlString(device, request));
-        if (deviceMap.containsKey(device) && deviceMap.get(device).password() != null) {
+        if (deviceMap.containsKey(device) && deviceMap.get(device).username() != null) {
-            client.addFilter(new HTTPBasicAuthFilter(deviceMap.get(device).name(),
+            client.addFilter(new HTTPBasicAuthFilter(deviceMap.get(device).username(),
-                                                     deviceMap.get(device).password()));
+                                                     deviceMap.get(device).password() == null ?
+                                                             "" : deviceMap.get(device).password()));
         }
         return webResource;
     }
--- a/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
View file @784ee0f
+++ b/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
View file @784ee0f
@@ -59,7 +59,10 @@ import java.security.NoSuchAlgorithmException;
 import java.util.Base64;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import static org.onlab.util.Tools.groupedThreads;
 import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_ADDED;
 import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_UPDATED;
 import static org.onosproject.net.config.basics.SubjectFactories.APP_SUBJECT_FACTORY;
@@ -102,6 +105,9 @@ public class RestDeviceProvider extends AbstractProvider
     protected static final String ISNOTNULL = "Rest device is not null";
     private static final String UNKNOWN = "unknown";
+    private final ExecutorService executor =
+            Executors.newFixedThreadPool(5, groupedThreads("onos/restsbprovider", "device-installer-%d"));
+
     private final ConfigFactory factory =
             new ConfigFactory<ApplicationId, RestProviderConfig>(APP_SUBJECT_FACTORY,
                                                                  RestProviderConfig.class,
@@ -115,6 +121,8 @@ public class RestDeviceProvider extends AbstractProvider
     private final NetworkConfigListener cfgLister = new InternalNetworkConfigListener();
     private ApplicationId appId;
+    private Set<DeviceId> addedDevices = new HashSet<>();
+
     @Activate
     public void activate() {
@@ -156,9 +164,9 @@ public class RestDeviceProvider extends AbstractProvider
     public boolean isReachable(DeviceId deviceId) {
         RestSBDevice restDevice = controller.getDevice(deviceId);
         if (restDevice == null) {
-            log.warn("BAD REQUEST: the requested device id: " +
+            log.debug("the requested device id: " +
-                             deviceId.toString() +
+                              deviceId.toString() +
-                             "  is not associated to any REST Device");
+                              "  is not associated to any REST Device");
             return false;
         }
         return restDevice.isActive();
@@ -181,6 +189,7 @@ public class RestDeviceProvider extends AbstractProvider
         providerService.deviceConnected(deviceId, deviceDescription);
         nodeId.setActive(true);
         controller.addDevice(nodeId);
+        addedDevices.add(deviceId);
     }
     //when do I call it ?
@@ -212,7 +221,7 @@ public class RestDeviceProvider extends AbstractProvider
             log.error("Configuration error {}", e);
         }
         log.debug("REST Devices {}", controller.getDevices());
-        controller.getDevices().keySet().forEach(deviceId -> {
+        addedDevices.forEach(deviceId -> {
             DriverHandler h = driverService.createHandler(deviceId);
             PortDiscovery portConfig = h.behaviour(PortDiscovery.class);
             if (portConfig != null) {
@@ -221,6 +230,8 @@ public class RestDeviceProvider extends AbstractProvider
                 log.warn("No portGetter behaviour for device {}", deviceId);
             }
         });
+        addedDevices.clear();
+
     }
     private boolean testDeviceConnection(RestSBDevice device) {
@@ -240,8 +251,9 @@ public class RestDeviceProvider extends AbstractProvider
             } else {
                 urlConn = (HttpURLConnection) url.openConnection();
             }
-            if (device.password() != null) {
+            if (device.username() != null) {
-                String userPassword = device.name() + ":" + device.password();
+                String pwd = device.password() == null ? "" : ":" + device.password();
+                String userPassword = device.username() + pwd;
                 String basicAuth = Base64.getEncoder()
                         .encodeToString(userPassword.getBytes(StandardCharsets.UTF_8));
                 urlConn.setRequestProperty(AUTHORIZATION_PROPERTY, BASIC_AUTH_PREFIX + basicAuth);
@@ -267,7 +279,7 @@ public class RestDeviceProvider extends AbstractProvider
         @Override
         public void event(NetworkConfigEvent event) {
-            connectDevices();
+            executor.submit(RestDeviceProvider.this::connectDevices);
         }
         @Override
--- a/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestProviderConfig.java
View file @784ee0f
+++ b/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestProviderConfig.java
View file @784ee0f
@@ -39,7 +39,7 @@ public class RestProviderConfig extends Config<ApplicationId> {
     private static final String IP = "ip";
     private static final int DEFAULT_HTTP_PORT = 80;
     private static final String PORT = "port";
-    private static final String NAME = "name";
+    private static final String USERNAME = "username";
     private static final String PASSWORD = "password";
     private static final String PROTOCOL = "protocol";
     private static final String URL = "url";
@@ -52,11 +52,11 @@ public class RestProviderConfig extends Config<ApplicationId> {
                 String ip = node.path(IP).asText();
                 IpAddress ipAddr = ip.isEmpty() ? null : IpAddress.valueOf(ip);
                 int port = node.path(PORT).asInt(DEFAULT_HTTP_PORT);
-                String name = node.path(NAME).asText();
+                String username = node.path(USERNAME).asText();
                 String password = node.path(PASSWORD).asText();
                 String protocol = node.path(PROTOCOL).asText();
                 String url = node.path(URL).asText();
-                devicesAddresses.add(new DefaultRestSBDevice(ipAddr, port, name,
+                devicesAddresses.add(new DefaultRestSBDevice(ipAddr, port, username,
                                                              password, protocol,
                                                              url, false));