Committed by
Gerrit Code Review
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
60 additions
and
16 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 | - | ||
78 | log.info("Loaded initial network configuration from {}", CFG_FILE); | 77 | 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 | ||
218 | + try { | ||
215 | //Apply the configuration | 219 | //Apply the configuration |
216 | networkConfigService.applyConfig(subject, configClass, node); | 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 | } |
230 | + return isSuccess; | ||
222 | } | 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 -> |
203 | + { | ||
204 | + errorMsgs.addAll(consumeJson(service, (ObjectNode) root.path(sk), | ||
196 | service.getSubjectFactory(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 -> { |
293 | + List<String> error = consumeSubjectJson(service, (ObjectNode) classNode.path(s), | ||
274 | subjectFactory.createSubject(s), | 294 | subjectFactory.createSubject(s), |
275 | - subjectFactory.subjectClassKey())); | 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