From: Robert Varga Date: Sun, 25 Feb 2024 14:01:46 +0000 (+0100) Subject: NetconfClientFactoryImpl should propagate stack failure X-Git-Tag: v7.0.0~10 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=b51c3f7500a1e46d18ab65621a6db4780a9a83dc;p=netconf.git 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 Signed-off-by: Robert Varga --- 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 52d2cbf201..750567d751 100644 --- 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 { +final class ClientTransportChannelListener implements TransportChannelListener, FutureListener, + FutureCallback { private static final Logger LOG = LoggerFactory.getLogger(ClientTransportChannelListener.class); private final @NonNull SettableFuture 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 940da73b68..66714af59d 100644 --- 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 index 0000000000..b1c2204134 --- /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: "); + } +}