From a7bc1dbcfefd7a349f4792242ea0f7a2ad072374 Mon Sep 17 00:00:00 2001 From: Tomas Olvecky Date: Fri, 4 Apr 2014 12:29:19 +0200 Subject: [PATCH] Resolve Bug:654 - Fix config-persister exception handling. Modify NetconfUtil to fail on any , not detecting transaction conflicts. Move exception handling to ConfigPusher. Also fix bug when config-netconf-connector detects discrepancy between yang store and config manager factories: persister will retry as if some capabilities were not found. Change-Id: Ibd80fd3859031ae41301481dad9b50f74207e64c Signed-off-by: Tomas Olvecky --- .../netconf/persist/impl/ConfigPusher.java | 23 ++++++++++------ .../impl/osgi/ConfigPersisterTest.java | 26 +++++++++++-------- .../controller/netconf/util/NetconfUtil.java | 22 ++-------------- .../netconf/util/NetconfUtilTest.java | 3 +-- 4 files changed, 33 insertions(+), 41 deletions(-) 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 6dba9ac64e..ed5a3a97a4 100644 --- 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 @@ -110,6 +110,10 @@ public class ConfigPusher { } private static class NotEnoughCapabilitiesException extends Exception { + private NotEnoughCapabilitiesException(String message, Throwable cause) { + super(message, cause); + } + private NotEnoughCapabilitiesException(String message) { super(message); } @@ -123,13 +127,18 @@ public class ConfigPusher { * @return service if capabilities are present, otherwise absent value */ private NetconfOperationService getOperationService(Set expectedCapabilities, String idForReporting) throws NotEnoughCapabilitiesException { - NetconfOperationService serviceCandidate = configNetconfConnector.createService(idForReporting); + NetconfOperationService serviceCandidate; + try { + serviceCandidate = configNetconfConnector.createService(idForReporting); + } catch(RuntimeException e) { + throw new NotEnoughCapabilitiesException("Netconf service not stable for " + idForReporting, e); + } Set notFoundDiff = computeNotFoundCapabilities(expectedCapabilities, serviceCandidate); if (notFoundDiff.isEmpty()) { return serviceCandidate; } else { serviceCandidate.close(); - logger.debug("Netconf server did not provide required capabilities for {} " + + logger.trace("Netconf server did not provide required capabilities for {} " + "Expected but not found: {}, all expected {}, current {}", idForReporting, notFoundDiff, expectedCapabilities, serviceCandidate.getCapabilities() ); @@ -226,15 +235,13 @@ public class ConfigPusher { try { response = operation.handle(request.getDocument(), NetconfOperationChainedExecution.EXECUTION_TERMINATION_POINT); } catch (NetconfDocumentedException | RuntimeException e) { + if (e instanceof NetconfDocumentedException && e.getCause() instanceof ConflictingVersionException) { + throw (ConflictingVersionException) e.getCause(); + } throw new IllegalStateException("Failed to send " + operationNameForReporting + " for configuration " + configIdForReporting, e); } - try { - return NetconfUtil.checkIsMessageOk(response); - } catch (ConflictingVersionException e) { - logger.trace("conflicting version detected: {} while committing {}", e.toString(), configIdForReporting); - throw e; - } + return NetconfUtil.checkIsMessageOk(response); } // load editConfig.xml template, populate /rpc/edit-config/config with parameter diff --git a/opendaylight/netconf/config-persister-impl/src/test/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterTest.java b/opendaylight/netconf/config-persister-impl/src/test/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterTest.java index b722496142..493ecd9250 100644 --- a/opendaylight/netconf/config-persister-impl/src/test/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterTest.java +++ b/opendaylight/netconf/config-persister-impl/src/test/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterTest.java @@ -33,6 +33,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; public class ConfigPersisterTest { @@ -113,26 +114,29 @@ public class ConfigPersisterTest { @Test public void testPersisterConflictingVersionException() throws Exception { setUpContextAndStartPersister("cap1"); - NetconfOperationService service = getWorkingService(getConflictVersionDocument()); - doReturn(service).when(ctx.serviceFactory).createService(anyString()); + + doReturn(getConflictingService()).when(ctx.serviceFactory).createService(anyString()); Thread.sleep(2000); handler.assertException(IllegalStateException.class, "Max wait for conflicting version stabilization timeout"); } - private Document getConflictVersionDocument() throws SAXException, IOException { - return XmlUtil.readXmlToDocument( - "\n" + - "" + - ConflictingVersionException.class.getCanonicalName() + - "\n" + - "" - ); + private NetconfOperationService getConflictingService() throws Exception { + NetconfOperationService service = getWorkingService(getOKDocument()); + ConflictingVersionException cve = new ConflictingVersionException(""); + try { + NetconfDocumentedException.wrap(cve); + throw new AssertionError("Should throw an exception"); + }catch(NetconfDocumentedException e) { + NetconfOperation mockedOperation = service.getNetconfOperations().iterator().next(); + doThrow(e).when(mockedOperation).handle(any(Document.class), any(NetconfOperationChainedExecution.class)); + return service; + } } @Test public void testSuccessConflictingVersionException() throws Exception { setUpContextAndStartPersister("cap1"); - doReturn(getWorkingService(getConflictVersionDocument())).when(ctx.serviceFactory).createService(anyString()); + doReturn(getConflictingService()).when(ctx.serviceFactory).createService(anyString()); Thread.sleep(500); // working service: logger.info("Switching to working service **"); diff --git a/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/NetconfUtil.java b/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/NetconfUtil.java index b0884ca2fb..29f40ce624 100644 --- a/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/NetconfUtil.java +++ b/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/NetconfUtil.java @@ -8,9 +8,7 @@ package org.opendaylight.controller.netconf.util; import com.google.common.base.Preconditions; -import org.opendaylight.controller.config.api.ConflictingVersionException; import org.opendaylight.controller.netconf.api.NetconfMessage; -import org.opendaylight.controller.netconf.util.xml.XMLNetconfUtil; import org.opendaylight.controller.netconf.util.xml.XmlElement; import org.opendaylight.controller.netconf.util.xml.XmlNetconfConstants; import org.opendaylight.controller.netconf.util.xml.XmlUtil; @@ -19,8 +17,6 @@ import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.xml.sax.SAXException; -import javax.xml.xpath.XPathConstants; -import javax.xml.xpath.XPathExpression; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -56,31 +52,17 @@ public final class NetconfUtil { return (doc == null) ? null : new NetconfMessage(doc); } - public static Document checkIsMessageOk(NetconfMessage responseMessage) throws ConflictingVersionException { + public static Document checkIsMessageOk(NetconfMessage responseMessage) { return checkIsMessageOk(responseMessage.getDocument()); } - public static Document checkIsMessageOk(Document response) throws ConflictingVersionException { + public static Document checkIsMessageOk(Document response) { XmlElement element = XmlElement.fromDomDocument(response); Preconditions.checkState(element.getName().equals(XmlNetconfConstants.RPC_REPLY_KEY)); element = element.getOnlyChildElement(); - if (element.getName().equals(XmlNetconfConstants.OK)) { return response; } - - if (element.getName().equals(XmlNetconfConstants.RPC_ERROR)) { - logger.warn("Can not load last configuration, operation failed"); - // is it ConflictingVersionException ? - XPathExpression xPathExpression = XMLNetconfUtil.compileXPath("/netconf:rpc-reply/netconf:rpc-error/netconf:error-info/netconf:error"); - String error = (String) XmlUtil.evaluateXPath(xPathExpression, element.getDomElement(), XPathConstants.STRING); - if (error!=null && error.contains(ConflictingVersionException.class.getCanonicalName())) { - throw new ConflictingVersionException(error); - } - throw new IllegalStateException("Can not load last configuration, operation failed: " - + XmlUtil.toString(response)); - } - logger.warn("Can not load last configuration. Operation failed."); throw new IllegalStateException("Can not load last configuration. Operation failed: " + XmlUtil.toString(response)); diff --git a/opendaylight/netconf/netconf-util/src/test/java/org/opendaylight/controller/netconf/util/NetconfUtilTest.java b/opendaylight/netconf/netconf-util/src/test/java/org/opendaylight/controller/netconf/util/NetconfUtilTest.java index b4b8aba81f..85eeb30a07 100644 --- a/opendaylight/netconf/netconf-util/src/test/java/org/opendaylight/controller/netconf/util/NetconfUtilTest.java +++ b/opendaylight/netconf/netconf-util/src/test/java/org/opendaylight/controller/netconf/util/NetconfUtilTest.java @@ -8,7 +8,6 @@ package org.opendaylight.controller.netconf.util; import org.junit.Test; -import org.opendaylight.controller.config.api.ConflictingVersionException; import org.opendaylight.controller.netconf.api.NetconfMessage; import org.opendaylight.controller.netconf.util.xml.XmlUtil; import org.w3c.dom.Document; @@ -25,7 +24,7 @@ public class NetconfUtilTest { try{ NetconfUtil.checkIsMessageOk(new NetconfMessage(document)); fail(); - }catch(ConflictingVersionException e){ + }catch(IllegalStateException e){ assertThat(e.getMessage(), containsString("Optimistic lock failed. Expected parent version 21, was 18")); } } -- 2.36.6