Patch for ONOS-6840 NETCONF timeouts per device
Change-Id: Ia2e578245b97e0f68ea720cefe783e708e255ca7
diff --git a/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/NetconfSessionImpl.java b/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/NetconfSessionImpl.java
index 0f98c77..a2c6a3d 100644
--- a/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/NetconfSessionImpl.java
+++ b/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/NetconfSessionImpl.java
@@ -104,6 +104,9 @@
private boolean subscriptionConnected = false;
private String notificationFilterSchema = null;
+ private int connectTimeout;
+ private int replyTimeout;
+
public NetconfSessionImpl(NetconfDeviceInfo deviceInfo) throws NetconfException {
this.deviceInfo = deviceInfo;
@@ -112,13 +115,18 @@
connectionActive = false;
replies = new ConcurrentHashMap<>();
errorReplies = new ArrayList<>();
+ connectTimeout = deviceInfo.getConnectTimeoutSec().orElse(
+ NetconfControllerImpl.netconfConnectTimeout);
+ replyTimeout = deviceInfo.getReplyTimeoutSec().orElse(
+ NetconfControllerImpl.netconfReplyTimeout);
+ log.info("Connecting to {} with timeouts C:{}, R:{}. idle=connect", deviceInfo,
+ connectTimeout, replyTimeout);
startConnection();
}
private void startConnection() throws NetconfException {
if (!connectionActive) {
netconfConnection = new Connection(deviceInfo.ip().toString(), deviceInfo.port());
- int connectTimeout = NetconfControllerImpl.netconfConnectTimeout;
try {
netconfConnection.connect(null, 1000 * connectTimeout, 1000 * connectTimeout);
@@ -312,7 +320,6 @@
request = formatRequestMessageId(request, messageId);
request = formatXmlHeader(request);
CompletableFuture<String> futureReply = request(request, messageId);
- int replyTimeout = NetconfControllerImpl.netconfReplyTimeout;
String rp;
try {
rp = futureReply.get(replyTimeout, TimeUnit.SECONDS);
@@ -622,6 +629,24 @@
}
@Override
+ public int timeoutConnectSec() {
+ return connectTimeout;
+ }
+
+ @Override
+ public int timeoutReplySec() {
+ return replyTimeout;
+ }
+
+ /**
+ * Idle timeout is not settable on ETZ_SSH - the valuse used is the connect timeout.
+ */
+ @Override
+ public int timeoutIdleSec() {
+ return connectTimeout;
+ }
+
+ @Override
public void addDeviceOutputListener(NetconfDeviceOutputEventListener listener) {
streamHandler.addDeviceEventListener(listener);
}
diff --git a/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfControllerImpl.java b/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfControllerImpl.java
index 2265234..5e6d7f9 100644
--- a/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfControllerImpl.java
+++ b/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfControllerImpl.java
@@ -29,6 +29,7 @@
import org.onosproject.net.AnnotationKeys;
import org.onosproject.net.Device;
import org.onosproject.net.DeviceId;
+import org.onosproject.net.config.NetworkConfigRegistry;
import org.onosproject.net.device.DeviceService;
import org.onosproject.net.key.DeviceKey;
import org.onosproject.net.key.DeviceKeyId;
@@ -42,6 +43,8 @@
import org.onosproject.netconf.NetconfDeviceOutputEvent;
import org.onosproject.netconf.NetconfDeviceOutputEventListener;
import org.onosproject.netconf.NetconfException;
+import org.onosproject.netconf.config.NetconfDeviceConfig;
+import org.onosproject.netconf.config.NetconfSshClientLib;
import org.osgi.service.component.ComponentContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -55,8 +58,8 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
-import static com.google.common.base.Strings.isNullOrEmpty;
import static org.onlab.util.Tools.get;
+import static org.onlab.util.Tools.getIntegerProperty;
import static org.onlab.util.Tools.groupedThreads;
/**
@@ -87,10 +90,10 @@
protected static int netconfIdleTimeout = DEFAULT_IDLE_TIMEOUT_SECONDS;
private static final String SSH_LIBRARY = "sshLibrary";
- private static final String APACHE_MINA = "apache_mina";
- @Property(name = SSH_LIBRARY, value = APACHE_MINA,
+ private static final String APACHE_MINA_STR = "apache-mina";
+ @Property(name = SSH_LIBRARY, value = APACHE_MINA_STR,
label = "Ssh Library instead of apache_mina (i.e. ethz-ssh2")
- protected static String sshLibrary = APACHE_MINA;
+ protected static NetconfSshClientLib sshLibrary = NetconfSshClientLib.APACHE_MINA;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected ComponentConfigService cfgService;
@@ -101,6 +104,9 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected DeviceKeyService deviceKeyService;
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected NetworkConfigRegistry netCfgService;
+
public static final Logger log = LoggerFactory
.getLogger(NetconfControllerImpl.class);
@@ -139,36 +145,24 @@
if (context == null) {
netconfReplyTimeout = DEFAULT_REPLY_TIMEOUT_SECONDS;
netconfConnectTimeout = DEFAULT_CONNECT_TIMEOUT_SECONDS;
- sshLibrary = APACHE_MINA;
+ netconfIdleTimeout = DEFAULT_IDLE_TIMEOUT_SECONDS;
+ sshLibrary = NetconfSshClientLib.APACHE_MINA;
log.info("No component configuration");
return;
}
Dictionary<?, ?> properties = context.getProperties();
- int newNetconfReplyTimeout;
- int newNetconfConnectTimeout;
- int newNetconfIdleTimeout;
String newSshLibrary;
- try {
- String s = get(properties, PROP_NETCONF_REPLY_TIMEOUT);
- newNetconfReplyTimeout = isNullOrEmpty(s) ?
- netconfReplyTimeout : Integer.parseInt(s.trim());
- s = get(properties, PROP_NETCONF_CONNECT_TIMEOUT);
- newNetconfConnectTimeout = isNullOrEmpty(s) ?
- netconfConnectTimeout : Integer.parseInt(s.trim());
+ int newNetconfReplyTimeout = getIntegerProperty(
+ properties, PROP_NETCONF_REPLY_TIMEOUT, netconfReplyTimeout);
+ int newNetconfConnectTimeout = getIntegerProperty(
+ properties, PROP_NETCONF_CONNECT_TIMEOUT, netconfConnectTimeout);
+ int newNetconfIdleTimeout = getIntegerProperty(
+ properties, PROP_NETCONF_IDLE_TIMEOUT, netconfIdleTimeout);
- s = get(properties, PROP_NETCONF_IDLE_TIMEOUT);
- newNetconfIdleTimeout = isNullOrEmpty(s) ?
- netconfIdleTimeout : Integer.parseInt(s.trim());
-
- newSshLibrary = get(properties, SSH_LIBRARY);
-
- } catch (NumberFormatException e) {
- log.warn("Component configuration had invalid value", e);
- return;
- }
+ newSshLibrary = get(properties, SSH_LIBRARY);
if (newNetconfConnectTimeout < 0) {
log.warn("netconfConnectTimeout is invalid - less than 0");
@@ -184,8 +178,10 @@
netconfReplyTimeout = newNetconfReplyTimeout;
netconfConnectTimeout = newNetconfConnectTimeout;
netconfIdleTimeout = newNetconfIdleTimeout;
- sshLibrary = newSshLibrary;
- log.info("Settings: {} = {}, {} = {}, {} = {}",
+ if (newSshLibrary != null) {
+ sshLibrary = NetconfSshClientLib.getEnum(newSshLibrary);
+ }
+ log.info("Settings: {} = {}, {} = {}, {} = {}, {} = {}",
PROP_NETCONF_REPLY_TIMEOUT, netconfReplyTimeout,
PROP_NETCONF_CONNECT_TIMEOUT, netconfConnectTimeout,
PROP_NETCONF_IDLE_TIMEOUT, netconfIdleTimeout,
@@ -221,9 +217,17 @@
@Override
public NetconfDevice connectDevice(DeviceId deviceId) throws NetconfException {
+ NetconfDeviceConfig netCfg = netCfgService.getConfig(
+ deviceId, NetconfDeviceConfig.class);
+ NetconfDeviceInfo deviceInfo = null;
+
if (netconfDeviceMap.containsKey(deviceId)) {
log.debug("Device {} is already present", deviceId);
return netconfDeviceMap.get(deviceId);
+ } else if (netCfg != null) {
+ log.debug("Device {} is present in NetworkConfig", deviceId);
+ deviceInfo = new NetconfDeviceInfo(netCfg);
+
} else {
log.debug("Creating NETCONF device {}", deviceId);
Device device = deviceService.getDevice(deviceId);
@@ -249,7 +253,6 @@
try {
DeviceKey deviceKey = deviceKeyService.getDeviceKey(
DeviceKeyId.deviceKeyId(deviceId.toString()));
- NetconfDeviceInfo deviceInfo = null;
if (deviceKey.type() == DeviceKey.Type.USERNAME_PASSWORD) {
UsernamePassword usernamepasswd = deviceKey.asUsernamePassword();
@@ -271,13 +274,13 @@
} else {
log.error("Unknown device key for device {}", deviceId);
}
- NetconfDevice netconfDevicedevice = createDevice(deviceInfo);
- netconfDevicedevice.getSession().addDeviceOutputListener(downListener);
- return netconfDevicedevice;
} catch (NullPointerException e) {
throw new NetconfException("No Device Key for device " + deviceId, e);
}
}
+ NetconfDevice netconfDevicedevice = createDevice(deviceInfo);
+ netconfDevicedevice.getSession().addDeviceOutputListener(downListener);
+ return netconfDevicedevice;
}
@Override
@@ -341,10 +344,15 @@
@Override
public NetconfDevice createNetconfDevice(NetconfDeviceInfo netconfDeviceInfo)
throws NetconfException {
- if (sshLibrary.equals(ETHZ_SSH2)) {
+ if (NetconfSshClientLib.ETHZ_SSH2.equals(netconfDeviceInfo.sshClientLib()) ||
+ NetconfSshClientLib.ETHZ_SSH2.equals(sshLibrary)) {
+ log.info("Creating NETCONF session to {} with {}",
+ netconfDeviceInfo.name(), NetconfSshClientLib.ETHZ_SSH2);
return new DefaultNetconfDevice(netconfDeviceInfo,
- new NetconfSessionImpl.SshNetconfSessionFactory());
+ new NetconfSessionImpl.SshNetconfSessionFactory());
}
+ log.info("Creating NETCONF session to {} with {}",
+ netconfDeviceInfo.getDeviceId(), NetconfSshClientLib.APACHE_MINA);
return new DefaultNetconfDevice(netconfDeviceInfo);
}
}
@@ -372,8 +380,8 @@
} catch (NetconfException e) {
log.error("The SSH connection with device {} couldn't be " +
- "reestablished due to {}. " +
- "Marking the device as unreachable", e.getMessage());
+ "reestablished due to {}. " +
+ "Marking the device as unreachable", e.getMessage());
log.debug("Complete exception: ", e);
removeDevice(did);
}
diff --git a/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfSessionImpl.java b/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfSessionImpl.java
index 99ad5f2..959c807 100644
--- a/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfSessionImpl.java
+++ b/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfSessionImpl.java
@@ -136,6 +136,8 @@
private final Collection<NetconfSession> children =
new CopyOnWriteArrayList<>();
+ private int connectTimeout;
+ private int replyTimeout;
public NetconfSessionImpl(NetconfDeviceInfo deviceInfo) throws NetconfException {
this.deviceInfo = deviceInfo;
@@ -144,6 +146,7 @@
connectionActive = false;
replies = new ConcurrentHashMap<>();
errorReplies = new ArrayList<>();
+
startConnection();
}
@@ -160,9 +163,15 @@
}
private void startConnection() throws NetconfException {
+ connectTimeout = deviceInfo.getConnectTimeoutSec().orElse(
+ NetconfControllerImpl.netconfConnectTimeout);
+ replyTimeout = deviceInfo.getReplyTimeoutSec().orElse(
+ NetconfControllerImpl.netconfReplyTimeout);
+ log.debug("Connecting to {} with timeouts C:{}, R:{}. I:connect-timeout", deviceInfo,
+ connectTimeout, replyTimeout);
+
if (!connectionActive) {
netconfConnection = new Connection(deviceInfo.ip().toString(), deviceInfo.port());
- int connectTimeout = NetconfControllerImpl.netconfConnectTimeout;
try {
netconfConnection.connect(null, 1000 * connectTimeout, 1000 * connectTimeout);
@@ -431,16 +440,18 @@
request = formatXmlHeader(request);
request = formatRequestMessageId(request, messageId);
CompletableFuture<String> futureReply = request(request, messageId);
- int replyTimeout = NetconfControllerImpl.netconfReplyTimeout;
String rp;
try {
+ log.debug("Sending request to NETCONF with timeout {} for {}",
+ replyTimeout, deviceInfo.name());
rp = futureReply.get(replyTimeout, TimeUnit.SECONDS);
replies.remove(messageId);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new NetconfException("Interrupted waiting for reply for request" + request, e);
} catch (TimeoutException e) {
- throw new NetconfException("Timed out waiting for reply for request " + request, e);
+ throw new NetconfException("Timed out waiting for reply for request " +
+ request + " after " + replyTimeout + " sec.", e);
} catch (ExecutionException e) {
log.warn("Closing session {} for {} due to unexpected Error", sessionID, deviceInfo, e);
@@ -790,7 +801,6 @@
onosCapabilities = capabilities;
}
-
@Override
public void addDeviceOutputListener(NetconfDeviceOutputEventListener listener) {
streamHandler.addDeviceEventListener(listener);
@@ -798,6 +808,24 @@
}
@Override
+ public int timeoutConnectSec() {
+ return connectTimeout;
+ }
+
+ @Override
+ public int timeoutReplySec() {
+ return replyTimeout;
+ }
+
+ /**
+ * Idle timeout is not settable on ETZ_SSH - the valuse used is the connect timeout.
+ */
+ @Override
+ public int timeoutIdleSec() {
+ return connectTimeout;
+ }
+
+ @Override
public void removeDeviceOutputListener(NetconfDeviceOutputEventListener listener) {
primaryListeners.remove(listener);
streamHandler.removeDeviceEventListener(listener);
@@ -902,4 +930,4 @@
return new NetconfSessionImpl(netconfDeviceInfo);
}
}
-}
\ No newline at end of file
+}
diff --git a/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfSessionMinaImpl.java b/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfSessionMinaImpl.java
index 0e2e3ea..6d30314 100644
--- a/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfSessionMinaImpl.java
+++ b/protocols/netconf/ctl/src/main/java/org/onosproject/netconf/ctl/impl/NetconfSessionMinaImpl.java
@@ -143,6 +143,10 @@
private final Collection<NetconfSession> children =
new CopyOnWriteArrayList<>();
+ private int connectTimeout;
+ private int replyTimeout;
+ private int idleTimeout;
+
private ClientChannel channel = null;
private ClientSession session = null;
@@ -153,6 +157,7 @@
this.deviceInfo = deviceInfo;
replies = new ConcurrentHashMap<>();
errorReplies = new ArrayList<>();
+
startConnection();
}
@@ -165,6 +170,15 @@
}
private void startConnection() throws NetconfException {
+ connectTimeout = deviceInfo.getConnectTimeoutSec().orElse(
+ NetconfControllerImpl.netconfConnectTimeout);
+ replyTimeout = deviceInfo.getReplyTimeoutSec().orElse(
+ NetconfControllerImpl.netconfReplyTimeout);
+ idleTimeout = deviceInfo.getIdleTimeoutSec().orElse(
+ NetconfControllerImpl.netconfIdleTimeout);
+ log.info("Connecting to {} with timeouts C:{}, R:{}, I:{}", deviceInfo,
+ connectTimeout, replyTimeout, idleTimeout);
+
try {
startClient();
} catch (IOException e) {
@@ -174,11 +188,10 @@
private void startClient() throws IOException {
client = SshClient.setUpDefaultClient();
- int replyTimeoutSec = NetconfControllerImpl.netconfIdleTimeout;
client.getProperties().putIfAbsent(FactoryManager.IDLE_TIMEOUT,
- TimeUnit.SECONDS.toMillis(replyTimeoutSec));
+ TimeUnit.SECONDS.toMillis(idleTimeout));
client.getProperties().putIfAbsent(FactoryManager.NIO2_READ_TIMEOUT,
- TimeUnit.SECONDS.toMillis(replyTimeoutSec + 15L));
+ TimeUnit.SECONDS.toMillis(idleTimeout + 15L));
client.start();
client.setKeyPairProvider(new SimpleGeneratorHostKeyProvider());
startSession();
@@ -189,7 +202,7 @@
connectFuture = client.connect(deviceInfo.name(),
deviceInfo.ip().toString(),
deviceInfo.port())
- .verify(NetconfControllerImpl.netconfConnectTimeout, TimeUnit.SECONDS);
+ .verify(connectTimeout, TimeUnit.SECONDS);
session = connectFuture.getSession();
//Using the device ssh key if possible
if (deviceInfo.getKey() != null) {
@@ -213,7 +226,7 @@
} else {
session.addPasswordIdentity(deviceInfo.password());
}
- session.auth().verify(NetconfControllerImpl.netconfConnectTimeout, TimeUnit.SECONDS);
+ session.auth().verify(connectTimeout, TimeUnit.SECONDS);
Set<ClientSession.ClientSessionEvent> event = session.waitFor(
ImmutableSet.of(ClientSession.ClientSessionEvent.WAIT_AUTH,
ClientSession.ClientSessionEvent.CLOSED,
@@ -239,7 +252,7 @@
private void openChannel() throws IOException {
channel = session.createSubsystemChannel("netconf");
OpenFuture channelFuture = channel.open();
- if (channelFuture.await(NetconfControllerImpl.netconfConnectTimeout, TimeUnit.SECONDS)) {
+ if (channelFuture.await(connectTimeout, TimeUnit.SECONDS)) {
if (channelFuture.isOpened()) {
streamHandler = new NetconfStreamThread(channel.getInvertedOut(), channel.getInvertedIn(),
channel.getInvertedErr(), deviceInfo,
@@ -456,6 +469,21 @@
return streamHandler.sendMessage(request);
}
+ @Override
+ public int timeoutConnectSec() {
+ return connectTimeout;
+ }
+
+ @Override
+ public int timeoutReplySec() {
+ return replyTimeout;
+ }
+
+ @Override
+ public int timeoutIdleSec() {
+ return idleTimeout;
+ }
+
private CompletableFuture<String> request(String request, int messageId) {
return streamHandler.sendMessage(request, messageId);
}
@@ -474,16 +502,18 @@
request = formatXmlHeader(request);
request = formatRequestMessageId(request, messageId);
CompletableFuture<String> futureReply = request(request, messageId);
- int replyTimeout = NetconfControllerImpl.netconfReplyTimeout;
String rp;
try {
+ log.debug("Sending request to NETCONF with timeout {} for {}",
+ replyTimeout, deviceInfo.name());
rp = futureReply.get(replyTimeout, TimeUnit.SECONDS);
replies.remove(messageId); // Why here???
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new NetconfException("Interrupted waiting for reply for request" + request, e);
} catch (TimeoutException e) {
- throw new NetconfException("Timed out waiting for reply for request " + request, e);
+ throw new NetconfException("Timed out waiting for reply for request " +
+ request + " after " + replyTimeout + " sec.", e);
} catch (ExecutionException e) {
log.warn("Closing session {} for {} due to unexpected Error", sessionID, deviceInfo, e);
try {
@@ -951,4 +981,4 @@
return new NetconfSessionMinaImpl(netconfDeviceInfo);
}
}
-}
\ No newline at end of file
+}