From 7fae18929c62d68ffbf5d968c40f0b06d3df7dfd Mon Sep 17 00:00:00 2001 From: Deepa Vaddireddy Date: Wed, 15 Jun 2016 17:13:15 +0530 Subject: [PATCH] Fix for ONOS-4621. If any errors in parsing the configurations, continue with the next config key and return the list of subjectClass, subject, config failures Change-Id: I4883342b4920aa4d6d641a17a395e5f6e4f27d6a --- .../net/config/impl/NetworkConfigLoader.java | 29 ++++++--- .../resources/NetworkConfigWebResource.java | 63 ++++++++++++++----- 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/core/net/src/main/java/org/onosproject/net/config/impl/NetworkConfigLoader.java b/core/net/src/main/java/org/onosproject/net/config/impl/NetworkConfigLoader.java index 8921716305..afff6a4524 100644 --- a/core/net/src/main/java/org/onosproject/net/config/impl/NetworkConfigLoader.java +++ b/core/net/src/main/java/org/onosproject/net/config/impl/NetworkConfigLoader.java @@ -73,13 +73,14 @@ public class NetworkConfigLoader { populateConfigurations(); - applyConfigurations(); - - log.info("Loaded initial network configuration from {}", CFG_FILE); + if (applyConfigurations()) { + log.info("Loaded initial network configuration from {}", CFG_FILE); + } else { + log.error("Partially loaded initial network configuration from {}", CFG_FILE); + } } } catch (Exception e) { - log.warn("Unable to load initial network configuration from {}", - CFG_FILE, e); + log.warn("Unable to load initial network configuration from {}", CFG_FILE, e); } } @@ -185,8 +186,10 @@ public class NetworkConfigLoader { /** * Apply the configurations associated with all of the config classes that * are imported and have not yet been applied. + * + * @return false if any of the configuration parsing fails */ - private void applyConfigurations() { + private boolean applyConfigurations() { Iterator> iter = jsons.entrySet().iterator(); Map.Entry entry; @@ -195,7 +198,7 @@ public class NetworkConfigLoader { String subjectKey; String subjectString; String configKey; - + boolean isSuccess = true; while (iter.hasNext()) { entry = iter.next(); node = entry.getValue(); @@ -212,13 +215,19 @@ public class NetworkConfigLoader { Object subject = networkConfigService.getSubjectFactory(subjectKey). createSubject(subjectString); - //Apply the configuration - networkConfigService.applyConfig(subject, configClass, node); + try { + //Apply the configuration + networkConfigService.applyConfig(subject, configClass, node); + } catch (IllegalArgumentException e) { + log.warn("Error parsing config " + subjectKey + "/" + subject + "/" + configKey); + isSuccess = false; + } //Now that it has been applied the corresponding JSON entry is no longer needed iter.remove(); } } - } + return isSuccess; + } } diff --git a/web/api/src/main/java/org/onosproject/rest/resources/NetworkConfigWebResource.java b/web/api/src/main/java/org/onosproject/rest/resources/NetworkConfigWebResource.java index fc0b55df02..b8dbb0929e 100644 --- a/web/api/src/main/java/org/onosproject/rest/resources/NetworkConfigWebResource.java +++ b/web/api/src/main/java/org/onosproject/rest/resources/NetworkConfigWebResource.java @@ -16,6 +16,7 @@ package org.onosproject.rest.resources; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import org.onosproject.net.config.Config; import org.onosproject.net.config.NetworkConfigService; @@ -33,6 +34,8 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import static org.onlab.util.Tools.emptyIsNotFound; @@ -44,6 +47,9 @@ import static org.onlab.util.Tools.nullIsNotFound; @Path("network/configuration") public class NetworkConfigWebResource extends AbstractWebResource { + //FIX ME not found Multi status error code 207 in jaxrs Response Status. + private static final int MULTI_STATUS_RESPONE = 207; + private String subjectClassNotFoundErrorString(String subjectClassKey) { return "Config for '" + subjectClassKey + "' not found"; } @@ -191,9 +197,16 @@ public class NetworkConfigWebResource extends AbstractWebResource { public Response upload(InputStream request) throws IOException { NetworkConfigService service = get(NetworkConfigService.class); ObjectNode root = (ObjectNode) mapper().readTree(request); + List errorMsgs = new ArrayList(); root.fieldNames() - .forEachRemaining(sk -> consumeJson(service, (ObjectNode) root.path(sk), - service.getSubjectFactory(sk))); + .forEachRemaining(sk -> + { + errorMsgs.addAll(consumeJson(service, (ObjectNode) root.path(sk), + service.getSubjectFactory(sk))); + }); + if (errorMsgs.toString().length() > 0) { + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); + } return Response.ok().build(); } @@ -213,7 +226,10 @@ public class NetworkConfigWebResource extends AbstractWebResource { InputStream request) throws IOException { NetworkConfigService service = get(NetworkConfigService.class); ObjectNode root = (ObjectNode) mapper().readTree(request); - consumeJson(service, root, service.getSubjectFactory(subjectClassKey)); + List errorMsgs = consumeJson(service, root, service.getSubjectFactory(subjectClassKey)); + if (errorMsgs.toString().length() > 0) { + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); + } return Response.ok().build(); } @@ -235,9 +251,12 @@ public class NetworkConfigWebResource extends AbstractWebResource { InputStream request) throws IOException { NetworkConfigService service = get(NetworkConfigService.class); ObjectNode root = (ObjectNode) mapper().readTree(request); - consumeSubjectJson(service, root, - service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), - subjectClassKey); + List errorMsgs = consumeSubjectJson(service, root, + service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), + subjectClassKey); + if (errorMsgs.size() > 0) { + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); + } return Response.ok().build(); } @@ -267,21 +286,37 @@ public class NetworkConfigWebResource extends AbstractWebResource { return Response.ok().build(); } - private void consumeJson(NetworkConfigService service, ObjectNode classNode, + private List consumeJson(NetworkConfigService service, ObjectNode classNode, SubjectFactory subjectFactory) { - classNode.fieldNames().forEachRemaining(s -> - consumeSubjectJson(service, (ObjectNode) classNode.path(s), - subjectFactory.createSubject(s), - subjectFactory.subjectClassKey())); + List errorMsgs = new ArrayList(); + classNode.fieldNames().forEachRemaining(s -> { + List error = consumeSubjectJson(service, (ObjectNode) classNode.path(s), + subjectFactory.createSubject(s), + subjectFactory.subjectClassKey()); + errorMsgs.addAll(error); + }); + return errorMsgs; } - private void consumeSubjectJson(NetworkConfigService service, + private List consumeSubjectJson(NetworkConfigService service, ObjectNode subjectNode, Object subject, String subjectClassKey) { - subjectNode.fieldNames().forEachRemaining(configKey -> - service.applyConfig(subjectClassKey, subject, configKey, subjectNode.path(configKey))); + List errorMsgs = new ArrayList(); + subjectNode.fieldNames().forEachRemaining(configKey -> { + try { + service.applyConfig(subjectClassKey, subject, configKey, subjectNode.path(configKey)); + } catch (IllegalArgumentException e) { + errorMsgs.add("Error parsing config " + subjectClassKey + "/" + subject + "/" + configKey); + } + }); + return errorMsgs; } + private ObjectNode produceErrorJson(List errorMsgs) { + ObjectMapper mapper = new ObjectMapper(); + ObjectNode result = mapper.createObjectNode().put("code", 207).putPOJO("message", errorMsgs); + return result; + } // FIXME: Refactor to allow queued configs to be removed