From: Robert Varga Date: Tue, 4 Jul 2023 16:13:47 +0000 (+0200) Subject: Do not use AbstractNetconfTopology in netconf-topology-singleton X-Git-Tag: v6.0.0~7 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=6e2319272c880a4c5b85b6017e62a9f05fadfac2;p=netconf.git 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 --- 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 32354b7b66..ea407fd9ed 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/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 dbef0a2ed7..677e807314 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 @@ -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 6f22d004e1..04e452f783 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 @@ -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 3ec5445cc6..5b94b55533 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 @@ -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 6309225425..931f716167 100644 --- 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 84dddf9825..9fa81131f1 100644 --- 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> 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(); }