Merge "Fix config transaction handling in netconf."

diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java
index 0fa005e04a7930df7b1e85fc3aa933da4db98ac4..cd3b44852c352cdf9ca905980790ae329e6143ba 100644 (file)
--- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java
+++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java
@@ -95,15 +95,29 @@ public class TransactionProvider implements AutoCloseable {
      * Commit and notification send must be atomic
      */
     public synchronized CommitStatus commitTransaction() throws NetconfDocumentedException {
-        final Optional<ObjectName> taON = getTransaction();
-        Preconditions.checkState(taON.isPresent(), "No transaction found for session " + netconfSessionIdForReporting);
-        CommitStatus status = configRegistryClient.commitConfig(taON.get());
-        allOpenedTransactions.remove(transaction);
-        transaction = null;
-        return status;
+        final Optional<ObjectName> maybeTaON = getTransaction();
+        Preconditions.checkState(maybeTaON.isPresent(), "No transaction found for session " + netconfSessionIdForReporting);
+        ObjectName taON = maybeTaON.get();
+        try {
+            CommitStatus status = configRegistryClient.commitConfig(taON);
+            // clean up
+            allOpenedTransactions.remove(transaction);
+            transaction = null;
+            return status;
+        } catch (ValidationException validationException) {
+            // no clean up: user can reconfigure and recover this transaction
+            logger.warn("Transaction {} failed on {}", taON, validationException.toString());
+            throw validationException;
+        } catch (Exception e) {
+            logger.error("Exception while commit of {}, aborting transaction", taON, e);
+            // clean up
+            abortTransaction();
+            throw e;
+        }
     }
 
     public synchronized void abortTransaction() {
+        logger.debug("Aborting current transaction");
         Optional<ObjectName> taON = getTransaction();
         Preconditions.checkState(taON.isPresent(), "No transaction found for session " + netconfSessionIdForReporting);
 
@@ -114,6 +128,7 @@ public class TransactionProvider implements AutoCloseable {
     }
 
     public synchronized void abortTestTransaction(ObjectName testTx) {
+        logger.debug("Aborting transaction {}", testTx);
         ConfigTransactionClient transactionClient = configRegistryClient.getConfigTransactionClient(testTx);
         allOpenedTransactions.remove(testTx);
         transactionClient.abortConfig();

diff --git a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusher.java b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusher.java
index 6e62a982d1588217215f999011b6cfba4a443e46..3c968f8e2013423d4c405e1e95b8c4c711e5640e 100644 (file)
--- a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusher.java
+++ b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusher.java
@@ -72,7 +72,9 @@ public class ConfigPusher {
     }
 
     private synchronized NetconfClient pushAllConfigs(List<ConfigSnapshotHolder> configs) throws InterruptedException {
+        // first just make sure we can connect to netconf, even if nothing is being pushed
         NetconfClient netconfClient = makeNetconfConnection(Collections.<String>emptySet(), Optional.<NetconfClient>absent());
+        // start pushing snapshots:
         for (ConfigSnapshotHolder configSnapshotHolder: configs){
             netconfClient = pushSnapshotWithRetries(configSnapshotHolder, Optional.of(netconfClient));
         }
@@ -106,21 +108,16 @@ public class ConfigPusher {
     /**
      * @param expectedCaps capabilities that server hello must contain. Will retry until all are found or throws RuntimeException.
      *                     If empty set is provided, will only make sure netconf client successfuly connected to the server.
-     * @param oldClientForPossibleReuse if present, try to get expected capabilities from it before closing it and retrying with
-     *                                  new client connection.
+     * @param maybeOldClient if present, close it.
      * @return NetconfClient that has all required capabilities from server.
      */
     private synchronized NetconfClient makeNetconfConnection(Set<String> expectedCaps,
-                                                             Optional<NetconfClient> oldClientForPossibleReuse)
+                                                             Optional<NetconfClient> maybeOldClient)
             throws InterruptedException {
 
-        if (oldClientForPossibleReuse.isPresent()) {
-            NetconfClient oldClient = oldClientForPossibleReuse.get();
-            if (Util.isSubset(oldClient, expectedCaps)) {
-                return oldClient;
-            } else {
-                Util.closeClientAndDispatcher(oldClient);
-            }
+        if (maybeOldClient.isPresent()) {
+            NetconfClient oldClient = maybeOldClient.get();
+            Util.closeClientAndDispatcher(oldClient);
         }
 
         // TODO think about moving capability subset check to netconf client