NetconfClientFactoryImpl should propagate stack failure

createClient() is ignoring failures to set up the transport stack,
leading to the future not completing on, for example, refused
connection.

Fix this by making ClientTransportChannelListener also a FutureCallback,
routing such failures to the returned future.

JIRA: NETCONF-1252
Change-Id: I3abc1faed94ee793ac6116eaca56a3cf473d8261
Signed-off-by: Oleksandr Zharov <oleksandr.zharov@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/protocol/netconf-client/src/main/java/org/opendaylight/netconf/client/ClientTransportChannelListener.java b/protocol/netconf-client/src/main/java/org/opendaylight/netconf/client/ClientTransportChannelListener.java
index 52d2cbf201255e4a8819d95bf88819a03617fe31..750567d7513a1b2c7554b289f87315250058a61b 100644 (file)
--- a/protocol/netconf-client/src/main/java/org/opendaylight/netconf/client/ClientTransportChannelListener.java
+++ b/protocol/netconf-client/src/main/java/org/opendaylight/netconf/client/ClientTransportChannelListener.java
@@ -9,6 +9,7 @@ package org.opendaylight.netconf.client;
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
 import io.netty.util.concurrent.Future;
@@ -16,10 +17,12 @@ import io.netty.util.concurrent.FutureListener;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.netconf.transport.api.TransportChannel;
 import org.opendaylight.netconf.transport.api.TransportChannelListener;
+import org.opendaylight.netconf.transport.api.TransportStack;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-final class ClientTransportChannelListener implements TransportChannelListener, FutureListener<NetconfClientSession> {
+final class ClientTransportChannelListener implements TransportChannelListener, FutureListener<NetconfClientSession>,
+        FutureCallback<TransportStack> {
     private static final Logger LOG = LoggerFactory.getLogger(ClientTransportChannelListener.class);
 
     private final @NonNull SettableFuture<NetconfClientSession> sessionFuture = SettableFuture.create();
@@ -56,4 +59,14 @@ final class ClientTransportChannelListener implements TransportChannelListener,
             sessionFuture.set(future.getNow());
         }
     }
+
+    @Override
+    public void onSuccess(final TransportStack result) {
+        // No-op
+    }
+
+    @Override
+    public void onFailure(final Throwable cause) {
+        onTransportChannelFailed(cause);
+    }
 }
\ No newline at end of file

diff --git a/protocol/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientFactoryImpl.java b/protocol/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientFactoryImpl.java
index 940da73b68ae2b736964e52dfe68cdf8573d81a7..66714af59d654ddc518cd1f301f60b58db4ca285 100644 (file)
--- a/protocol/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientFactoryImpl.java
+++ b/protocol/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientFactoryImpl.java
@@ -10,7 +10,9 @@ package org.opendaylight.netconf.client;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.MoreExecutors;
 import javax.annotation.PreDestroy;
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -28,14 +30,10 @@ import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Deactivate;
 import org.osgi.service.component.annotations.Reference;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 @Singleton
 @Component(service = NetconfClientFactory.class)
 public final class NetconfClientFactoryImpl implements NetconfClientFactory {
-    private static final Logger LOG = LoggerFactory.getLogger(NetconfClientFactoryImpl.class);
-
     private final SSHTransportStackFactory factory;
     private final NetconfTimer timer;
 
@@ -78,7 +76,7 @@ public final class NetconfClientFactoryImpl implements NetconfClientFactory {
                     handlerFactory);
             }
         };
-        LOG.trace("Future stack is {}", stackFuture);
+        Futures.addCallback(stackFuture, transportListener, MoreExecutors.directExecutor());
 
         return transportListener.sessionFuture();
     }

diff --git a/protocol/netconf-client/src/test/java/org/opendaylight/netconf/client/NC1252Test.java b/protocol/netconf-client/src/test/java/org/opendaylight/netconf/client/NC1252Test.java
new file mode 100644 (file)
index 0000000..b1c2204
--- /dev/null
+++ b/protocol/netconf-client/src/test/java/org/opendaylight/netconf/client/NC1252Test.java
@@ -0,0 +1,76 @@
+/*
+ * Copyright (c) 2024 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.netconf.client;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.net.ConnectException;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.opendaylight.netconf.client.conf.NetconfClientConfiguration;
+import org.opendaylight.netconf.client.conf.NetconfClientConfigurationBuilder;
+import org.opendaylight.netconf.common.impl.DefaultNetconfTimer;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Host;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IetfInetUtil;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.client.rev240208.netconf.client.initiate.stack.grouping.transport.tls.tls.TcpClientParametersBuilder;
+import org.opendaylight.yangtools.yang.common.Uint16;
+
+@ExtendWith(MockitoExtension.class)
+class NC1252Test {
+    private static DefaultNetconfTimer TIMER;
+    private static NetconfClientFactoryImpl FACTORY;
+
+    @Mock
+    private NetconfClientSessionListener sessionListener;
+
+    @BeforeAll
+    static void beforeAll() {
+        TIMER = new DefaultNetconfTimer();
+        FACTORY = new NetconfClientFactoryImpl(TIMER);
+    }
+
+    @AfterAll
+    static void afterAll() {
+        FACTORY.close();
+        TIMER.close();
+    }
+
+    @Test
+    void tcpClientNoServer() throws Exception {
+        // Find a free local port
+        final int localPort;
+        try (var socket = new ServerSocket(0)) {
+            localPort = socket.getLocalPort();
+        }
+
+        final var clientConfig = NetconfClientConfigurationBuilder.create()
+            .withProtocol(NetconfClientConfiguration.NetconfClientProtocol.TCP)
+            .withTcpParameters(new TcpClientParametersBuilder()
+                .setRemoteAddress(new Host(IetfInetUtil.ipAddressFor(InetAddress.getLoopbackAddress())))
+                .setRemotePort(new PortNumber(Uint16.valueOf(localPort)))
+                .build())
+            .withSessionListener(sessionListener)
+            .build();
+        final var future = FACTORY.createClient(clientConfig);
+
+        final var ex = assertThrows(ExecutionException.class, () -> future.get(1, TimeUnit.SECONDS)).getCause();
+        assertInstanceOf(ConnectException.class, ex);
+        assertThat(ex.getMessage()).startsWith("Connection refused: ");
+    }
+}