Add consistency check of visible yang modules to netconf. 92/3692/1
authorMaros Marsalek <mmarsale@cisco.com>
Fri, 13 Dec 2013 08:30:49 +0000 (09:30 +0100)
committerMaros Marsalek <mmarsale@cisco.com>
Fri, 13 Dec 2013 09:19:28 +0000 (10:19 +0100)
Netconf connector to config subsystem performs this check before a netconf session is established.
This check consists of comparing yang modules visible to config subsystem to modules visibile to yangstore.
If an inconsistency is detected, session negotiation is cancelled and the session is dropped.

Change-Id: I6e07c6aa0d6c3bf1ee5fd238cfc7ec664951fe87
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationServiceImpl.java
opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationServiceImplTest.java [new file with mode: 0644]
opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/AbstractNetconfSessionNegotiator.java

index ed8f02b..7cd43bd 100644 (file)
@@ -8,12 +8,16 @@
 
 package org.opendaylight.controller.netconf.confignetconfconnector.osgi;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.Sets;
+import org.opendaylight.controller.config.api.LookupRegistry;
 import org.opendaylight.controller.config.util.ConfigRegistryJMXClient;
 import org.opendaylight.controller.config.yang.store.api.YangStoreException;
 import org.opendaylight.controller.config.yang.store.api.YangStoreService;
 import org.opendaylight.controller.config.yang.store.api.YangStoreSnapshot;
+import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry;
 import org.opendaylight.controller.netconf.confignetconfconnector.transactions.TransactionProvider;
 import org.opendaylight.controller.netconf.confignetconfconnector.util.Util;
 import org.opendaylight.controller.netconf.mapping.api.Capability;
@@ -42,12 +46,38 @@ public class NetconfOperationServiceImpl implements NetconfOperationService {
             String netconfSessionIdForReporting) throws YangStoreException {
 
         yangStoreSnapshot = yangStoreService.getYangStoreSnapshot();
+        checkConsistencyBetweenYangStoreAndConfig(jmxClient, yangStoreSnapshot);
+
         transactionProvider = new TransactionProvider(jmxClient, netconfSessionIdForReporting);
         operationProvider = new NetconfOperationProvider(yangStoreSnapshot, jmxClient, transactionProvider,
                 netconfSessionIdForReporting);
         capabilities = setupCapabilities(yangStoreSnapshot);
     }
 
