From a80115e1c14f3f8ecca9e25f67e06885fc1bf3c5 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sat, 5 Aug 2023 11:08:47 +0200 Subject: [PATCH] Clean up NetconfDevice shutdown When we fail to setup schemas, we should not be calling onRemoteSessionDown(), as it ends up emitting two changes to NetconfDeviceListener -- onDeviceDisconnected() and onDeviceFailed(). JIRA: NETCONF-1097 Change-Id: I1b9a3751accfa15791ac2c84a11ad31d51256f16 Signed-off-by: Robert Varga (cherry picked from commit c775691891f930cba1f6f920cadd1fb4cf61d31b) --- .../netconf/client/mdsal/NetconfDevice.java | 41 +++++-------------- .../client/mdsal/NetconfDeviceTest.java | 1 - 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDevice.java b/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDevice.java index c087d22e44..4c0bdeec22 100644 --- a/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDevice.java +++ b/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDevice.java @@ -104,9 +104,6 @@ public class NetconfDevice implements RemoteDevice { @GuardedBy("this") private boolean connected = false; - // Message transformer is constructed once the schemas are available - private NetconfMessageTransformer messageTransformer; - public NetconfDevice(final SchemaResourcesDTO schemaResourcesDTO, final BaseNetconfSchemas baseSchemas, final RemoteDeviceId id, final RemoteDeviceHandler salFacade, final Executor globalProcessingExecutor, final boolean reconnectOnSchemasChange) { @@ -132,11 +129,9 @@ public class NetconfDevice implements RemoteDevice { @Override public void onRemoteSessionUp(final NetconfSessionPreferences remoteSessionCapabilities, final NetconfDeviceCommunicator listener) { - // SchemaContext setup has to be performed in a dedicated thread since - // we are in a netty thread in this method - // Yang models are being downloaded in this method and it would cause a - // deadlock if we used the netty thread - // http://netty.io/wiki/thread-model.html + // SchemaContext setup has to be performed in a dedicated thread since we are in a Netty thread in this method + // YANG models are being downloaded in this method and it would cause a deadlock if we used the netty thread + // https://netty.io/wiki/thread-model.html setConnected(true); LOG.debug("{}: Session to remote device established with {}", id, remoteSessionCapabilities); @@ -221,7 +216,7 @@ public class NetconfDevice implements RemoteDevice { return; } - messageTransformer = new NetconfMessageTransformer(deviceSchema.mountContext(), true, + final var messageTransformer = new NetconfMessageTransformer(deviceSchema.mountContext(), true, resolveBaseSchema(remoteSessionCapabilities.isNotificationsSupported())); // Order is important here: salFacade has to see the device come up and then the notificationHandler can deliver @@ -235,26 +230,17 @@ public class NetconfDevice implements RemoteDevice { } private void handleSalInitializationFailure(final RemoteDeviceCommunicator listener, final Throwable cause) { - // FIXME: pick one of these messages LOG.warn("{}: Unexpected error resolving device sources", id, cause); - LOG.error("{}: Initialization in sal failed, disconnecting from device", id, cause); listener.close(); - onRemoteSessionDown(); - resetMessageTransformer(); - - // FIXME: this causes salFacade to see onDeviceDisconnected() and then onDeviceFailed(), which is quite weird + cleanupInitialization(); salFacade.onDeviceFailed(cause); } - /** - * Set the transformer to null as is in initial state. - */ - private void resetMessageTransformer() { - updateTransformer(null); - } - - private synchronized void updateTransformer(final NetconfMessageTransformer transformer) { - messageTransformer = transformer; + private synchronized void cleanupInitialization() { + connected = false; + notificationHandler.onRemoteSchemaDown(); + sourceRegistrations.forEach(Registration::close); + sourceRegistrations.clear(); } private synchronized void setConnected(final boolean connected) { @@ -305,13 +291,8 @@ public class NetconfDevice implements RemoteDevice { @Override public void onRemoteSessionDown() { - setConnected(false); - notificationHandler.onRemoteSchemaDown(); - + cleanupInitialization(); salFacade.onDeviceDisconnected(); - sourceRegistrations.forEach(Registration::close); - sourceRegistrations.clear(); - resetMessageTransformer(); } @Override diff --git a/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceTest.java b/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceTest.java index 03212717cb..d73d1e47ee 100644 --- a/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceTest.java +++ b/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceTest.java @@ -173,7 +173,6 @@ public class NetconfDeviceTest extends AbstractTestModelTest { final NetconfSessionPreferences sessionCaps = getSessionCaps(false, List.of(TEST_CAPABILITY)); device.onRemoteSessionUp(sessionCaps, listener); - verify(facade, timeout(5000)).onDeviceDisconnected(); final var captor = ArgumentCaptor.forClass(Throwable.class); verify(facade, timeout(5000)).onDeviceFailed(captor.capture()); assertThat(captor.getValue(), instanceOf(EmptySchemaContextException.class)); -- 2.36.6