ONOS-3575 Netconf connection exceptions refactoring and port number in netconf-cfg.json

Change-Id: I46771a1a3ce99b25c2aecd7ba1838f9f1614e789

@@ -27,6 +27,7 @@ import org.onosproject.netconf.NetconfDevice;
 import org.slf4j.Logger;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
@@ -49,10 +50,14 @@ public class NetconfControllerConfig extends AbstractHandlerBehaviour
         DeviceId ofDeviceId = handler.data().deviceId();
         Preconditions.checkNotNull(controller, "Netconf controller is null");
         List<ControllerInfo> controllers = new ArrayList<>();
+        try {
             controllers.addAll(XmlConfigParser.parseStreamControllers(XmlConfigParser.
                     loadXml(new ByteArrayInputStream(controller.
                             getDevicesMap().get(ofDeviceId).getSession().
                             getConfig("running").getBytes(StandardCharsets.UTF_8)))));
+        } catch (IOException e) {
+            log.error("Cannot comunicate to device {} ", ofDeviceId);
+        }
         return controllers;
     }
 
@@ -65,7 +70,9 @@ public class NetconfControllerConfig extends AbstractHandlerBehaviour
         try {
             NetconfDevice device = controller.getNetconfDevice(deviceId);
             log.warn("provider map {}", controller.getDevicesMap());
-            String config = XmlConfigParser.createControllersConfig(
+            String config = null;
+            try {
+                config = XmlConfigParser.createControllersConfig(
                         XmlConfigParser.loadXml(getClass().getResourceAsStream("controllers.xml")),
                         XmlConfigParser.loadXml(
                                 new ByteArrayInputStream(device.getSession()
@@ -74,10 +81,15 @@ public class NetconfControllerConfig extends AbstractHandlerBehaviour
                                                                          StandardCharsets.UTF_8))),
                         "running", "merge", "create", controllers
                 );
+            } catch (IOException e) {
+                log.error("Cannot comunicate to device {} , exception {}", deviceId, e.getMessage());
+            }
             device.getSession().editConfig(config.substring(config.indexOf("-->") + 3));
         } catch (NullPointerException e) {
             log.warn("No NETCONF device with requested parameters " + e);
             throw new NullPointerException("No NETCONF device with requested parameters " + e);
+        } catch (IOException e) {
+            log.error("Cannot comunicate to device {} , exception {}", deviceId, e.getMessage());
         }
 
     }