+
+    @VisibleForTesting
+    static void checkConsistencyBetweenYangStoreAndConfig(LookupRegistry jmxClient, YangStoreSnapshot yangStoreSnapshot) {
+        Set<String> missingModulesFromConfig = Sets.newHashSet();
+
+        Set<String> modulesSeenByConfig = jmxClient.getAvailableModuleFactoryQNames();
+        Map<String, Map<String, ModuleMXBeanEntry>> moduleMXBeanEntryMap = yangStoreSnapshot.getModuleMXBeanEntryMap();
+
+        for (Map<String, ModuleMXBeanEntry> moduleNameToMBE : moduleMXBeanEntryMap.values()) {
+            for (ModuleMXBeanEntry moduleMXBeanEntry : moduleNameToMBE.values()) {
+                String moduleSeenByYangStore = moduleMXBeanEntry.getYangModuleQName().toString();
+                if(modulesSeenByConfig.contains(moduleSeenByYangStore) == false)
+                    missingModulesFromConfig.add(moduleSeenByYangStore);
+            }
+        }
+
+        Preconditions
+                .checkState(
+                        missingModulesFromConfig.isEmpty(),
+                        "There are inconsistencies between configuration subsystem and yangstore in terms of discovered yang modules, yang modules missing from config subsystem but present in yangstore: %s, %sAll modules present in config: %s",
+                        missingModulesFromConfig, System.lineSeparator(), modulesSeenByConfig);
+
+    }
+
     @Override
     public void close() {
         yangStoreSnapshot.close();
diff --git a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationServiceImplTest.java b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationServiceImplTest.java
new file mode 100644 (file)
index 0000000..0d459cf
--- /dev/null
@@ -0,0 +1,113 @@
+/*
+ * Copyright (c) 2013 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.controller.netconf.confignetconfconnector.osgi;
+
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.matchers.JUnitMatchers;
+import org.opendaylight.controller.config.api.LookupRegistry;
+import org.opendaylight.controller.config.yang.store.api.YangStoreSnapshot;
+import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry;
+import org.opendaylight.yangtools.yang.common.QName;
+
+import java.net.URI;
+import java.util.Date;
+import java.util.Map;
+import java.util.Set;
+
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
+public class NetconfOperationServiceImplTest {
+
+    private Date date = new Date(0);
+
+    @Test
+    public void testCheckConsistencyBetweenYangStoreAndConfig_ok() throws Exception {
+        NetconfOperationServiceImpl.checkConsistencyBetweenYangStoreAndConfig(
+                mockJmxClient("qname1", "qname2"),
+                mockYangStoreSnapshot("qname2", "qname1"));
+    }
+
+    @Test
+    public void testCheckConsistencyBetweenYangStoreAndConfig_ok2() throws Exception {
+        NetconfOperationServiceImpl.checkConsistencyBetweenYangStoreAndConfig(
+                mockJmxClient("qname1", "qname2", "qname4", "qname5"),
+                mockYangStoreSnapshot("qname2", "qname1"));
+    }
+
+    @Test
+    public void testCheckConsistencyBetweenYangStoreAndConfig_ok3() throws Exception {
+        NetconfOperationServiceImpl.checkConsistencyBetweenYangStoreAndConfig(
+                mockJmxClient(),
+                mockYangStoreSnapshot());
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testCheckConsistencyBetweenYangStoreAndConfig_yangStoreMore() throws Exception {
+        try {
+            NetconfOperationServiceImpl.checkConsistencyBetweenYangStoreAndConfig(mockJmxClient("qname1"),
+                    mockYangStoreSnapshot("qname2", "qname1"));
+        } catch (IllegalStateException e) {
+            String message = e.getMessage();
+            Assert.assertThat(
+                    message,
+                    JUnitMatchers
+                            .containsString(" missing from config subsystem but present in yangstore: [(namespace?revision=1970-01-01)qname2]"));
+            Assert.assertThat(
+                    message,
+                    JUnitMatchers
+                            .containsString("All modules present in config: [(namespace?revision=1970-01-01)qname1]"));
+            throw e;
+        }
+    }
+
+    private YangStoreSnapshot mockYangStoreSnapshot(String... qnames) {
+        YangStoreSnapshot mock = mock(YangStoreSnapshot.class);
+
+        Map<String, Map<String, ModuleMXBeanEntry>> map = Maps.newHashMap();
+
+        Map<String, ModuleMXBeanEntry> innerMap = Maps.newHashMap();
+
+        int i = 1;
+        for (String qname : qnames) {
+            innerMap.put(Integer.toString(i++), mockMBeanEntry(qname));
+        }
+
+        map.put("1", innerMap);
+
+        doReturn(map).when(mock).getModuleMXBeanEntryMap();
+
+        return mock;
+    }
+
+    private ModuleMXBeanEntry mockMBeanEntry(String qname) {
+        ModuleMXBeanEntry mock = mock(ModuleMXBeanEntry.class);
+        QName q = getQName(qname);
+        doReturn(q).when(mock).getYangModuleQName();
+        return mock;
+    }
+
+    private QName getQName(String qname) {
+        return new QName(URI.create("namespace"), date, qname);
+    }
+
+    private LookupRegistry mockJmxClient(String... visibleQNames) {
+        LookupRegistry mock = mock(LookupRegistry.class);
+        Set<String> qnames = Sets.newHashSet();
+        for (String visibleQName : visibleQNames) {
+            QName q = getQName(visibleQName);
+            qnames.add(q.toString());
+        }
+        doReturn(qnames).when(mock).getAvailableModuleFactoryQNames();
+        return mock;
+    }
+}
index 95d2feb..e30ce5b 100644 (file)
@@ -10,8 +10,9 @@ package org.opendaylight.controller.netconf.util;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
-
 import io.netty.channel.Channel;
+import io.netty.channel.ChannelHandler;
+import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.ssl.SslHandler;
 import io.netty.util.Timeout;
 import io.netty.util.Timer;
@@ -19,9 +20,8 @@ import io.netty.util.TimerTask;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.GenericFutureListener;
 import io.netty.util.concurrent.Promise;
-
-import org.opendaylight.controller.netconf.api.NetconfSession;
 import org.opendaylight.controller.netconf.api.NetconfMessage;
+import org.opendaylight.controller.netconf.api.NetconfSession;
 import org.opendaylight.controller.netconf.api.NetconfSessionPreferences;
 import org.opendaylight.controller.netconf.util.handler.FramingMechanismHandlerFactory;
 import org.opendaylight.controller.netconf.util.handler.NetconfMessageAggregator;
@@ -44,11 +44,14 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
 
     // TODO what time ?
     private static final long INITIAL_HOLDTIMER = 1;
+
     private static final Logger logger = LoggerFactory.getLogger(AbstractNetconfSessionNegotiator.class);
+    public static final String NAME_OF_EXCEPTION_HANDLER = "lastExceptionHandler";
 
     protected final P sessionPreferences;
 
     private final SessionListener sessionListener;
+    private Timeout timeout;
 
     /**
      * Possible states for Finite State Machine
@@ -69,7 +72,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
     }
 
     @Override
-    protected void startNegotiation() throws Exception {
+    protected void startNegotiation() {
         final Optional<SslHandler> sslHandler = getSslHandler(channel);
         if (sslHandler.isPresent()) {
             Future<Channel> future = sslHandler.get().handshakeFuture();
@@ -94,10 +97,25 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
         final NetconfMessage helloMessage = this.sessionPreferences.getHelloMessage();
         logger.debug("Session negotiation started with hello message {}", XmlUtil.toString(helloMessage.getDocument()));
 
-        sendMessage(helloMessage);
-        changeState(State.OPEN_WAIT);
+        channel.pipeline().addLast(NAME_OF_EXCEPTION_HANDLER, new ChannelHandler() {
+            @Override
+            public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+            }
+
+            @Override
+            public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+            }
+
+            @Override
+            public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
+                logger.warn("An exception occurred during negotiation on channel {}", channel.localAddress(), cause);
+                cancelTimeout();
+                negotiationFailed(cause);
+                changeState(State.FAILED);
+            }
+        });
 
-        this.timer.newTimeout(new TimerTask() {
+        timeout = this.timer.newTimeout(new TimerTask() {
             @Override
             public void run(final Timeout timeout) throws Exception {
                 synchronized (this) {
@@ -106,10 +124,19 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
                                 "Session was not established after " + timeout);
                         negotiationFailed(cause);
                         changeState(State.FAILED);
-                    }
+                    } else
+                        channel.pipeline().remove(NAME_OF_EXCEPTION_HANDLER);
                 }
             }
         }, INITIAL_HOLDTIMER, TimeUnit.MINUTES);
+
+        sendMessage(helloMessage);
+        changeState(State.OPEN_WAIT);
+    }
+
+    private void cancelTimeout() {
+        if(timeout!=null)
+            timeout.cancel();
     }
 
     private void sendMessage(NetconfMessage message) {