Do not use AbstractNetconfTopology in netconf-topology-singleton

An AbstractNetconfTopology is meant to represent the entire topology,
do not misuse it to track single nodes.

JIRA: NETCONF-1039
Change-Id: If4e98f2b59bf92e90beed40df09e52fe8bacd7d7
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

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/NetconfNodeContext.java
similarity index 57%
rename from apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologySingletonImpl.java
rename to apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeContext.java
index 32354b7b66cd61e54721a62bdec0e555944e5efe..ea407fd9edc2f9045fcbee6fbfea832642b96fe5 100644 (file)
--- 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/NetconfNodeContext.java
@@ -7,11 +7,16 @@
  */
 package org.opendaylight.netconf.topology.singleton.impl;
 
+import static java.util.Objects.requireNonNull;
+
 import akka.actor.ActorRef;
 import akka.cluster.Cluster;
 import akka.dispatch.OnComplete;
 import akka.pattern.Patterns;
 import akka.util.Timeout;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.common.util.concurrent.MoreExecutors;
 import io.netty.util.concurrent.EventExecutor;
 import org.opendaylight.controller.config.threadpool.ScheduledThreadPool;
 import org.opendaylight.controller.config.threadpool.ThreadPool;
@@ -20,22 +25,32 @@ import org.opendaylight.mdsal.dom.api.DOMMountPointService;
 import org.opendaylight.netconf.client.NetconfClientDispatcher;
 import org.opendaylight.netconf.client.mdsal.api.BaseNetconfSchemas;
 import org.opendaylight.netconf.client.mdsal.api.DeviceActionFactory;
-import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceHandler;
 import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceId;
 import org.opendaylight.netconf.client.mdsal.api.SchemaResourceManager;
 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.opendaylight.netconf.topology.spi.NetconfClientConfigurationBuilderFactory;
-import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId;
+import org.opendaylight.netconf.topology.spi.NetconfNodeHandler;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.node.optional.rev221225.NetconfNodeAugmentedOptional;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.node.topology.rev221225.NetconfNode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-final class NetconfTopologySingletonImpl extends AbstractNetconfTopology implements AutoCloseable {
-    private static final Logger LOG = LoggerFactory.getLogger(NetconfTopologySingletonImpl.class);
-
+final class NetconfNodeContext implements AutoCloseable {
+    private static final Logger LOG = LoggerFactory.getLogger(NetconfNodeContext.class);
+
+    private final NetconfClientDispatcher clientDispatcher;
+    private final EventExecutor eventExecutor;
+    private final DeviceActionFactory deviceActionFactory;
+    private final SchemaResourceManager schemaManager;
+    private final BaseNetconfSchemas baseSchemas;
+    private final NetconfClientConfigurationBuilderFactory builderFactory;
+    private final ScheduledThreadPool keepaliveExecutor;
+    private final ListeningExecutorService processingExecutor;
+    private final DataBroker dataBroker;
+    private final DOMMountPointService mountPointService;
     private final RemoteDeviceId remoteDeviceId;
     private final NetconfTopologySetup setup;
     private final Timeout actorResponseWaitTime;
@@ -43,19 +58,28 @@ final class NetconfTopologySingletonImpl extends AbstractNetconfTopology impleme
     private ActorRef masterActorRef;
     private MasterSalFacade masterSalFacade;
     private NetconfNodeManager netconfNodeManager;
+    private NetconfNodeHandler nodeHandler;
 
-    NetconfTopologySingletonImpl(final String topologyId, final NetconfClientDispatcher clientDispatcher,
-            final EventExecutor eventExecutor, final ScheduledThreadPool keepaliveExecutor,
-            final ThreadPool processingExecutor, final SchemaResourceManager schemaManager,
-            final DataBroker dataBroker, final DOMMountPointService mountPointService,
-            final NetconfClientConfigurationBuilderFactory builderFactory,
+    NetconfNodeContext(final NetconfClientDispatcher clientDispatcher, final EventExecutor eventExecutor,
+            final ScheduledThreadPool keepaliveExecutor, final ThreadPool processingExecutor,
+            final SchemaResourceManager schemaManager, final DataBroker dataBroker,
+            final DOMMountPointService mountPointService, final NetconfClientConfigurationBuilderFactory builderFactory,
             final DeviceActionFactory deviceActionFactory, final BaseNetconfSchemas baseSchemas,
             final RemoteDeviceId remoteDeviceId, final NetconfTopologySetup setup,
             final Timeout actorResponseWaitTime) {
-        super(topologyId, clientDispatcher, eventExecutor, keepaliveExecutor, processingExecutor, schemaManager,
-                dataBroker, mountPointService, builderFactory, deviceActionFactory, baseSchemas);
-        this.remoteDeviceId = remoteDeviceId;
-        this.setup = setup;
+        this.clientDispatcher = requireNonNull(clientDispatcher);
+        this.eventExecutor = requireNonNull(eventExecutor);
+        this.keepaliveExecutor = requireNonNull(keepaliveExecutor);
+        // FIXME: share a single instance!
+        this.processingExecutor = MoreExecutors.listeningDecorator(processingExecutor.getExecutor());
+        this.schemaManager = requireNonNull(schemaManager);
+        this.dataBroker = requireNonNull(dataBroker);
+        this.mountPointService = requireNonNull(mountPointService);
+        this.builderFactory = requireNonNull(builderFactory);
+        this.deviceActionFactory = deviceActionFactory;
+        this.baseSchemas = requireNonNull(baseSchemas);
+        this.remoteDeviceId = requireNonNull(remoteDeviceId);
+        this.setup = requireNonNull(setup);
         this.actorResponseWaitTime = actorResponseWaitTime;
         registerNodeManager();
     }
@@ -70,14 +94,15 @@ final class NetconfTopologySingletonImpl extends AbstractNetconfTopology impleme
                 actorResponseWaitTime, mountPointService), NetconfTopologyUtils.createMasterActorName(
                 remoteDeviceId.name(), masterAddress));
 
-        // setup connection to device
-        ensureNode(setup.getNode());
+        connectNode();
     }
 
     void becomeTopologyFollower() {
         registerNodeManager();
+
         // disconnect device from this node and listen for changes from leader
-        deleteNode(setup.getNode().getNodeId());
+        dropNode();
+
         if (masterActorRef != null) {
             // was leader before
             setup.getActorSystem().stop(masterActorRef);
@@ -85,6 +110,8 @@ final class NetconfTopologySingletonImpl extends AbstractNetconfTopology impleme
     }
 
     void refreshSetupConnection(final NetconfTopologySetup netconfTopologyDeviceSetup, final RemoteDeviceId device) {
+        dropNode();
+
         Patterns.ask(masterActorRef, new RefreshSetupMasterActorData(netconfTopologyDeviceSetup, device),
             actorResponseWaitTime).onComplete(
                 new OnComplete<>() {
@@ -95,7 +122,7 @@ final class NetconfTopologySingletonImpl extends AbstractNetconfTopology impleme
                             return;
                         }
                         LOG.debug("Succeed to refresh Master Action data. Creating Connector...");
-                        setupConnection(setup.getNode().getNodeId(), setup.getNode());
+                        connectNode();
                     }
                 }, netconfTopologyDeviceSetup.getActorSystem().dispatcher());
     }
