Resolve exception from registerClusterSingletonService

registerClusterSingletonService return RuntimeException in
case of problems with registration and client should implement
strategy to resolve this issue

Change-Id: If6350969f1d1c2917c78850f4c66629b52137b0b
Signed-off-by: miroslav.kovac <miroslav.kovac@pantheon.tech>
(cherry picked from commit dbcd66681f76ef4175917d1620b9066b86f2a267)

diff --git a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java
index ebaafcf8866a3082a138981b6d33d85fe9799c71..1f166247b517661cb5346802207ea3e5b05bf45c 100644 (file)
--- a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java
+++ b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java
@@ -137,6 +137,11 @@ public class NetconfTopologyManager
         context.refresh(createSetup(instanceIdentifier, node));
     }
 
+    // ClusterSingletonServiceRegistration registerClusterSingletonService method throws a Runtime exception if there
+    // are problems with registration and client has to deal with it. Only thing we can do if this error occurs is to
+    // retry registration several times and log the error.
+    // TODO change to a specific documented Exception when changed in ClusterSingletonServiceProvider
+    @SuppressWarnings("checkstyle:IllegalCatch")
     private void startNetconfDeviceContext(final InstanceIdentifier<Node> instanceIdentifier, final Node node) {
         final NetconfNode netconfNode = node.getAugmentation(NetconfNode.class);
         Preconditions.checkNotNull(netconfNode);
@@ -153,11 +158,26 @@ public class NetconfTopologyManager
                 new NetconfTopologyContext(createSetup(instanceIdentifier, node), serviceGroupIdent,
                         actorResponseWaitTime, mountPointService);
 
-        final ClusterSingletonServiceRegistration clusterSingletonServiceRegistration =
-                clusterSingletonServiceProvider.registerClusterSingletonService(newNetconfTopologyContext);
+        int tries = 3;
+        while (true) {
+            try {
+                final ClusterSingletonServiceRegistration clusterSingletonServiceRegistration =
+                        clusterSingletonServiceProvider.registerClusterSingletonService(newNetconfTopologyContext);
+                clusterRegistrations.put(instanceIdentifier, clusterSingletonServiceRegistration);
+                contexts.put(instanceIdentifier, newNetconfTopologyContext);
+                break;
+            } catch (final RuntimeException e) {
+                LOG.warn("Unable to register cluster singleton service {}, trying again", newNetconfTopologyContext, e);
+
+                if (--tries <= 0) {
+                    LOG.error("Unable to register cluster singleton service {} - done trying, closing topology context",
+                            newNetconfTopologyContext, e);
+                    close();
+                    break;
+                }
+            }
+        }
 
-        clusterRegistrations.put(instanceIdentifier, clusterSingletonServiceRegistration);
-        contexts.put(instanceIdentifier, newNetconfTopologyContext);
     }
 
     private void stopNetconfDeviceContext(final InstanceIdentifier<Node> instanceIdentifier) {

diff --git a/netconf/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java b/netconf/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java
index ffc52a5f4663184c2d9f01e575f378a9af9b4ba2..817a58a5eca7eabd5800507c92c0aa00d33e5b83 100644 (file)
--- a/netconf/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java
+++ b/netconf/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java
@@ -14,6 +14,7 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -92,16 +93,83 @@ public class NetconfTopologyManagerTest {
 
     @Test
     public void testWriteConfiguration() throws Exception {
+        writeConfiguration(false);
+    }
 
-        final ClusterSingletonServiceRegistration clusterRegistration = mock(ClusterSingletonServiceRegistration.class);
+    @Test
+    public void testWriteConfigurationFail() throws Exception {
+        writeConfiguration(true);
+    }
+
+    @Test
+    public void testRegisterDataTreeChangeListener() {
+
+        final WriteTransaction wtx = mock(WriteTransaction.class);
+
+        doReturn(wtx).when(dataBroker).newWriteOnlyTransaction();
+        doNothing().when(wtx).merge(any(), any(), any());
+        doReturn(Futures.immediateCheckedFuture(null)).when(wtx).submit();
+        doReturn(null).when(dataBroker).registerDataChangeListener(any(), any(), any(), any());
+
+        netconfTopologyManager.init();
+
+        // verify if listener is called with right parameters = registered on right path
+
+        verify(dataBroker, times(1)).registerDataTreeChangeListener(
+                new DataTreeIdentifier<>(LogicalDatastoreType.CONFIGURATION, NetconfTopologyUtils
+                        .createTopologyListPath(topologyId).child(Node.class)), netconfTopologyManager);
+
+    }
+
+    @Test
+    public void testClose() throws Exception {
 
         final Field fieldContexts = NetconfTopologyManager.class.getDeclaredField("contexts");
         fieldContexts.setAccessible(true);
+        @SuppressWarnings("unchecked") final Map<InstanceIdentifier<Node>, NetconfTopologyContext> contexts =
+                (Map<InstanceIdentifier<Node>, NetconfTopologyContext>) fieldContexts.get(netconfTopologyManager);
+
+        final Field fieldClusterRegistrations =
+                NetconfTopologyManager.class.getDeclaredField("clusterRegistrations");
+        fieldClusterRegistrations.setAccessible(true);
         @SuppressWarnings("unchecked")
-        final Map<InstanceIdentifier<Node>, NetconfTopologyContext> contexts =
+        final Map<InstanceIdentifier<Node>, ClusterSingletonServiceRegistration> clusterRegistrations =
+                (Map<InstanceIdentifier<Node>, ClusterSingletonServiceRegistration>)
+                        fieldClusterRegistrations.get(netconfTopologyManager);
+
+        final InstanceIdentifier<Node> instanceIdentifier = NetconfTopologyUtils.createTopologyNodeListPath(
+                new NodeKey(new NodeId("node-id-1")), "topology-1");
+
+
+        final NetconfTopologyContext context = mock(NetconfTopologyContext.class);
+        final ClusterSingletonServiceRegistration clusterRegistration =
+                mock(ClusterSingletonServiceRegistration.class);
+        contexts.put(instanceIdentifier, context);
+        clusterRegistrations.put(instanceIdentifier, clusterRegistration);
+
+        doNothing().when(context).closeFinal();
+        doNothing().when(clusterRegistration).close();
+
+        netconfTopologyManager.close();
+        verify(context, times(1)).closeFinal();
+        verify(clusterRegistration, times(1)).close();
+
+        assertTrue(contexts.isEmpty());
+        assertTrue(clusterRegistrations.isEmpty());
+
+    }
+
+    private void writeConfiguration(final boolean fail) throws Exception {
+
+        final ClusterSingletonServiceRegistration clusterRegistration = mock(ClusterSingletonServiceRegistration.class);
+
+        final Field fieldContexts = NetconfTopologyManager.class.getDeclaredField("contexts");
+        fieldContexts.setAccessible(true);
+        @SuppressWarnings("unchecked") final Map<InstanceIdentifier<Node>, NetconfTopologyContext> contexts =
                 (Map<InstanceIdentifier<Node>, NetconfTopologyContext>) fieldContexts.get(netconfTopologyManager);
 
-        final Field fieldClusterRegistrations = NetconfTopologyManager.class.getDeclaredField("clusterRegistrations");
+        final Field fieldClusterRegistrations =
+                NetconfTopologyManager.class.getDeclaredField("clusterRegistrations");
         fieldClusterRegistrations.setAccessible(true);
         @SuppressWarnings("unchecked")
         final Map<InstanceIdentifier<Node>, ClusterSingletonServiceRegistration> clusterRegistrations =
@@ -111,10 +179,10 @@ public class NetconfTopologyManagerTest {
         final Collection<DataTreeModification<Node>> changes = new ArrayList<>();
 
         final InstanceIdentifier<Node> instanceIdentifier = NetconfTopologyUtils.createTopologyNodeListPath(
-                new NodeKey(new NodeId("node-id-1")),"topology-1");
+                new NodeKey(new NodeId("node-id-1")), "topology-1");
 
         final InstanceIdentifier<Node> instanceIdentifierDiferent = NetconfTopologyUtils.createTopologyNodeListPath(
-                new NodeKey(new NodeId("node-id-2")),"topology-2");
+                new NodeKey(new NodeId("node-id-2")), "topology-2");
 
         final DataTreeIdentifier<Node> rootIdentifier =
                 new DataTreeIdentifier<>(LogicalDatastoreType.CONFIGURATION, instanceIdentifier);
@@ -122,8 +190,8 @@ public class NetconfTopologyManagerTest {
         final DataTreeIdentifier<Node> rootIdentifierDifferent =
                 new DataTreeIdentifier<>(LogicalDatastoreType.CONFIGURATION, instanceIdentifierDiferent);
 
-        @SuppressWarnings("unchecked")
-        final DataObjectModification<Node> objectModification = mock(DataObjectModification.class);
+        @SuppressWarnings("unchecked") final DataObjectModification<Node> objectModification =
+                mock(DataObjectModification.class);
 
         final NetconfNode netconfNode = new NetconfNodeBuilder()
                 .setHost(new Host(new IpAddress(new Ipv4Address("127.0.0.1"))))
@@ -140,113 +208,68 @@ public class NetconfTopologyManagerTest {
 
         final Identifier key = new NodeKey(new NodeId("node-id"));
 
-        @SuppressWarnings("unchecked")
-        final InstanceIdentifier.IdentifiableItem<Node, NodeKey> pathArgument =
+        @SuppressWarnings("unchecked") final InstanceIdentifier.IdentifiableItem<Node, NodeKey> pathArgument =
                 new InstanceIdentifier.IdentifiableItem(Node.class, key);
 
 
         // testing WRITE on two identical rootIdentifiers and one different
-
-        changes.add(new CustomTreeModification(rootIdentifier, objectModification));
-        changes.add(new CustomTreeModification(rootIdentifier, objectModification));
-        changes.add(new CustomTreeModification(rootIdentifierDifferent, objectModification));
-
+        if (fail) {
+            changes.add(new CustomTreeModification(rootIdentifier, objectModification));
+        } else {
+            changes.add(new CustomTreeModification(rootIdentifier, objectModification));
+            changes.add(new CustomTreeModification(rootIdentifier, objectModification));
+            changes.add(new CustomTreeModification(rootIdentifierDifferent, objectModification));
+        }
         doReturn(WRITE).when(objectModification).getModificationType();
         doReturn(node).when(objectModification).getDataAfter();
         doReturn(pathArgument).when(objectModification).getIdentifier();
-        doReturn(clusterRegistration).when(clusterSingletonServiceProvider).registerClusterSingletonService(any());
-
-        netconfTopologyManager.onDataTreeChanged(changes);
-
-        verify(clusterSingletonServiceProvider, times(2)).registerClusterSingletonService(any());
-
-        // only two created contexts
-        assertEquals(2, contexts.size());
-        assertTrue(contexts.containsKey(rootIdentifier.getRootIdentifier()));
-        assertTrue(contexts.containsKey(rootIdentifierDifferent.getRootIdentifier()));
-
-        // only two created cluster registrations
-        assertEquals(2, contexts.size());
-        assertTrue(clusterRegistrations.containsKey(rootIdentifier.getRootIdentifier()));
-        assertTrue(clusterRegistrations.containsKey(rootIdentifierDifferent.getRootIdentifier()));
-
-        // after delete there should be no context and clustered registrations
-        doReturn(DELETE).when(objectModification).getModificationType();
-
-        doNothing().when(clusterRegistration).close();
 
+        if (fail) {
+            doThrow(new RuntimeException("error")).when(clusterSingletonServiceProvider)
+                    .registerClusterSingletonService(any());
+        } else {
+            doReturn(clusterRegistration).when(clusterSingletonServiceProvider).registerClusterSingletonService(any());
+        }
         netconfTopologyManager.onDataTreeChanged(changes);
 
-        verify(clusterRegistration, times(2)).close();
-
-        // empty map of contexts
-        assertTrue(contexts.isEmpty());
-        assertFalse(contexts.containsKey(rootIdentifier.getRootIdentifier()));
-        assertFalse(contexts.containsKey(rootIdentifierDifferent.getRootIdentifier()));
-
-        // empty map of clustered registrations
-        assertTrue(clusterRegistrations.isEmpty());
-        assertFalse(clusterRegistrations.containsKey(rootIdentifier.getRootIdentifier()));
-        assertFalse(clusterRegistrations.containsKey(rootIdentifierDifferent.getRootIdentifier()));
-
-    }
-
-    @Test
-    public void testRegisterDataTreeChangeListener() {
+        if (fail) {
+            verify(clusterSingletonServiceProvider, times(3))
+                    .registerClusterSingletonService(any());
+            assertTrue(contexts.isEmpty());
+            assertTrue(clusterRegistrations.isEmpty());
+        } else {
+            verify(clusterSingletonServiceProvider, times(2))
+                    .registerClusterSingletonService(any());
 
-        final WriteTransaction wtx = mock(WriteTransaction.class);
+            // only two created contexts
+            assertEquals(2, contexts.size());
+            assertTrue(contexts.containsKey(rootIdentifier.getRootIdentifier()));
+            assertTrue(contexts.containsKey(rootIdentifierDifferent.getRootIdentifier()));
 
-        doReturn(wtx).when(dataBroker).newWriteOnlyTransaction();
-        doNothing().when(wtx).merge(any(), any(), any());
-        doReturn(Futures.immediateCheckedFuture(null)).when(wtx).submit();
-        doReturn(null).when(dataBroker).registerDataChangeListener(any(), any(), any(), any());
+            // only two created cluster registrations
+            assertEquals(2, clusterRegistrations.size());
+            assertTrue(clusterRegistrations.containsKey(rootIdentifier.getRootIdentifier()));
+            assertTrue(clusterRegistrations.containsKey(rootIdentifierDifferent.getRootIdentifier()));
 
-        netconfTopologyManager.init();
+            // after delete there should be no context and clustered registrations
+            doReturn(DELETE).when(objectModification).getModificationType();
 
-        // verify if listener is called with right parameters = registered on right path
+            doNothing().when(clusterRegistration).close();
 
-        verify(dataBroker, times(1)).registerDataTreeChangeListener(
-                new DataTreeIdentifier<>(LogicalDatastoreType.CONFIGURATION, NetconfTopologyUtils
-                        .createTopologyListPath(topologyId).child(Node.class)), netconfTopologyManager);
+            netconfTopologyManager.onDataTreeChanged(changes);
 
-    }
+            verify(clusterRegistration, times(2)).close();
 
-    @Test
-    public void testClose() throws Exception {
-
-        final Field fieldContexts = NetconfTopologyManager.class.getDeclaredField("contexts");
-        fieldContexts.setAccessible(true);
-        @SuppressWarnings("unchecked")
-        final Map<InstanceIdentifier<Node>, NetconfTopologyContext> contexts =
-                (Map<InstanceIdentifier<Node>, NetconfTopologyContext>) fieldContexts.get(netconfTopologyManager);
-
-        final Field fieldClusterRegistrations = NetconfTopologyManager.class.getDeclaredField("clusterRegistrations");
-        fieldClusterRegistrations.setAccessible(true);
-        @SuppressWarnings("unchecked")
-        final Map<InstanceIdentifier<Node>, ClusterSingletonServiceRegistration> clusterRegistrations =
-                (Map<InstanceIdentifier<Node>, ClusterSingletonServiceRegistration>)
-                        fieldClusterRegistrations.get(netconfTopologyManager);
-
-        final InstanceIdentifier<Node> instanceIdentifier = NetconfTopologyUtils.createTopologyNodeListPath(
-                new NodeKey(new NodeId("node-id-1")),"topology-1");
-
-
-        final NetconfTopologyContext context = mock(NetconfTopologyContext.class);
-        final ClusterSingletonServiceRegistration clusterRegistration =
-                mock(ClusterSingletonServiceRegistration.class);
-        contexts.put(instanceIdentifier, context);
-        clusterRegistrations.put(instanceIdentifier, clusterRegistration);
-
-        doNothing().when(context).closeFinal();
-        doNothing().when(clusterRegistration).close();
-
-        netconfTopologyManager.close();
-        verify(context, times(1)).closeFinal();
-        verify(clusterRegistration, times(1)).close();
-
-        assertTrue(contexts.isEmpty());
-        assertTrue(clusterRegistrations.isEmpty());
+            // empty map of contexts
+            assertTrue(contexts.isEmpty());
+            assertFalse(contexts.containsKey(rootIdentifier.getRootIdentifier()));
+            assertFalse(contexts.containsKey(rootIdentifierDifferent.getRootIdentifier()));
 
+            // empty map of clustered registrations
+            assertTrue(clusterRegistrations.isEmpty());
+            assertFalse(clusterRegistrations.containsKey(rootIdentifier.getRootIdentifier()));
+            assertFalse(clusterRegistrations.containsKey(rootIdentifierDifferent.getRootIdentifier()));
+        }
     }
 
     private class CustomTreeModification  implements DataTreeModification<Node> {