Committed by
Thomas Vachuska
Fix for ONOS-4621. If any errors in parsing the configurations, continue with th…
…e next config key and return the list of subjectClass, subject, config failures Change-Id: I4883342b4920aa4d6d641a17a395e5f6e4f27d6a
Showing
2 changed files
with
68 additions
and
24 deletions
| ... | @@ -73,13 +73,14 @@ public class NetworkConfigLoader { | ... | @@ -73,13 +73,14 @@ public class NetworkConfigLoader { |
| 73 | 73 | ||
| 74 | populateConfigurations(); | 74 | populateConfigurations(); |
| 75 | 75 | ||
| 76 | - applyConfigurations(); | 76 | + if (applyConfigurations()) { |
| 77 | - | 77 | + log.info("Loaded initial network configuration from {}", CFG_FILE); |
| 78 | - log.info("Loaded initial network configuration from {}", CFG_FILE); | 78 | + } else { |
| 79 | + log.error("Partially loaded initial network configuration from {}", CFG_FILE); | ||
| 80 | + } | ||
| 79 | } | 81 | } |
| 80 | } catch (Exception e) { | 82 | } catch (Exception e) { |
| 81 | - log.warn("Unable to load initial network configuration from {}", | 83 | + log.warn("Unable to load initial network configuration from {}", CFG_FILE, e); |
| 82 | - CFG_FILE, e); | ||
| 83 | } | 84 | } |
| 84 | } | 85 | } |
| 85 | 86 | ||
| ... | @@ -185,8 +186,10 @@ public class NetworkConfigLoader { | ... | @@ -185,8 +186,10 @@ public class NetworkConfigLoader { |
| 185 | /** | 186 | /** |
| 186 | * Apply the configurations associated with all of the config classes that | 187 | * Apply the configurations associated with all of the config classes that |
| 187 | * are imported and have not yet been applied. | 188 | * are imported and have not yet been applied. |
| 189 | + * | ||
| 190 | + * @return false if any of the configuration parsing fails | ||
| 188 | */ | 191 | */ |
| 189 | - private void applyConfigurations() { | 192 | + private boolean applyConfigurations() { |
| 190 | Iterator<Map.Entry<InnerConfigPosition, JsonNode>> iter = jsons.entrySet().iterator(); | 193 | Iterator<Map.Entry<InnerConfigPosition, JsonNode>> iter = jsons.entrySet().iterator(); |
| 191 | 194 | ||
| 192 | Map.Entry<InnerConfigPosition, JsonNode> entry; | 195 | Map.Entry<InnerConfigPosition, JsonNode> entry; |
| ... | @@ -195,7 +198,7 @@ public class NetworkConfigLoader { | ... | @@ -195,7 +198,7 @@ public class NetworkConfigLoader { |
| 195 | String subjectKey; | 198 | String subjectKey; |
| 196 | String subjectString; | 199 | String subjectString; |
| 197 | String configKey; | 200 | String configKey; |
| 198 | - | 201 | + boolean isSuccess = true; |
| 199 | while (iter.hasNext()) { | 202 | while (iter.hasNext()) { |
| 200 | entry = iter.next(); | 203 | entry = iter.next(); |
| 201 | node = entry.getValue(); | 204 | node = entry.getValue(); |
| ... | @@ -212,13 +215,19 @@ public class NetworkConfigLoader { | ... | @@ -212,13 +215,19 @@ public class NetworkConfigLoader { |
| 212 | Object subject = networkConfigService.getSubjectFactory(subjectKey). | 215 | Object subject = networkConfigService.getSubjectFactory(subjectKey). |
| 213 | createSubject(subjectString); | 216 | createSubject(subjectString); |
| 214 | 217 | ||
| 215 | - //Apply the configuration | 218 | + try { |
| 216 | - networkConfigService.applyConfig(subject, configClass, node); | 219 | + //Apply the configuration |
| 220 | + networkConfigService.applyConfig(subject, configClass, node); | ||
| 221 | + } catch (IllegalArgumentException e) { | ||
| 222 | + log.warn("Error parsing config " + subjectKey + "/" + subject + "/" + configKey); | ||
| 223 | + isSuccess = false; | ||
| 224 | + } | ||
| 217 | 225 | ||
| 218 | //Now that it has been applied the corresponding JSON entry is no longer needed | 226 | //Now that it has been applied the corresponding JSON entry is no longer needed |
| 219 | iter.remove(); | 227 | iter.remove(); |
| 220 | } | 228 | } |
| 221 | } | 229 | } |
| 222 | - } | 230 | + return isSuccess; |
| 231 | + } | ||
| 223 | 232 | ||
| 224 | } | 233 | } | ... | ... |
| ... | @@ -16,6 +16,7 @@ | ... | @@ -16,6 +16,7 @@ |
| 16 | package org.onosproject.rest.resources; | 16 | package org.onosproject.rest.resources; |
| 17 | 17 | ||
| 18 | import com.fasterxml.jackson.databind.JsonNode; | 18 | import com.fasterxml.jackson.databind.JsonNode; |
| 19 | +import com.fasterxml.jackson.databind.ObjectMapper; | ||
| 19 | import com.fasterxml.jackson.databind.node.ObjectNode; | 20 | import com.fasterxml.jackson.databind.node.ObjectNode; |
| 20 | import org.onosproject.net.config.Config; | 21 | import org.onosproject.net.config.Config; |
| 21 | import org.onosproject.net.config.NetworkConfigService; | 22 | import org.onosproject.net.config.NetworkConfigService; |
| ... | @@ -33,6 +34,8 @@ import javax.ws.rs.core.MediaType; | ... | @@ -33,6 +34,8 @@ import javax.ws.rs.core.MediaType; |
| 33 | import javax.ws.rs.core.Response; | 34 | import javax.ws.rs.core.Response; |
| 34 | import java.io.IOException; | 35 | import java.io.IOException; |
| 35 | import java.io.InputStream; | 36 | import java.io.InputStream; |
| 37 | +import java.util.ArrayList; | ||
| 38 | +import java.util.List; | ||
| 36 | import java.util.Set; | 39 | import java.util.Set; |
| 37 | 40 | ||
| 38 | import static org.onlab.util.Tools.emptyIsNotFound; | 41 | import static org.onlab.util.Tools.emptyIsNotFound; |
| ... | @@ -44,6 +47,9 @@ import static org.onlab.util.Tools.nullIsNotFound; | ... | @@ -44,6 +47,9 @@ import static org.onlab.util.Tools.nullIsNotFound; |
| 44 | @Path("network/configuration") | 47 | @Path("network/configuration") |
| 45 | public class NetworkConfigWebResource extends AbstractWebResource { | 48 | public class NetworkConfigWebResource extends AbstractWebResource { |
| 46 | 49 | ||
| 50 | + //FIX ME not found Multi status error code 207 in jaxrs Response Status. | ||
| 51 | + private static final int MULTI_STATUS_RESPONE = 207; | ||
| 52 | + | ||
| 47 | private String subjectClassNotFoundErrorString(String subjectClassKey) { | 53 | private String subjectClassNotFoundErrorString(String subjectClassKey) { |
| 48 | return "Config for '" + subjectClassKey + "' not found"; | 54 | return "Config for '" + subjectClassKey + "' not found"; |
| 49 | } | 55 | } |
| ... | @@ -191,9 +197,16 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -191,9 +197,16 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 191 | public Response upload(InputStream request) throws IOException { | 197 | public Response upload(InputStream request) throws IOException { |
| 192 | NetworkConfigService service = get(NetworkConfigService.class); | 198 | NetworkConfigService service = get(NetworkConfigService.class); |
| 193 | ObjectNode root = (ObjectNode) mapper().readTree(request); | 199 | ObjectNode root = (ObjectNode) mapper().readTree(request); |
| 200 | + List<String> errorMsgs = new ArrayList<String>(); | ||
| 194 | root.fieldNames() | 201 | root.fieldNames() |
| 195 | - .forEachRemaining(sk -> consumeJson(service, (ObjectNode) root.path(sk), | 202 | + .forEachRemaining(sk -> |
| 196 | - service.getSubjectFactory(sk))); | 203 | + { |
| 204 | + errorMsgs.addAll(consumeJson(service, (ObjectNode) root.path(sk), | ||
| 205 | + service.getSubjectFactory(sk))); | ||
| 206 | + }); | ||
| 207 | + if (errorMsgs.toString().length() > 0) { | ||
| 208 | + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); | ||
| 209 | + } | ||
| 197 | return Response.ok().build(); | 210 | return Response.ok().build(); |
| 198 | } | 211 | } |
| 199 | 212 | ||
| ... | @@ -213,7 +226,10 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -213,7 +226,10 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 213 | InputStream request) throws IOException { | 226 | InputStream request) throws IOException { |
| 214 | NetworkConfigService service = get(NetworkConfigService.class); | 227 | NetworkConfigService service = get(NetworkConfigService.class); |
| 215 | ObjectNode root = (ObjectNode) mapper().readTree(request); | 228 | ObjectNode root = (ObjectNode) mapper().readTree(request); |
| 216 | - consumeJson(service, root, service.getSubjectFactory(subjectClassKey)); | 229 | + List<String> errorMsgs = consumeJson(service, root, service.getSubjectFactory(subjectClassKey)); |
| 230 | + if (errorMsgs.toString().length() > 0) { | ||
| 231 | + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); | ||
| 232 | + } | ||
| 217 | return Response.ok().build(); | 233 | return Response.ok().build(); |
| 218 | } | 234 | } |
| 219 | 235 | ||
| ... | @@ -235,9 +251,12 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -235,9 +251,12 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 235 | InputStream request) throws IOException { | 251 | InputStream request) throws IOException { |
| 236 | NetworkConfigService service = get(NetworkConfigService.class); | 252 | NetworkConfigService service = get(NetworkConfigService.class); |
| 237 | ObjectNode root = (ObjectNode) mapper().readTree(request); | 253 | ObjectNode root = (ObjectNode) mapper().readTree(request); |
| 238 | - consumeSubjectJson(service, root, | 254 | + List<String> errorMsgs = consumeSubjectJson(service, root, |
| 239 | - service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), | 255 | + service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), |
| 240 | - subjectClassKey); | 256 | + subjectClassKey); |
| 257 | + if (errorMsgs.size() > 0) { | ||
| 258 | + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); | ||
| 259 | + } | ||
| 241 | return Response.ok().build(); | 260 | return Response.ok().build(); |
| 242 | } | 261 | } |
| 243 | 262 | ||
| ... | @@ -267,21 +286,37 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -267,21 +286,37 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 267 | return Response.ok().build(); | 286 | return Response.ok().build(); |
| 268 | } | 287 | } |
| 269 | 288 | ||
| 270 | - private void consumeJson(NetworkConfigService service, ObjectNode classNode, | 289 | + private List<String> consumeJson(NetworkConfigService service, ObjectNode classNode, |
| 271 | SubjectFactory subjectFactory) { | 290 | SubjectFactory subjectFactory) { |
| 272 | - classNode.fieldNames().forEachRemaining(s -> | 291 | + List<String> errorMsgs = new ArrayList<String>(); |
| 273 | - consumeSubjectJson(service, (ObjectNode) classNode.path(s), | 292 | + classNode.fieldNames().forEachRemaining(s -> { |
| 274 | - subjectFactory.createSubject(s), | 293 | + List<String> error = consumeSubjectJson(service, (ObjectNode) classNode.path(s), |
| 275 | - subjectFactory.subjectClassKey())); | 294 | + subjectFactory.createSubject(s), |
| 295 | + subjectFactory.subjectClassKey()); | ||
| 296 | + errorMsgs.addAll(error); | ||
| 297 | + }); | ||
| 298 | + return errorMsgs; | ||
| 276 | } | 299 | } |
| 277 | 300 | ||
| 278 | - private void consumeSubjectJson(NetworkConfigService service, | 301 | + private List<String> consumeSubjectJson(NetworkConfigService service, |
| 279 | ObjectNode subjectNode, Object subject, | 302 | ObjectNode subjectNode, Object subject, |
| 280 | String subjectClassKey) { | 303 | String subjectClassKey) { |
| 281 | - subjectNode.fieldNames().forEachRemaining(configKey -> | 304 | + List<String> errorMsgs = new ArrayList<String>(); |
| 282 | - service.applyConfig(subjectClassKey, subject, configKey, subjectNode.path(configKey))); | 305 | + subjectNode.fieldNames().forEachRemaining(configKey -> { |
| 306 | + try { | ||
| 307 | + service.applyConfig(subjectClassKey, subject, configKey, subjectNode.path(configKey)); | ||
| 308 | + } catch (IllegalArgumentException e) { | ||
| 309 | + errorMsgs.add("Error parsing config " + subjectClassKey + "/" + subject + "/" + configKey); | ||
| 310 | + } | ||
| 311 | + }); | ||
| 312 | + return errorMsgs; | ||
| 283 | } | 313 | } |
| 284 | 314 | ||
| 315 | + private ObjectNode produceErrorJson(List<String> errorMsgs) { | ||
| 316 | + ObjectMapper mapper = new ObjectMapper(); | ||
| 317 | + ObjectNode result = mapper.createObjectNode().put("code", 207).putPOJO("message", errorMsgs); | ||
| 318 | + return result; | ||
| 319 | + } | ||
| 285 | 320 | ||
| 286 | // FIXME: Refactor to allow queued configs to be removed | 321 | // FIXME: Refactor to allow queued configs to be removed |
| 287 | 322 | ... | ... |
-
Please register or login to post a comment