@@ -113,10 +140,6 @@ final class NetconfTopologySingletonImpl extends AbstractNetconfTopology impleme
         netconfNodeManager.close();
     }
 
-    void dropNode(final NodeId nodeId) {
-        deleteNode(nodeId);
-    }
-
     @Override
     public void close() {
         unregisterNodeManager();
@@ -130,10 +153,34 @@ final class NetconfTopologySingletonImpl extends AbstractNetconfTopology impleme
         }
     }
 
-    @Override
-    public RemoteDeviceHandler createSalFacade(final RemoteDeviceId deviceId, final boolean lockDatastore) {
-        masterSalFacade = new MasterSalFacade(deviceId, setup.getActorSystem(), masterActorRef,
-                actorResponseWaitTime, mountPointService, dataBroker, lockDatastore);
-        return masterSalFacade;
+    private void connectNode() {
+        final var configNode = setup.getNode();
+
+        final var netconfNode = configNode.augmentation(NetconfNode.class);
+        final var nodeOptional = configNode.augmentation(NetconfNodeAugmentedOptional.class);
+
+        requireNonNull(netconfNode.getHost());
+        requireNonNull(netconfNode.getPort());
+
+        // Instantiate the handler ...
+        masterSalFacade = createSalFacade(netconfNode.requireLockDatastore());
+
+        nodeHandler = new NetconfNodeHandler(clientDispatcher, eventExecutor, keepaliveExecutor.getExecutor(),
+            baseSchemas, schemaManager, processingExecutor, builderFactory, deviceActionFactory, masterSalFacade,
+            remoteDeviceId, configNode.getNodeId(), netconfNode, nodeOptional);
+        nodeHandler.connect();
+    }
+
+    private void dropNode() {
+        if (nodeHandler != null) {
+            nodeHandler.close();
+            nodeHandler = null;
+        }
+    }
+
+    @VisibleForTesting
+    MasterSalFacade createSalFacade(final boolean lockDatastore) {
+        return new MasterSalFacade(remoteDeviceId, setup.getActorSystem(), masterActorRef, actorResponseWaitTime,
+            mountPointService, dataBroker, lockDatastore);
     }
 }

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 dbef0a2ed75241e31f0b740b71732b5cc2cdbb49..677e807314a4bee135c80334dc2b31ca262eee15 100644 (file)
--- 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
@@ -37,14 +37,14 @@ class NetconfTopologyContext implements ClusterSingletonService, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(NetconfTopologyContext.class);
 
     private final @NonNull ServiceGroupIdentifier serviceGroupIdent;
