From: Peter Puškár Date: Thu, 13 Oct 2022 13:25:41 +0000 (+0200) Subject: Add the concept of a netconf client name X-Git-Tag: v4.0.3~5 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=netconf.git;a=commitdiff_plain;h=30bedf0e68631bea63c64cb996dbcba633626bd3 Add the concept of a netconf client name Clients can be optionally named with a simple string, this is then propagated to AsyncSshHandler and used for logging purposes. JIRA: NETCONF-896 Change-Id: Iebb6c094c50ff4646b8ef3e9221cebd3dbebe5e3 Signed-off-by: Zdenko Filip Signed-off-by: Jaroslav Tóth Signed-off-by: Peter Puškár Signed-off-by: Robert Varga --- diff --git a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientDispatcherImpl.java b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientDispatcherImpl.java index ac7ecfe438..ef8fb66d57 100644 --- a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientDispatcherImpl.java +++ b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientDispatcherImpl.java @@ -91,7 +91,8 @@ public class NetconfClientDispatcherImpl return super.createClient(currentConfiguration.getAddress(), currentConfiguration.getReconnectStrategy(), (ch, sessionPromise) -> new SshClientChannelInitializer(currentConfiguration.getAuthHandler(), getNegotiatorFactory(currentConfiguration), currentConfiguration.getSessionListener(), - currentConfiguration.getSshClient()).initialize(ch, sessionPromise)); + currentConfiguration.getSshClient(), currentConfiguration.getName()) + .initialize(ch, sessionPromise)); } private ReconnectFuture createReconnectingSshClient( @@ -99,7 +100,7 @@ public class NetconfClientDispatcherImpl LOG.debug("Creating reconnecting SSH client with configuration: {}", currentConfiguration); final SshClientChannelInitializer init = new SshClientChannelInitializer(currentConfiguration.getAuthHandler(), getNegotiatorFactory(currentConfiguration), currentConfiguration.getSessionListener(), - currentConfiguration.getSshClient()); + currentConfiguration.getSshClient(), currentConfiguration.getName()); return super.createReconnectingClient(currentConfiguration.getAddress(), currentConfiguration.getConnectStrategyFactory(), init::initialize); diff --git a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/SshClientChannelInitializer.java b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/SshClientChannelInitializer.java index 81f2d00b97..3f2cf22042 100644 --- a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/SshClientChannelInitializer.java +++ b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/SshClientChannelInitializer.java @@ -16,26 +16,30 @@ import org.opendaylight.netconf.nettyutil.handler.ssh.client.NetconfSshClient; final class SshClientChannelInitializer extends AbstractClientChannelInitializer { private final AuthenticationHandler authenticationHandler; - private final NetconfSshClient sshClient; + private final @Nullable NetconfSshClient sshClient; + private final @Nullable String name; SshClientChannelInitializer(final AuthenticationHandler authHandler, final NetconfClientSessionNegotiatorFactory negotiatorFactory, - final NetconfClientSessionListener sessionListener, @Nullable final NetconfSshClient sshClient) { + final NetconfClientSessionListener sessionListener, final @Nullable NetconfSshClient sshClient, + final @Nullable String name) { super(negotiatorFactory, sessionListener); authenticationHandler = authHandler; this.sshClient = sshClient; + this.name = name; } SshClientChannelInitializer(final AuthenticationHandler authHandler, final NetconfClientSessionNegotiatorFactory negotiatorFactory, final NetconfClientSessionListener sessionListener) { - this(authHandler, negotiatorFactory, sessionListener, null); + this(authHandler, negotiatorFactory, sessionListener, null, null); } @Override public void initialize(final Channel ch, final Promise promise) { // ssh handler has to be the first handler in pipeline - ch.pipeline().addFirst(AsyncSshHandler.createForNetconfSubsystem(authenticationHandler, promise, sshClient)); + var asyncHandler = AsyncSshHandler.createForNetconfSubsystem(authenticationHandler, promise, sshClient, name); + ch.pipeline().addFirst(asyncHandler); super.initialize(ch, promise); } } diff --git a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfClientConfiguration.java b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfClientConfiguration.java index c1edb5abc8..1a7338ce8c 100644 --- a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfClientConfiguration.java +++ b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfClientConfiguration.java @@ -43,6 +43,7 @@ public class NetconfClientConfiguration { private final List odlHelloCapabilities; private final @NonNegative int maximumIncomingChunkSize; + private final String name; NetconfClientConfiguration(final NetconfClientProtocol protocol, final InetSocketAddress address, final Long connectionTimeoutMillis, @@ -50,7 +51,8 @@ public class NetconfClientConfiguration { final NetconfClientSessionListener sessionListener, final ReconnectStrategy reconnectStrategy, final AuthenticationHandler authHandler, final SslHandlerFactory sslHandlerFactory, final NetconfSshClient sshClient, - final List odlHelloCapabilities, final @NonNegative int maximumIncomingChunkSize) { + final List odlHelloCapabilities, final @NonNegative int maximumIncomingChunkSize, + final String name) { this.address = address; this.connectionTimeoutMillis = connectionTimeoutMillis; this.additionalHeader = additionalHeader; @@ -62,9 +64,14 @@ public class NetconfClientConfiguration { this.sshClient = sshClient; this.odlHelloCapabilities = odlHelloCapabilities; this.maximumIncomingChunkSize = maximumIncomingChunkSize; + this.name = name; validateConfiguration(); } + public final String getName() { + return name; + } + public final InetSocketAddress getAddress() { return address; } diff --git a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfClientConfigurationBuilder.java b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfClientConfigurationBuilder.java index 09002ab1e4..16aa5c467a 100644 --- a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfClientConfigurationBuilder.java +++ b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfClientConfigurationBuilder.java @@ -39,6 +39,7 @@ public class NetconfClientConfigurationBuilder { private List odlHelloCapabilities; private @NonNegative int maximumIncomingChunkSize = AbstractNetconfSessionNegotiator.DEFAULT_MAXIMUM_INCOMING_CHUNK_SIZE; + private String name; protected NetconfClientConfigurationBuilder() { } @@ -103,6 +104,12 @@ public class NetconfClientConfigurationBuilder { return this; } + @SuppressWarnings("checkstyle:hiddenField") + public NetconfClientConfigurationBuilder withName(final String name) { + this.name = name; + return this; + } + @SuppressWarnings("checkstyle:hiddenField") public NetconfClientConfigurationBuilder withOdlHelloCapabilities(final List odlHelloCapabilities) { this.odlHelloCapabilities = odlHelloCapabilities; @@ -161,9 +168,13 @@ public class NetconfClientConfigurationBuilder { return maximumIncomingChunkSize; } + final String getName() { + return name; + } + public NetconfClientConfiguration build() { return new NetconfClientConfiguration(clientProtocol, address, connectionTimeoutMillis, additionalHeader, sessionListener, reconnectStrategy, authHandler, sslHandlerFactory, sshClient, odlHelloCapabilities, - maximumIncomingChunkSize); + maximumIncomingChunkSize, name); } } diff --git a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfReconnectingClientConfiguration.java b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfReconnectingClientConfiguration.java index e9d4889d9b..1612b50eb1 100644 --- a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfReconnectingClientConfiguration.java +++ b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfReconnectingClientConfiguration.java @@ -23,7 +23,6 @@ import org.opendaylight.netconf.nettyutil.handler.ssh.client.NetconfSshClient; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Uri; public final class NetconfReconnectingClientConfiguration extends NetconfClientConfiguration { - private final ReconnectStrategyFactory connectStrategyFactory; NetconfReconnectingClientConfiguration(final NetconfClientProtocol clientProtocol, final InetSocketAddress address, @@ -36,9 +35,10 @@ public final class NetconfReconnectingClientConfiguration extends NetconfClientC final SslHandlerFactory sslHandlerFactory, final NetconfSshClient sshClient, final List odlHelloCapabilities, - final @NonNegative int maximumIncomingChunkSize) { + final @NonNegative int maximumIncomingChunkSize, + final String name) { super(clientProtocol, address, connectionTimeoutMillis, additionalHeader, sessionListener, reconnectStrategy, - authHandler, sslHandlerFactory, sshClient, odlHelloCapabilities, maximumIncomingChunkSize); + authHandler, sslHandlerFactory, sshClient, odlHelloCapabilities, maximumIncomingChunkSize, name); this.connectStrategyFactory = connectStrategyFactory; validateReconnectConfiguration(); } diff --git a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfReconnectingClientConfigurationBuilder.java b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfReconnectingClientConfigurationBuilder.java index 177f452f3d..a42d5f2cdd 100644 --- a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfReconnectingClientConfigurationBuilder.java +++ b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/conf/NetconfReconnectingClientConfigurationBuilder.java @@ -41,7 +41,7 @@ public final class NetconfReconnectingClientConfigurationBuilder extends Netconf return new NetconfReconnectingClientConfiguration(getProtocol(), getAddress(), getConnectionTimeoutMillis(), getAdditionalHeader(), getSessionListener(), getReconnectStrategy(), connectStrategyFactory, getAuthHandler(), getSslHandlerFactory(), getSshClient(), getOdlHelloCapabilities(), - getMaximumIncomingChunkSize()); + getMaximumIncomingChunkSize(), getName()); } // Override setter methods to return subtype @@ -99,6 +99,11 @@ public final class NetconfReconnectingClientConfigurationBuilder extends Netconf return (NetconfReconnectingClientConfigurationBuilder) super.withSshClient(sshClient); } + @Override + public NetconfReconnectingClientConfigurationBuilder withName(final String name) { + return (NetconfReconnectingClientConfigurationBuilder) super.withName(name); + } + @Override public NetconfReconnectingClientConfigurationBuilder withOdlHelloCapabilities( final List odlHelloCapabilities) { diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java index b659187b87..d30ec087be 100644 --- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java +++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java @@ -73,6 +73,7 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { private final AuthenticationHandler authenticationHandler; private final Future negotiationFuture; private final NetconfSshClient sshClient; + private final String name; // Initialized by connect() @GuardedBy("this") @@ -85,11 +86,17 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { private volatile boolean disconnected; - public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient, - final Future negotiationFuture) { + private AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient, + final @Nullable Future negotiationFuture, final @Nullable String name) { this.authenticationHandler = requireNonNull(authenticationHandler); this.sshClient = requireNonNull(sshClient); this.negotiationFuture = negotiationFuture; + this.name = name != null && !name.isBlank() ? name : "UNNAMED"; + } + + public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient, + final @Nullable Future negotiationFuture) { + this(authenticationHandler, sshClient, negotiationFuture, null); } /** @@ -115,9 +122,10 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { * @return {@code AsyncSshHandler} */ public static AsyncSshHandler createForNetconfSubsystem(final AuthenticationHandler authenticationHandler, - final Future negotiationFuture, final @Nullable NetconfSshClient sshClient) { + final Future negotiationFuture, final @Nullable NetconfSshClient sshClient, + final @Nullable String name) { return new AsyncSshHandler(authenticationHandler, sshClient != null ? sshClient : DEFAULT_CLIENT, - negotiationFuture); + negotiationFuture, name); } @Override @@ -128,7 +136,7 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { @Override public synchronized void connect(final ChannelHandlerContext ctx, final SocketAddress remoteAddress, final SocketAddress localAddress, final ChannelPromise promise) throws IOException { - LOG.debug("SSH session connecting on channel {}. promise: {}", ctx.channel(), promise); + LOG.debug("{}: SSH session connecting on channel {}. promise: {}", name, ctx.channel(), promise); connectPromise = requireNonNull(promise); if (negotiationFuture != null) { @@ -141,7 +149,7 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { negotiationFuture.addListener(negotiationFutureListener); } - LOG.debug("Starting SSH to {} on channel: {}", remoteAddress, ctx.channel()); + LOG.debug("{}: Starting SSH to {} on channel: {}", name, remoteAddress, ctx.channel()); sshClient.connect(authenticationHandler.getUsername(), remoteAddress) // FIXME: this is a blocking call, we should handle this with a concurrently-scheduled timeout. We do not // have a Timer ready, so perhaps we should be using the event loop? @@ -157,7 +165,7 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { } final var clientSession = connectFuture.getSession(); - LOG.trace("SSH session {} created on channel: {}", clientSession, ctx.channel()); + LOG.trace("{}: SSH session {} created on channel: {}", name, clientSession, ctx.channel()); verify(clientSession instanceof NettyAwareClientSession, "Unexpected session %s", clientSession); final var localSession = (NettyAwareClientSession) clientSession; @@ -182,11 +190,11 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { return; } if (disconnected) { - LOG.debug("Skipping SSH subsystem allocation, channel: {}", ctx.channel()); + LOG.debug("{}: Skipping SSH subsystem allocation, channel: {}", name, ctx.channel()); return; } - LOG.debug("SSH session authenticated on channel: {}, server version: {}", ctx.channel(), + LOG.debug("{}: SSH session authenticated on channel: {}, server version: {}", name, ctx.channel(), clientSession.getServerVersion()); final OpenFuture openFuture; @@ -212,11 +220,11 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { return; } if (disconnected) { - LOG.trace("Skipping activation, channel: {}", ctx.channel()); + LOG.trace("{}: Skipping activation, channel: {}", name, ctx.channel()); return; } - LOG.trace("SSH subsystem channel opened successfully on channel: {}", ctx.channel()); + LOG.trace("{}: SSH subsystem channel opened successfully on channel: {}", name, ctx.channel()); if (negotiationFuture == null) { connectPromise.setSuccess(); } @@ -228,7 +236,7 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { @Holding("this") private void onOpenFailure(final ChannelHandlerContext ctx, final Throwable cause) { - LOG.warn("Unable to setup SSH connection on channel: {}", ctx.channel(), cause); + LOG.warn("{}: Unable to setup SSH connection on channel: {}", name, ctx.channel(), cause); // If the promise is not yet done, we have failed with initial connect and set connectPromise to failure if (!connectPromise.isDone()) { @@ -255,7 +263,7 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { // the channel's executor. @SuppressWarnings("checkstyle:IllegalCatch") private synchronized void safelyDisconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) { - LOG.trace("Closing SSH session on channel: {} with connect promise in state: {}", ctx.channel(), + LOG.trace("{}: Closing SSH session on channel: {} with connect promise in state: {}", name, ctx.channel(), connectPromise); // If we have already succeeded and the session was dropped after, @@ -300,7 +308,7 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { // Disconnect has to be closed after inactive channel event was fired, because it interferes with it super.disconnect(ctx, ctx.newPromise()); } catch (final Exception e) { - LOG.warn("Unable to cleanup all resources for channel: {}. Ignoring.", ctx.channel(), e); + LOG.warn("{}: Unable to cleanup all resources for channel: {}. Ignoring.", name, ctx.channel(), e); } if (channel != null) { @@ -310,6 +318,6 @@ public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { channel = null; } promise.setSuccess(); - LOG.debug("SSH session closed on channel: {}", ctx.channel()); + LOG.debug("{}: SSH session closed on channel: {}", name, ctx.channel()); } } diff --git a/netconf/netconf-topology-impl/src/test/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImplTest.java b/netconf/netconf-topology-impl/src/test/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImplTest.java index b3ffa6ea71..e8c43a533e 100644 --- a/netconf/netconf-topology-impl/src/test/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImplTest.java +++ b/netconf/netconf-topology-impl/src/test/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImplTest.java @@ -211,27 +211,27 @@ public class NetconfTopologyImplTest { .setConnectionTimeoutMillis(Uint32.valueOf(20000)); final NetconfReconnectingClientConfiguration configuration = - spyTopology.getClientConfig(sessionListener, nodeBuilder.setTcpOnly(true).build()); + spyTopology.getClientConfig(sessionListener, nodeBuilder.setTcpOnly(true).build(), NODE_ID); assertEquals(NetconfClientConfiguration.NetconfClientProtocol.TCP, configuration.getProtocol()); assertNotNull(configuration.getAuthHandler()); assertNull(configuration.getSslHandlerFactory()); final NetconfReconnectingClientConfiguration configuration2 = - spyTopology.getClientConfig(sessionListener, nodeBuilder.setTcpOnly(false).build()); + spyTopology.getClientConfig(sessionListener, nodeBuilder.setTcpOnly(false).build(), NODE_ID); assertEquals(NetconfClientConfiguration.NetconfClientProtocol.SSH, configuration2.getProtocol()); assertNotNull(configuration2.getAuthHandler()); assertNull(configuration2.getSslHandlerFactory()); final NetconfReconnectingClientConfiguration configuration3 = spyTopology.getClientConfig(sessionListener, nodeBuilder - .setProtocol(new ProtocolBuilder().setName(Name.SSH).build()).build()); + .setProtocol(new ProtocolBuilder().setName(Name.SSH).build()).build(), NODE_ID); assertEquals(NetconfClientConfiguration.NetconfClientProtocol.SSH, configuration3.getProtocol()); assertNotNull(configuration3.getAuthHandler()); assertNull(configuration3.getSslHandlerFactory()); final NetconfReconnectingClientConfiguration configuration4 = spyTopology.getClientConfig(sessionListener, nodeBuilder - .setProtocol(new ProtocolBuilder().setName(Name.TLS).build()).build()); + .setProtocol(new ProtocolBuilder().setName(Name.TLS).build()).build(), NODE_ID); assertEquals(NetconfClientConfiguration.NetconfClientProtocol.TLS, configuration4.getProtocol()); assertNull(configuration4.getAuthHandler()); assertNotNull(configuration4.getSslHandlerFactory()); diff --git a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java index 740ad3a869..2a47a126f4 100644 --- a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java +++ b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java @@ -171,7 +171,7 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { final NetconfDeviceCommunicator deviceCommunicator = deviceCommunicatorDTO.getCommunicator(); final NetconfClientSessionListener netconfClientSessionListener = deviceCommunicatorDTO.getSessionListener(); final NetconfReconnectingClientConfiguration clientConfig = - getClientConfig(netconfClientSessionListener, netconfNode); + getClientConfig(netconfClientSessionListener, netconfNode, nodeId); final ListenableFuture future = deviceCommunicator.initializeRemoteConnection(clientDispatcher, clientConfig); @@ -298,7 +298,7 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { } public NetconfReconnectingClientConfiguration getClientConfig(final NetconfClientSessionListener listener, - final NetconfNode node) { + final NetconfNode node, final NodeId nodeId) { final ReconnectStrategyFactory sf = new TimedReconnectStrategyFactory(eventExecutor, node.requireMaxConnectionAttempts().toJava(), node.requireBetweenAttemptsTimeoutMillis().toJava(), node.requireSleepFactor().decimalValue()); @@ -326,6 +326,7 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { } return reconnectingClientConfigurationBuilder + .withName(nodeId.getValue()) .withAddress(NetconfNodeUtils.toInetSocketAddress(node)) .withConnectionTimeoutMillis(node.requireConnectionTimeoutMillis().toJava()) .withReconnectStrategy(sf.createReconnectStrategy())