Fix Broken refresh in network-topology-singleton 25/106425/6
authorPeter Suna <[email protected]>
Thu, 8 Jun 2023 11:20:56 +0000 (13:20 +0200)
committerIvan Hrasko <[email protected]>
Thu, 15 Jun 2023 14:57:04 +0000 (16:57 +0200)
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 <[email protected]>
Signed-off-by: Ivan Hrasko <[email protected]>
apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java
apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java
apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologySingletonImpl.java
apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java
apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java

index c086eaea205ec33ade1d9b0a0bdffcf008385e8a..91f4ecf4056f5b2d528c1f4b358bcb30cf8ea2d6 100644 (file)
@@ -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;
index 2889966fcfb18d3c15d04c25867063a4b50e845e..21a1b489c72630210cb34681e23dfe902d5be86d 100644 (file)
@@ -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<Node> instanceIdentifier) {
+    private void refreshNetconfDeviceContext(final InstanceIdentifier<Node> 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
index 5befd9f20c2780394e33315c4943fa2d8eec40d7..a5a8c4342fa66beca83b1cf18eab301f3b293c27 100644 (file)
@@ -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() {
index 3f8c95ac2b2f390fdfc135001d92c7a5b65ee80f..3f943fb7b3b19ce0c0585618b125fa9ce9f7f94e 100644 (file)
@@ -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();
     }
index aa42a756006cc356be75bd7d52f47a9236f139a0..42875390d7ef8529e9286ffbe58cb7901a7adf01 100644 (file)
@@ -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<NetconfTopologySetup> 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.