From: Andrej Mak Date: Mon, 24 Apr 2017 13:08:27 +0000 (+0200) Subject: Bug 8197: Deregister schema sources on actor stop X-Git-Tag: release/carbon~12^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=8f72e44079758db1b2ce0db1e02f9e7a3f90a13c;p=netconf.git Bug 8197: Deregister schema sources on actor stop NetconfNodeActor registers schema source provider to schema registry. When mountpoint is removed, this registration should be removed too. If it isn't, following issue can happen: 1. Create mountpoint for device1 2. Master actor for device1 is registered as mod-1.yang provider 3. Delete device1 4. Create mountpoint for device2 5. Master actor for device2 is registered as mod-1.yang provider 6. Register slave - schemaContextFactory.createSchemaContext(sourceIdentifiers) is called 7. Since dead device1 master is still registered as provider, ask in ProxyYangTextSourceProvider timeouts 8. After timeout device2 master is queried 9. Device 2 slave mountpoint registered This delays slave mountpoint registration. Change-Id: I060c8b1988ba7b54f9a93d7eb37adb5c5e48b23b Signed-off-by: Andrej Mak --- diff --git a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/actors/NetconfNodeActor.java b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/actors/NetconfNodeActor.java index 8dd5e39d1e..fbd0422b69 100644 --- a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/actors/NetconfNodeActor.java +++ b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/actors/NetconfNodeActor.java @@ -17,6 +17,7 @@ import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import java.io.IOException; import java.util.List; +import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.opendaylight.controller.cluster.schema.provider.RemoteYangTextSourceProvider; @@ -59,6 +60,7 @@ import org.opendaylight.yangtools.yang.model.repo.api.SchemaSourceFilter; import org.opendaylight.yangtools.yang.model.repo.api.SourceIdentifier; import org.opendaylight.yangtools.yang.model.repo.api.YangTextSchemaSource; import org.opendaylight.yangtools.yang.model.repo.spi.PotentialSchemaSource; +import org.opendaylight.yangtools.yang.model.repo.spi.SchemaSourceRegistration; import org.opendaylight.yangtools.yang.model.repo.spi.SchemaSourceRegistry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -81,6 +83,7 @@ public class NetconfNodeActor extends UntypedActor { private DOMDataBroker deviceDataBroker; //readTxActor can be shared private ActorRef readTxActor; + private List> registeredSchemas; public static Props props(final NetconfTopologySetup setup, final RemoteDeviceId id, final SchemaSourceRegistry schemaRegistry, @@ -162,6 +165,11 @@ public class NetconfNodeActor extends UntypedActor { } } + @Override + public void postStop() throws Exception { + super.postStop(); + closeSchemaSourceRegistrations(); + } private void sendYangTextSchemaSourceProxy(final SourceIdentifier sourceIdentifier, final ActorRef sender) { final CheckedFuture yangTextSchemaSource = @@ -216,6 +224,7 @@ public class NetconfNodeActor extends UntypedActor { if (this.slaveSalManager != null) { slaveSalManager.close(); } + closeSchemaSourceRegistrations(); slaveSalManager = new SlaveSalFacade(id, setup.getDomBroker(), setup.getActorSystem(), actorResponseWaitTime); final CheckedFuture remoteSchemaContext = @@ -247,9 +256,11 @@ public class NetconfNodeActor extends UntypedActor { final RemoteSchemaProvider remoteProvider = new RemoteSchemaProvider(remoteYangTextSourceProvider, getContext().dispatcher()); - sourceIdentifiers.forEach(sourceId -> - schemaRegistry.registerSchemaSource(remoteProvider, PotentialSchemaSource.create(sourceId, - YangTextSchemaSource.class, PotentialSchemaSource.Costs.REMOTE_IO.getValue()))); + registeredSchemas = sourceIdentifiers.stream() + .map(sourceId -> + schemaRegistry.registerSchemaSource(remoteProvider, PotentialSchemaSource.create(sourceId, + YangTextSchemaSource.class, PotentialSchemaSource.Costs.REMOTE_IO.getValue()))) + .collect(Collectors.toList()); final SchemaContextFactory schemaContextFactory = schemaRepository.createSchemaContextFactory(SchemaSourceFilter.ALWAYS_ACCEPT); @@ -257,4 +268,11 @@ public class NetconfNodeActor extends UntypedActor { return schemaContextFactory.createSchemaContext(sourceIdentifiers); } + private void closeSchemaSourceRegistrations() { + if (registeredSchemas != null) { + registeredSchemas.forEach(SchemaSourceRegistration::close); + registeredSchemas = null; + } + } + } diff --git a/netconf/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeActorTest.java b/netconf/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeActorTest.java index b17b8daa70..02e4440dc2 100644 --- a/netconf/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeActorTest.java +++ b/netconf/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeActorTest.java @@ -12,8 +12,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; import static org.mockito.MockitoAnnotations.initMocks; import static org.opendaylight.netconf.topology.singleton.impl.utils.NetconfTopologyUtils.DEFAULT_SCHEMA_REPOSITORY; @@ -27,9 +30,11 @@ import akka.testkit.TestActorRef; import akka.util.Timeout; import com.google.common.base.MoreObjects; import com.google.common.base.Optional; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.SettableFuture; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -43,6 +48,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.opendaylight.controller.cluster.schema.provider.impl.YangTextSchemaSourceSerializationProxy; import org.opendaylight.controller.md.sal.dom.api.DOMDataBroker; @@ -50,6 +56,7 @@ import org.opendaylight.controller.md.sal.dom.api.DOMRpcException; import org.opendaylight.controller.md.sal.dom.api.DOMRpcResult; import org.opendaylight.controller.md.sal.dom.api.DOMRpcService; import org.opendaylight.controller.md.sal.dom.spi.DefaultDOMRpcResult; +import org.opendaylight.controller.sal.core.api.Broker; import org.opendaylight.netconf.sal.connect.util.RemoteDeviceId; import org.opendaylight.netconf.topology.singleton.impl.actors.NetconfNodeActor; import org.opendaylight.netconf.topology.singleton.impl.utils.ClusteringRpcException; @@ -66,13 +73,19 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeBuilder; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.opendaylight.yangtools.yang.model.api.SchemaPath; import org.opendaylight.yangtools.yang.model.repo.api.MissingSchemaSourceException; import org.opendaylight.yangtools.yang.model.repo.api.RevisionSourceIdentifier; +import org.opendaylight.yangtools.yang.model.repo.api.SchemaContextFactory; import org.opendaylight.yangtools.yang.model.repo.api.SchemaRepository; +import org.opendaylight.yangtools.yang.model.repo.api.SchemaResolutionException; import org.opendaylight.yangtools.yang.model.repo.api.SchemaSourceException; import org.opendaylight.yangtools.yang.model.repo.api.SourceIdentifier; import org.opendaylight.yangtools.yang.model.repo.api.YangTextSchemaSource; +import org.opendaylight.yangtools.yang.model.repo.spi.PotentialSchemaSource; +import org.opendaylight.yangtools.yang.model.repo.spi.SchemaSourceRegistration; +import org.opendaylight.yangtools.yang.model.repo.spi.SchemaSourceRegistry; import scala.concurrent.Await; import scala.concurrent.Future; import scala.concurrent.duration.Duration; @@ -175,6 +188,39 @@ public class NetconfNodeActorTest { } + @Test + public void testReceiveRegisterMountpoint() throws Exception { + final NetconfTopologySetup setup = mock(NetconfTopologySetup.class); + doReturn(mock(Broker.class)).when(setup).getDomBroker(); + final RevisionSourceIdentifier yang1 = RevisionSourceIdentifier.create("yang1"); + final RevisionSourceIdentifier yang2 = RevisionSourceIdentifier.create("yang2"); + final SchemaSourceRegistry registry = mock(SchemaSourceRegistry.class); + final SchemaRepository schemaRepository = mock(SchemaRepository.class); + final SchemaSourceRegistration regYang1 = mock(SchemaSourceRegistration.class); + final SchemaSourceRegistration regYang2 = mock(SchemaSourceRegistration.class); + doReturn(regYang1).when(registry).registerSchemaSource(any(), withSourceId(yang1)); + doReturn(regYang2).when(registry).registerSchemaSource(any(), withSourceId(yang2)); + final SchemaContextFactory schemaContextFactory = mock(SchemaContextFactory.class); + doReturn(schemaContextFactory).when(schemaRepository).createSchemaContextFactory(any()); + final SettableFuture schemaContextFuture = SettableFuture.create(); + final CheckedFuture checkedFuture = + Futures.makeChecked(schemaContextFuture, e -> new SchemaResolutionException("fail", e)); + doReturn(checkedFuture).when(schemaContextFactory).createSchemaContext(any()); + final ActorRef slaveRef = + system.actorOf(NetconfNodeActor.props(setup, remoteDeviceId, registry, schemaRepository, TIMEOUT)); + final List sources = ImmutableList.of(yang1, yang2); + slaveRef.tell(new RegisterMountPoint(sources), masterRef); + + verify(registry, timeout(1000)).registerSchemaSource(any(), withSourceId(yang1)); + verify(registry, timeout(1000)).registerSchemaSource(any(), withSourceId(yang2)); + //stop actor + final Future stopFuture = Patterns.gracefulStop(slaveRef, TIMEOUT.duration()); + Await.result(stopFuture, TIMEOUT.duration()); + //provider should be deregistered + verify(regYang1).close(); + verify(regYang2).close(); + } + @Test public void testYangTextSchemaSourceRequestMessage() throws Exception { final SchemaRepository schemaRepository = mock(SchemaRepository.class); @@ -325,6 +371,16 @@ public class NetconfNodeActorTest { } + private PotentialSchemaSource withSourceId(final SourceIdentifier identifier) { + return argThat(new ArgumentMatcher() { + @Override + public boolean matches(final Object argument) { + final PotentialSchemaSource potentialSchemaSource = (PotentialSchemaSource) argument; + return identifier.equals(potentialSchemaSource.getSourceIdentifier()); + } + }); + } + private String convertStreamToString(final java.io.InputStream is) { final java.util.Scanner s = new java.util.Scanner(is).useDelimiter("\\A"); return s.hasNext() ? s.next() : "";