@@ -68,7 +68,7 @@ public class NetconfDeviceInfo {
      * @param password  the password for the device
      * @param ipAddress the ip address
      * @param port      the tcp port
-     * @param keyString the string cointaing the key.
+     * @param keyString the string containing the key.
      */
     public NetconfDeviceInfo(String name, String password, IpAddress ipAddress,
                              int port, String keyString) {

@@ -16,6 +16,7 @@
 
 package org.onosproject.netconf;
 
+import java.io.IOException;
 import java.util.List;
 
 /**
@@ -30,14 +31,14 @@ public interface NetconfSession {
      * @param request the XML containing the request to the server.
      * @return device running configuration
      */
-    String get(String request);
+    String get(String request) throws IOException;
 
     /**
      * Executes an RPC to the server.
      * @param request the XML containing the RPC for the server.
      * @return Server response or ERROR
      */
-    String doRPC(String request);
+    String doRPC(String request) throws IOException;
 
     /**
      * Retrives the specified configuration.
@@ -45,7 +46,7 @@ public interface NetconfSession {
      * @param targetConfiguration the type of configuration to retrieve.
      * @return specified configuration.
      */
-    String getConfig(String targetConfiguration);
+    String getConfig(String targetConfiguration) throws IOException;
 
     /**
      * Retrives part of the specivied configuration based on the filterSchema.
@@ -55,7 +56,8 @@ public interface NetconfSession {
      *                                  elements we are interested in
      * @return device running configuration.
      */
-    String getConfig(String targetConfiguration, String configurationFilterSchema);
+    String getConfig(String targetConfiguration, String configurationFilterSchema)
+            throws IOException;
 
     /**
      * Retrives part of the specified configuration based on the filterSchema.
@@ -64,7 +66,7 @@ public interface NetconfSession {
      * @return true if the configuration was edited correctly
      */
 
-    boolean editConfig(String newConfiguration);
+    boolean editConfig(String newConfiguration) throws IOException;
 
     /**
      * Copies the new configuration, an Url or a complete configuration xml tree
@@ -75,7 +77,8 @@ public interface NetconfSession {
      * @param newConfiguration    configuration to set
      * @return true if the configuration was copied correctly
      */
-    boolean copyConfig(String targetConfiguration, String newConfiguration);
+    boolean copyConfig(String targetConfiguration, String newConfiguration)
+            throws IOException;
 
     /**
      * Deletes part of the specified configuration based on the filterSchema.
@@ -83,28 +86,28 @@ public interface NetconfSession {
      * @param targetConfiguration the name of the configuration to delete
      * @return true if the configuration was copied correctly
      */
-    boolean deleteConfig(String targetConfiguration);
+    boolean deleteConfig(String targetConfiguration) throws IOException;
 
     /**
      * Locks the candidate configuration.
      *
      * @return true if successful.
      */
-    boolean lock();
+    boolean lock() throws IOException;
 
     /**
      * Unlocks the candidate configuration.
      *
      * @return true if successful.
      */
-    boolean unlock();
+    boolean unlock() throws IOException;
 
     /**
      * Closes the Netconf session with the device.
      * the first time it tries gracefully, then kills it forcefully
      * @return true if closed
      */
-    boolean close();
+    boolean close() throws IOException;
 
     /**
      * Gets the session ID of the Netconf session.

@@ -80,20 +80,19 @@ public class NetconfControllerImpl implements NetconfController {
 
     @Override
     public NetconfDevice getNetconfDevice(IpAddress ip, int port) {
-        NetconfDevice device = null;
         for (DeviceId info : netconfDeviceMap.keySet()) {
             if (IpAddress.valueOf(info.uri().getHost()).equals(ip) &&
                     info.uri().getPort() == port) {
                 return netconfDeviceMap.get(info);
             }
         }
-        return device;
+        return null;
     }
 
     @Override
     public NetconfDevice connectDevice(NetconfDeviceInfo deviceInfo) throws IOException {
         if (netconfDeviceMap.containsKey(deviceInfo.getDeviceId())) {
-            log.info("Device {} is already present");
+            log.warn("Device {} is already present", deviceInfo);
             return netconfDeviceMap.get(deviceInfo.getDeviceId());
         } else {
             log.info("Creating NETCONF device {}", deviceInfo);
@@ -104,7 +103,7 @@ public class NetconfControllerImpl implements NetconfController {
     @Override
     public void removeDevice(NetconfDeviceInfo deviceInfo) {
         if (netconfDeviceMap.containsKey(deviceInfo.getDeviceId())) {
-            log.warn("Device {} is not present");
+            log.warn("Device {} is not present", deviceInfo);
         } else {
             stopDevice(deviceInfo);
         }

@@ -19,6 +19,8 @@ package org.onosproject.netconf.ctl;
 import org.onosproject.netconf.NetconfDevice;
 import org.onosproject.netconf.NetconfDeviceInfo;
 import org.onosproject.netconf.NetconfSession;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 
@@ -27,10 +29,12 @@ import java.io.IOException;
  */
 public class NetconfDeviceImpl implements NetconfDevice {
 
+    public static final Logger log = LoggerFactory
+            .getLogger(NetconfSessionImpl.class);
+
     private NetconfDeviceInfo netconfDeviceInfo;
     private boolean deviceState = false;
     private NetconfSession netconfSession;
-    //private String config;
 
     public NetconfDeviceImpl(NetconfDeviceInfo deviceInfo) throws IOException {
         netconfDeviceInfo = deviceInfo;
@@ -40,7 +44,6 @@ public class NetconfDeviceImpl implements NetconfDevice {
             throw new IOException("Cannot create connection and session", e);
         }
         deviceState = true;
-        //config = netconfSession.getConfig("running");
     }
 
     @Override
@@ -56,7 +59,11 @@ public class NetconfDeviceImpl implements NetconfDevice {
     @Override
     public void disconnect() {
         deviceState = false;
+        try {
             netconfSession.close();
+        } catch (IOException e) {
+            log.warn("Cannot communicate with the device {} ", netconfDeviceInfo);
+        }
     }
 
     @Override

@@ -18,6 +18,7 @@ package org.onosproject.netconf.ctl;
 
 import ch.ethz.ssh2.Connection;
 import ch.ethz.ssh2.Session;
+import ch.ethz.ssh2.StreamGobbler;
 import com.google.common.base.Preconditions;
 import org.onosproject.netconf.NetconfDeviceInfo;
 import org.onosproject.netconf.NetconfSession;
@@ -29,8 +30,7 @@ import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.PrintWriter;
 import java.io.StringWriter;
-import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -50,11 +50,9 @@ public class NetconfSessionImpl implements NetconfSession {
     private BufferedReader bufferReader = null;
     private PrintWriter out = null;
     private int messageID = 0;
-
+    //TODO inject these capabilites from yang model provided by app
     private List<String> deviceCapabilities =
-            new ArrayList<>(
-                    Arrays.asList("urn:ietf:params:netconf:base:1.0"));
-
+            Collections.singletonList("urn:ietf:params:netconf:base:1.0");
     private String serverCapabilities;
     private String endpattern = "]]>]]>";
 
@@ -98,8 +96,8 @@ public class NetconfSessionImpl implements NetconfSession {
         try {
             sshSession = netconfConnection.openSession();
             sshSession.startSubSystem("netconf");
-            bufferReader = new BufferedReader(new InputStreamReader(
-                    sshSession.getStdout()));
+            bufferReader = new BufferedReader(new InputStreamReader(new StreamGobbler(
+                    sshSession.getStdout())));
             out = new PrintWriter(sshSession.getStdin());
             sendHello();
         } catch (IOException e) {
@@ -127,51 +125,50 @@ public class NetconfSessionImpl implements NetconfSession {
     }
 
     @Override
-    public String doRPC(String request) {
-        String reply = "ERROR";
-        try {
-            reply = doRequest(request);
-            if (checkReply(reply)) {
-                return reply;
-            } else {
-                return "ERROR " + reply;
-            }
-        } catch (IOException e) {
-            log.error("Problem in the reading from the SSH connection " + e);
-        }
-        return reply;
+    public String doRPC(String request) throws IOException {
+        String reply = doRequest(request);
+        return checkReply(reply) ? reply : "ERROR " + reply;
     }
 
     private String doRequest(String request) throws IOException {
-        log.info("sshState " + sshSession.getState() + "request" + request);
+        //log.info("sshState " + sshSession.getState() + "request" + request);
+        checkAndRestablishSession();
+        //log.info("sshState after" + sshSession.getState());
+        out.print(request);
+        out.flush();
+        messageID++;
+        return readOne();
+    }
+
+    private void checkAndRestablishSession() throws IOException {
         if (sshSession.getState() != 2) {
             try {
                 startSshSession();
             } catch (IOException e) {
                 log.info("the connection had to be reopened");
+                try {
                     startConnection();
+                } catch (IOException e2) {
+                    log.error("No connection {} for device, exception {}", netconfConnection, e2);
+                    throw new IOException(e.getMessage());
+                    //TODO remove device from ONOS
+                }
             }
-            sendHello();
         }
-        log.info("sshState after" + sshSession.getState());
-        out.print(request);
-        out.flush();
-        messageID++;
-        return readOne();
     }
 
     @Override
-    public String get(String request) {
+    public String get(String request) throws IOException {
         return doRPC(request);
     }
 
     @Override
-    public String getConfig(String targetConfiguration) {
+    public String getConfig(String targetConfiguration) throws IOException {
         return getConfig(targetConfiguration, null);
     }
 
     @Override
-    public String getConfig(String targetConfiguration, String configurationSchema) {
+    public String getConfig(String targetConfiguration, String configurationSchema) throws IOException {
         StringBuilder rpc = new StringBuilder("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
         rpc.append("<rpc message-id=\"" + messageID + "\"  "
                            + "xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\">\n");
@@ -187,30 +184,19 @@ public class NetconfSessionImpl implements NetconfSession {
         rpc.append("</get-config>\n");
         rpc.append("</rpc>\n");
         rpc.append(endpattern);
-        String reply = null;
-        try {
-            reply = doRequest(rpc.toString());
-        } catch (IOException e) {
-            e.printStackTrace();
-        }
-
-        return checkReply(reply) ? reply : null;
+        String reply = doRequest(rpc.toString());
+        return checkReply(reply) ? reply : "ERROR " + reply;
     }
 
     @Override
-    public boolean editConfig(String newConfiguration) {
+    public boolean editConfig(String newConfiguration) throws IOException {
         newConfiguration = newConfiguration + endpattern;
-        String reply = null;
-        try {
-            reply = doRequest(newConfiguration);
-        } catch (IOException e) {
-            e.printStackTrace();
-        }
-        return checkReply(reply);
+        return checkReply(doRequest(newConfiguration));
     }
 
     @Override
-    public boolean copyConfig(String targetConfiguration, String newConfiguration) {
+    public boolean copyConfig(String targetConfiguration, String newConfiguration)
+            throws IOException {
         newConfiguration = newConfiguration.trim();
         if (!newConfiguration.startsWith("<configuration>")) {
             newConfiguration = "<configuration>" + newConfiguration
@@ -229,18 +215,11 @@ public class NetconfSessionImpl implements NetconfSession {
         rpc.append("</copy-config>");
         rpc.append("</rpc>");
         rpc.append(endpattern);
-        String reply = null;
-        try {
-            reply = doRequest(rpc.toString());
-        } catch (IOException e) {
-            e.printStackTrace();
-        }
-
-        return checkReply(reply);
+        return checkReply(doRequest(rpc.toString()));
     }
 
     @Override
-    public boolean deleteConfig(String targetConfiguration) {
+    public boolean deleteConfig(String targetConfiguration) throws IOException {
         if (targetConfiguration.equals("running")) {
             log.warn("Target configuration for delete operation can't be \"running\"",
                      targetConfiguration);
@@ -256,18 +235,11 @@ public class NetconfSessionImpl implements NetconfSession {
         rpc.append("</delete-config>");
         rpc.append("</rpc>");
         rpc.append(endpattern);
-        String reply = null;
-        try {
-            reply = doRequest(rpc.toString());
-        } catch (IOException e) {
-            e.printStackTrace();
-        }
-
-        return checkReply(reply);
+        return checkReply(doRequest(rpc.toString()));
     }
 
     @Override
-    public boolean lock() {
+    public boolean lock() throws IOException {
         StringBuilder rpc = new StringBuilder("<?xml version=\"1.0\" " +
                                                       "encoding=\"UTF-8\"?>");
         rpc.append("<rpc>");
@@ -278,17 +250,11 @@ public class NetconfSessionImpl implements NetconfSession {
         rpc.append("</lock>");
         rpc.append("</rpc>");
         rpc.append(endpattern);
-        String reply = null;
-        try {
-            reply = doRequest(rpc.toString());
-        } catch (IOException e) {
-            e.printStackTrace();
-        }
-        return checkReply(reply);
+        return checkReply(doRequest(rpc.toString()));
     }
 
     @Override
-    public boolean unlock() {
+    public boolean unlock() throws IOException {
         StringBuilder rpc = new StringBuilder("<?xml version=\"1.0\" " +
                                                       "encoding=\"UTF-8\"?>");
         rpc.append("<rpc>");
@@ -299,21 +265,15 @@ public class NetconfSessionImpl implements NetconfSession {
         rpc.append("</unlock>");
         rpc.append("</rpc>");
         rpc.append(endpattern);
-        String reply = null;
-        try {
-            reply = doRequest(rpc.toString());
-        } catch (IOException e) {
-            e.printStackTrace();
-        }
-        return checkReply(reply);
+        return checkReply(doRequest(rpc.toString()));
     }
 
     @Override
-    public boolean close() {
+    public boolean close() throws IOException {
         return close(false);
     }
 
-    private boolean close(boolean force) {
+    private boolean close(boolean force) throws IOException {
         StringBuilder rpc = new StringBuilder();
         rpc.append("<rpc>");
         if (force) {
@@ -324,7 +284,7 @@ public class NetconfSessionImpl implements NetconfSession {
         rpc.append("<close-configuration/>");
         rpc.append("</rpc>");
         rpc.append(endpattern);
-        return checkReply(rpc.toString()) ? true : close(true);
+        return checkReply(doRequest(rpc.toString())) || close(true);
     }
 
     @Override

@@ -40,7 +40,6 @@ import org.onosproject.net.device.DeviceDescription;
 import org.onosproject.net.device.DeviceProvider;
 import org.onosproject.net.device.DeviceProviderRegistry;
 import org.onosproject.net.device.DeviceProviderService;
-import org.onosproject.net.device.DeviceService;
 import org.onosproject.net.provider.AbstractProvider;
 import org.onosproject.net.provider.ProviderId;
 import org.onosproject.netconf.NetconfController;
@@ -66,8 +65,8 @@ public class NetconfDeviceProvider extends AbstractProvider
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected DeviceProviderRegistry providerRegistry;
 
-    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected DeviceService deviceService;
+    //    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+//    protected DeviceService deviceService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected NetconfController controller; //where is initiated ?
 
@@ -156,7 +155,6 @@ public class NetconfDeviceProvider extends AbstractProvider
         public void deviceAdded(NetconfDeviceInfo nodeId) {
             Preconditions.checkNotNull(nodeId, ISNOTNULL);
             DeviceId deviceId = nodeId.getDeviceId();
-            //TODO filter for not netconf devices
             //Netconf configuration object
             ChassisId cid = new ChassisId();
             String ipAddress = nodeId.ip().toString();
@@ -191,7 +189,6 @@ public class NetconfDeviceProvider extends AbstractProvider
     private void connectDevices() {
         NetconfProviderConfig cfg = cfgService.getConfig(appId, NetconfProviderConfig.class);
         if (cfg != null) {
-            log.info("cfg {}", cfg);
             try {
                 cfg.getDevicesAddresses().stream()
                         .forEach(addr -> {

@@ -89,5 +89,4 @@ public class NetconfProviderConfig extends Config<ApplicationId> {
         }
     }
 
-
 }

 {
   "devices":{
-    "netconf:mininet@10.1.9.24:1830":{
+    "netconf:mininet@10.1.9.24:830":{
       "basic":{
         "driver":"ovs-netconf"
       }
@@ -12,7 +12,7 @@
         "name":"mininet",
         "password":"mininet",
         "ip":"10.1.9.24",
-        "port":1830
+        "port":830
       }]
     }
   }