From 385845de6156ba424528d1504bad7f3e5dca00fd Mon Sep 17 00:00:00 2001 From: Peter Suna Date: Thu, 8 Jun 2023 13:20:56 +0200 Subject: [PATCH] Fix Broken refresh in network-topology-singleton Add missed functionality from patch: https://git.opendaylight.org/gerrit/c/netconf/+/104177 Revert NetconfTopologyManagerTest. Use refresh method instead of instantiateServiceInstance method. JIRA: NETCONF-1046 Change-Id: I351e4248928f3c49253a8ccefc414ee8f1a104ce Signed-off-by: Peter Suna Signed-off-by: Ivan Hrasko --- .../impl/NetconfTopologyContext.java | 18 +++++++++- .../impl/NetconfTopologyManager.java | 8 ++--- .../impl/NetconfTopologySingletonImpl.java | 33 +++++++++++++++++-- .../impl/MountPointEndToEndTest.java | 13 ++++---- .../impl/NetconfTopologyManagerTest.java | 21 +++++++++--- 5 files changed, 73 insertions(+), 20 deletions(-) diff --git a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java index c086eaea20..91f4ecf405 100644 --- a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java +++ b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java @@ -40,9 +40,11 @@ class NetconfTopologyContext implements ClusterSingletonService, AutoCloseable { private final @NonNull ServiceGroupIdentifier serviceGroupIdent; private final NetconfTopologySingletonImpl topologySingleton; - private final RemoteDeviceId remoteDeviceId; private volatile boolean closed; + private volatile boolean isMaster; + + private RemoteDeviceId remoteDeviceId; NetconfTopologyContext(final String topologyId, final NetconfClientDispatcher clientDispatcher, final EventExecutor eventExecutor, final ScheduledThreadPool keepaliveExecutor, @@ -64,6 +66,7 @@ class NetconfTopologyContext implements ClusterSingletonService, AutoCloseable { @VisibleForTesting protected NetconfTopologySingletonImpl getTopologySingleton() { + // FIXME we have to access topology singleton via this method because of mocking in MountPointEndToEndTest return topologySingleton; } @@ -74,6 +77,7 @@ class NetconfTopologyContext implements ClusterSingletonService, AutoCloseable { LOG.warn("Instance is already closed."); return; } + isMaster = true; getTopologySingleton().becomeTopologyLeader(); } @@ -90,6 +94,18 @@ class NetconfTopologyContext implements ClusterSingletonService, AutoCloseable { return FluentFutures.immediateNullFluentFuture(); } + void refresh(final @NonNull NetconfTopologySetup setup) { + final var node = requireNonNull(setup).getNode(); + remoteDeviceId = NetconfNodeUtils.toRemoteDeviceId(node.getNodeId(), node.augmentation(NetconfNode.class)); + + if (isMaster) { + getTopologySingleton().disconnectNode(setup.getNode().getNodeId()); + getTopologySingleton().refreshSetupConnection(setup, remoteDeviceId); + } else { + getTopologySingleton().refreshDevice(setup, remoteDeviceId); + } + } + @Override public ServiceGroupIdentifier getIdentifier() { return serviceGroupIdent; diff --git a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java index 2889966fcf..21a1b489c7 100644 --- a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java +++ b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java @@ -157,12 +157,12 @@ public class NetconfTopologyManager switch (rootNode.getModificationType()) { case SUBTREE_MODIFIED: LOG.debug("Config for node {} updated", nodeId); - refreshNetconfDeviceContext(dataModifIdent); + refreshNetconfDeviceContext(dataModifIdent, rootNode.getDataAfter()); break; case WRITE: if (contexts.containsKey(dataModifIdent)) { LOG.debug("RemoteDevice{{}} was already configured, reconfiguring node...", nodeId); - refreshNetconfDeviceContext(dataModifIdent); + refreshNetconfDeviceContext(dataModifIdent, rootNode.getDataAfter()); } else { LOG.debug("Config for node {} created", nodeId); startNetconfDeviceContext(dataModifIdent, rootNode.getDataAfter()); @@ -178,9 +178,9 @@ public class NetconfTopologyManager } } - private void refreshNetconfDeviceContext(final InstanceIdentifier instanceIdentifier) { + private void refreshNetconfDeviceContext(final InstanceIdentifier instanceIdentifier, final Node node) { final NetconfTopologyContext context = contexts.get(instanceIdentifier); - context.closeServiceInstance().addListener(context::instantiateServiceInstance, MoreExecutors.directExecutor()); + context.refresh(createSetup(instanceIdentifier, node)); } // ClusterSingletonServiceRegistration registerClusterSingletonService method throws a Runtime exception if there diff --git a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologySingletonImpl.java b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologySingletonImpl.java index 5befd9f20c..a5a8c4342f 100644 --- a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologySingletonImpl.java +++ b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologySingletonImpl.java @@ -9,6 +9,8 @@ package org.opendaylight.netconf.topology.singleton.impl; import akka.actor.ActorRef; import akka.cluster.Cluster; +import akka.dispatch.OnComplete; +import akka.pattern.Patterns; import akka.util.Timeout; import io.netty.util.concurrent.EventExecutor; import org.opendaylight.aaa.encrypt.AAAEncryptionService; @@ -27,9 +29,14 @@ import org.opendaylight.netconf.client.mdsal.api.SslHandlerFactoryProvider; import org.opendaylight.netconf.topology.singleton.impl.actors.NetconfNodeActor; import org.opendaylight.netconf.topology.singleton.impl.utils.NetconfTopologySetup; import org.opendaylight.netconf.topology.singleton.impl.utils.NetconfTopologyUtils; +import org.opendaylight.netconf.topology.singleton.messages.RefreshSetupMasterActorData; import org.opendaylight.netconf.topology.spi.AbstractNetconfTopology; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; final class NetconfTopologySingletonImpl extends AbstractNetconfTopology implements AutoCloseable { + private static final Logger LOG = LoggerFactory.getLogger(NetconfTopologySingletonImpl.class); + private final RemoteDeviceId remoteDeviceId; private final NetconfTopologySetup setup; private final Timeout actorResponseWaitTime; @@ -70,13 +77,33 @@ final class NetconfTopologySingletonImpl extends AbstractNetconfTopology impleme } void becomeTopologyFollower() { + registerNodeManager(); + // disconnect device from this node and listen for changes from leader + disconnectNode(setup.getNode().getNodeId()); if (masterActorRef != null) { // was leader before setup.getActorSystem().stop(masterActorRef); } - // disconnect device from this node and listen for changes from leader - disconnectNode(setup.getNode().getNodeId()); - registerNodeManager(); + } + + void refreshSetupConnection(final NetconfTopologySetup netconfTopologyDeviceSetup, final RemoteDeviceId device) { + Patterns.ask(masterActorRef, new RefreshSetupMasterActorData(netconfTopologyDeviceSetup, device), + actorResponseWaitTime).onComplete( + new OnComplete<>() { + @Override + public void onComplete(final Throwable failure, final Object success) { + if (failure != null) { + LOG.error("Failed to refresh master actor data", failure); + return; + } + LOG.debug("Succeed to refresh Master Action data. Creating Connector..."); + setupConnection(setup.getNode().getNodeId(), setup.getNode()); + } + }, netconfTopologyDeviceSetup.getActorSystem().dispatcher()); + } + + void refreshDevice(final NetconfTopologySetup netconfTopologyDeviceSetup, final RemoteDeviceId deviceId) { + netconfNodeManager.refreshDevice(netconfTopologyDeviceSetup, deviceId); } private void registerNodeManager() { diff --git a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java index 3f8c95ac2b..3f943fb7b3 100644 --- a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java +++ b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java @@ -330,15 +330,15 @@ public class MountPointEndToEndTest extends AbstractBaseSchemasTest { final var context = super.newNetconfTopologyContext(setup, serviceGroupIdent, actorResponseWaitTime, deviceActionFact); final var spiedContext = spy(context); - final var spiedTopology = spy(context.getTopologySingleton()); + final var spiedSingleton = spy(context.getTopologySingleton()); doAnswer(invocation -> { final var spiedFacade = (MasterSalFacade) spy(invocation.callRealMethod()); doReturn(deviceDOMDataBroker).when(spiedFacade) .newDeviceDataBroker(any(MountPointContext.class), any(NetconfSessionPreferences.class)); masterSalFacadeFuture.set(spiedFacade); return spiedFacade; - }).when(spiedTopology).createSalFacade(any(RemoteDeviceId.class), any(boolean.class)); - doReturn(spiedTopology).when(spiedContext).getTopologySingleton(); + }).when(spiedSingleton).createSalFacade(any(RemoteDeviceId.class), any(boolean.class)); + doReturn(spiedSingleton).when(spiedContext).getTopologySingleton(); return spiedContext; } }; @@ -395,14 +395,13 @@ public class MountPointEndToEndTest extends AbstractBaseSchemasTest { @Test public void test() throws Exception { - var masterSalFacade = testMaster(); + testMaster(); testSlave(); - testMasterDisconnected(masterSalFacade); + final MasterSalFacade masterSalFacade = testMasterNodeUpdated(); - // FIXME NETCONF-1046 - // masterSalFacade = testMasterNodeUpdated(); + testMasterDisconnected(masterSalFacade); testCleanup(); } diff --git a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java index aa42a75600..42875390d7 100644 --- a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java +++ b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java @@ -36,6 +36,7 @@ import java.util.function.Function; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import org.opendaylight.aaa.encrypt.AAAEncryptionService; @@ -78,7 +79,6 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology. import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeBuilder; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey; import org.opendaylight.yangtools.concepts.ListenerRegistration; -import org.opendaylight.yangtools.util.concurrent.FluentFutures; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.binding.YangModuleInfo; import org.opendaylight.yangtools.yang.common.Uint16; @@ -248,11 +248,18 @@ public class NetconfTopologyManagerTest extends AbstractBaseSchemasTest { // Notify of Node 1 replaced and Node 2 subtree modified. mockContextMap.clear(); + final NetconfNode updatedNetconfNode1 = new NetconfNodeBuilder(netconfNode1) + .setPort(new PortNumber(Uint16.valueOf(33333))).build(); + final Node updatedNode1 = new NodeBuilder().setNodeId(nodeId1).addAugmentation(updatedNetconfNode1).build(); + doReturn(WRITE).when(dataObjectModification1).getModificationType(); + doReturn(updatedNode1).when(dataObjectModification1).getDataAfter(); + doReturn(SUBTREE_MODIFIED).when(dataObjectModification2).getModificationType(); + doReturn(node2).when(dataObjectModification2).getDataAfter(); - doReturn(FluentFutures.immediateNullFluentFuture()).when(mockContext1).closeServiceInstance(); - doReturn(FluentFutures.immediateNullFluentFuture()).when(mockContext2).closeServiceInstance(); + doNothing().when(mockContext1).refresh(any()); + doNothing().when(mockContext2).refresh(any()); netconfTopologyManager.onDataTreeChanged(Arrays.asList( new CustomTreeModification(DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, @@ -260,8 +267,12 @@ public class NetconfTopologyManagerTest extends AbstractBaseSchemasTest { new CustomTreeModification(DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, nodeInstanceId2), dataObjectModification2))); - verify(mockContext1).instantiateServiceInstance(); - verify(mockContext2).instantiateServiceInstance(); + ArgumentCaptor mockContext1Setup = ArgumentCaptor.forClass(NetconfTopologySetup.class); + verify(mockContext1).refresh(mockContext1Setup.capture()); + assertEquals(updatedNode1, mockContext1Setup.getValue().getNode()); + + verify(mockContext2).refresh(any()); + verifyNoMoreInteractions(clusterSingletonServiceProvider); // Notify of Node 1 deleted. -- 2.36.6