-    private final NetconfTopologySingletonImpl topologySingleton;
+    private final NetconfNodeContext topologySingleton;
 
     private volatile boolean closed;
     private volatile boolean isMaster;
 
     private RemoteDeviceId remoteDeviceId;
 
-    NetconfTopologyContext(final String topologyId, final NetconfClientDispatcher clientDispatcher,
+    NetconfTopologyContext(final NetconfClientDispatcher clientDispatcher,
             final EventExecutor eventExecutor, final ScheduledThreadPool keepaliveExecutor,
             final ThreadPool processingExecutor, final SchemaResourceManager schemaManager,
             final DataBroker dataBroker, final DOMMountPointService mountPointService,
@@ -56,13 +56,13 @@ class NetconfTopologyContext implements ClusterSingletonService, AutoCloseable {
         remoteDeviceId = NetconfNodeUtils.toRemoteDeviceId(setup.getNode().getNodeId(),
                 setup.getNode().augmentation(NetconfNode.class));
 
-        topologySingleton = new NetconfTopologySingletonImpl(topologyId, clientDispatcher,
+        topologySingleton = new NetconfNodeContext(clientDispatcher,
                 eventExecutor, keepaliveExecutor, processingExecutor, schemaManager, dataBroker, mountPointService,
                 builderFactory, deviceActionFactory, baseSchemas, remoteDeviceId, setup, actorResponseWaitTime);
     }
 
     @VisibleForTesting
-    protected NetconfTopologySingletonImpl getTopologySingleton() {
+    protected NetconfNodeContext getTopologySingleton() {
         // FIXME we have to access topology singleton via this method because of mocking in MountPointEndToEndTest
         return topologySingleton;
     }
@@ -95,11 +95,11 @@ class NetconfTopologyContext implements ClusterSingletonService, AutoCloseable {
         final var node = requireNonNull(setup).getNode();
         remoteDeviceId = NetconfNodeUtils.toRemoteDeviceId(node.getNodeId(), node.augmentation(NetconfNode.class));
 
+        final var singleton = getTopologySingleton();
         if (isMaster) {
-            getTopologySingleton().dropNode(setup.getNode().getNodeId());
-            getTopologySingleton().refreshSetupConnection(setup, remoteDeviceId);
+            singleton.refreshSetupConnection(setup, remoteDeviceId);
         } else {
-            getTopologySingleton().refreshDevice(setup, remoteDeviceId);
+            singleton.refreshDevice(setup, remoteDeviceId);
         }
     }
 

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 6f22d004e1177638dcd38a051b4bce9844645d39..04e452f78335a0a6459b9f5eb6e3790107333d4d 100644 (file)
--- 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
@@ -230,7 +230,7 @@ public class NetconfTopologyManager
     protected NetconfTopologyContext newNetconfTopologyContext(final NetconfTopologySetup setup,
             final ServiceGroupIdentifier serviceGroupIdent, final Timeout actorResponseWaitTime,
             final DeviceActionFactory deviceActionFact) {
-        return new NetconfTopologyContext(setup.getTopologyId(), setup.getNetconfClientDispatcher(),
+        return new NetconfTopologyContext(setup.getNetconfClientDispatcher(),
                 setup.getEventExecutor(), keepaliveExecutor,
                 processingExecutor, resourceManager,
                 dataBroker, mountPointService,

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 3ec5445cc682b8c14581effda18a7d81812b9b0a..5b94b5553320c9c4ff6c963efcaf17210e007cda 100644 (file)
--- 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
@@ -103,7 +103,6 @@ import org.opendaylight.netconf.client.mdsal.NetconfDeviceSchema;
 import org.opendaylight.netconf.client.mdsal.api.CredentialProvider;
 import org.opendaylight.netconf.client.mdsal.api.DeviceActionFactory;
 import org.opendaylight.netconf.client.mdsal.api.NetconfSessionPreferences;
-import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceId;
 import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceServices;
 import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceServices.Rpcs;
 import org.opendaylight.netconf.client.mdsal.api.SchemaResourceManager;
@@ -338,7 +337,7 @@ public class MountPointEndToEndTest extends AbstractBaseSchemasTest {
                         .newDeviceDataBroker(any(MountPointContext.class), any(NetconfSessionPreferences.class));
                     masterSalFacadeFuture.set(spiedFacade);
                     return spiedFacade;
-                }).when(spiedSingleton).createSalFacade(any(RemoteDeviceId.class), any(boolean.class));
+                }).when(spiedSingleton).createSalFacade(any(boolean.class));
                 doReturn(spiedSingleton).when(spiedContext).getTopologySingleton();
                 return spiedContext;
             }

diff --git a/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java b/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java
index 630922542579e72b91e3dd11e4cf05add30d00bd..931f716167e8cc65d4f9b112ccb54d04fed69c9c 100644 (file)
--- a/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java
+++ b/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java
@@ -140,9 +140,8 @@ public abstract class AbstractNetconfTopology {
         final var deviceId = NetconfNodeUtils.toRemoteDeviceId(nodeId, netconfNode);
         final var deviceSalFacade = createSalFacade(deviceId, netconfNode.requireLockDatastore());
         final var nodeHandler = new NetconfNodeHandler(clientDispatcher, eventExecutor, keepaliveExecutor.getExecutor(),
-            baseSchemas, schemaManager, processingExecutor, deviceActionFactory,
-            deviceSalFacade, deviceId, nodeId, netconfNode, nodeOptional,
-            builderFactory.createClientConfigurationBuilder(nodeId, netconfNode));
+            baseSchemas, schemaManager, processingExecutor, builderFactory, deviceActionFactory, deviceSalFacade,
+            deviceId, nodeId, netconfNode, nodeOptional);
 
         // ... record it ...
         activeConnectors.put(nodeId, nodeHandler);

diff --git a/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandler.java b/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandler.java
index 84dddf98250603b99e6f5ca99838b7741c19ad4e..9fa81131f174a675f2f7ee4ce6510d1d31aa72fe 100644 (file)
--- a/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandler.java
+++ b/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandler.java
@@ -23,7 +23,6 @@ import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.dom.api.DOMNotification;
 import org.opendaylight.netconf.client.NetconfClientDispatcher;
 import org.opendaylight.netconf.client.conf.NetconfClientConfiguration;
-import org.opendaylight.netconf.client.conf.NetconfClientConfigurationBuilder;
 import org.opendaylight.netconf.client.mdsal.LibraryModulesSchemas;
 import org.opendaylight.netconf.client.mdsal.LibrarySchemaSourceProvider;
 import org.opendaylight.netconf.client.mdsal.NetconfDevice.SchemaResourcesDTO;
@@ -54,7 +53,7 @@ import org.slf4j.LoggerFactory;
 /**
  * All state associated with a NETCONF topology node. Each node handles its own reconnection.
  */
-final class NetconfNodeHandler extends AbstractRegistration implements RemoteDeviceHandler {
+public final class NetconfNodeHandler extends AbstractRegistration implements RemoteDeviceHandler {
     private static final Logger LOG = LoggerFactory.getLogger(NetconfNodeHandler.class);
 
     private final @NonNull List<SchemaSourceRegistration<?>> yanglibRegistrations;
@@ -76,12 +75,13 @@ final class NetconfNodeHandler extends AbstractRegistration implements RemoteDev
     @GuardedBy("this")
     private Future<?> currentTask;
 
-    NetconfNodeHandler(final NetconfClientDispatcher clientDispatcher, final EventExecutor eventExecutor,
+    public NetconfNodeHandler(final NetconfClientDispatcher clientDispatcher, final EventExecutor eventExecutor,
             final ScheduledExecutorService keepaliveExecutor, final BaseNetconfSchemas baseSchemas,
             final SchemaResourceManager schemaManager, final ListeningExecutorService processingExecutor,
+            final NetconfClientConfigurationBuilderFactory builderFactory,
             final DeviceActionFactory deviceActionFactory, final RemoteDeviceHandler delegate,
             final RemoteDeviceId deviceId, final NodeId nodeId, final NetconfNode node,
-            final NetconfNodeAugmentedOptional nodeOptional, final NetconfClientConfigurationBuilder configBuilder) {
+            final NetconfNodeAugmentedOptional nodeOptional) {
         this.clientDispatcher = requireNonNull(clientDispatcher);
         this.eventExecutor = requireNonNull(eventExecutor);
         this.delegate = requireNonNull(delegate);
@@ -140,10 +140,12 @@ final class NetconfNodeHandler extends AbstractRegistration implements RemoteDev
             keepAliveFacade.setListener(communicator);
         }
 
-        clientConfig = configBuilder.withSessionListener(communicator).build();
+        clientConfig = builderFactory.createClientConfigurationBuilder(nodeId, node)
+            .withSessionListener(communicator)
+            .build();
     }
 
-    synchronized void connect() {
+    public synchronized void connect() {
         lockedConnect();
     }