Committed by
Ray Milkey
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
67 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 | } | ... | ... |
... | @@ -17,6 +17,8 @@ package org.onosproject.rest.resources; | ... | @@ -17,6 +17,8 @@ package org.onosproject.rest.resources; |
17 | 17 | ||
18 | import java.io.IOException; | 18 | import java.io.IOException; |
19 | import java.io.InputStream; | 19 | import java.io.InputStream; |
20 | +import java.util.ArrayList; | ||
21 | +import java.util.List; | ||
20 | import java.util.Set; | 22 | import java.util.Set; |
21 | 23 | ||
22 | import javax.ws.rs.Consumes; | 24 | import javax.ws.rs.Consumes; |
... | @@ -34,6 +36,7 @@ import org.onosproject.net.config.NetworkConfigService; | ... | @@ -34,6 +36,7 @@ import org.onosproject.net.config.NetworkConfigService; |
34 | import org.onosproject.net.config.SubjectFactory; | 36 | import org.onosproject.net.config.SubjectFactory; |
35 | import org.onosproject.rest.AbstractWebResource; | 37 | import org.onosproject.rest.AbstractWebResource; |
36 | 38 | ||
39 | +import com.fasterxml.jackson.databind.ObjectMapper; | ||
37 | import com.fasterxml.jackson.databind.node.ObjectNode; | 40 | import com.fasterxml.jackson.databind.node.ObjectNode; |
38 | 41 | ||
39 | import static org.onlab.util.Tools.emptyIsNotFound; | 42 | import static org.onlab.util.Tools.emptyIsNotFound; |
... | @@ -45,6 +48,8 @@ import static org.onlab.util.Tools.nullIsNotFound; | ... | @@ -45,6 +48,8 @@ import static org.onlab.util.Tools.nullIsNotFound; |
45 | @Path("network/configuration") | 48 | @Path("network/configuration") |
46 | public class NetworkConfigWebResource extends AbstractWebResource { | 49 | public class NetworkConfigWebResource extends AbstractWebResource { |
47 | 50 | ||
51 | + //FIX ME not found Multi status error code 207 in jaxrs Response Status. | ||
52 | + private static final int MULTI_STATUS_RESPONE = 207; | ||
48 | 53 | ||
49 | private String subjectClassNotFoundErrorString(String subjectClassKey) { | 54 | private String subjectClassNotFoundErrorString(String subjectClassKey) { |
50 | return "Config for '" + subjectClassKey + "' not found"; | 55 | return "Config for '" + subjectClassKey + "' not found"; |
... | @@ -194,9 +199,16 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -194,9 +199,16 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
194 | public Response upload(InputStream request) throws IOException { | 199 | public Response upload(InputStream request) throws IOException { |
195 | NetworkConfigService service = get(NetworkConfigService.class); | 200 | NetworkConfigService service = get(NetworkConfigService.class); |
196 | ObjectNode root = (ObjectNode) mapper().readTree(request); | 201 | ObjectNode root = (ObjectNode) mapper().readTree(request); |
202 | + List<String> errorMsgs = new ArrayList<String>(); | ||
197 | root.fieldNames() | 203 | root.fieldNames() |
198 | - .forEachRemaining(sk -> consumeJson(service, (ObjectNode) root.path(sk), | 204 | + .forEachRemaining(sk -> |
199 | - service.getSubjectFactory(sk))); | 205 | + { |
206 | + errorMsgs.addAll(consumeJson(service, (ObjectNode) root.path(sk), | ||
207 | + service.getSubjectFactory(sk))); | ||
208 | + }); | ||
209 | + if (errorMsgs.toString().length() > 0) { | ||
210 | + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); | ||
211 | + } | ||
200 | return Response.ok().build(); | 212 | return Response.ok().build(); |
201 | } | 213 | } |
202 | 214 | ||
... | @@ -216,7 +228,10 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -216,7 +228,10 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
216 | InputStream request) throws IOException { | 228 | InputStream request) throws IOException { |
217 | NetworkConfigService service = get(NetworkConfigService.class); | 229 | NetworkConfigService service = get(NetworkConfigService.class); |
218 | ObjectNode root = (ObjectNode) mapper().readTree(request); | 230 | ObjectNode root = (ObjectNode) mapper().readTree(request); |
219 | - consumeJson(service, root, service.getSubjectFactory(subjectClassKey)); | 231 | + List<String> errorMsgs = consumeJson(service, root, service.getSubjectFactory(subjectClassKey)); |
232 | + if (errorMsgs.toString().length() > 0) { | ||
233 | + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); | ||
234 | + } | ||
220 | return Response.ok().build(); | 235 | return Response.ok().build(); |
221 | } | 236 | } |
222 | 237 | ||
... | @@ -238,9 +253,12 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -238,9 +253,12 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
238 | InputStream request) throws IOException { | 253 | InputStream request) throws IOException { |
239 | NetworkConfigService service = get(NetworkConfigService.class); | 254 | NetworkConfigService service = get(NetworkConfigService.class); |
240 | ObjectNode root = (ObjectNode) mapper().readTree(request); | 255 | ObjectNode root = (ObjectNode) mapper().readTree(request); |
241 | - consumeSubjectJson(service, root, | 256 | + List<String> errorMsgs = consumeSubjectJson(service, root, |
242 | - service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), | 257 | + service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), |
243 | - subjectClassKey); | 258 | + subjectClassKey); |
259 | + if (errorMsgs.size() > 0) { | ||
260 | + return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build(); | ||
261 | + } | ||
244 | return Response.ok().build(); | 262 | return Response.ok().build(); |
245 | } | 263 | } |
246 | 264 | ||
... | @@ -270,21 +288,37 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -270,21 +288,37 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
270 | return Response.ok().build(); | 288 | return Response.ok().build(); |
271 | } | 289 | } |
272 | 290 | ||
273 | - private void consumeJson(NetworkConfigService service, ObjectNode classNode, | 291 | + private List<String> consumeJson(NetworkConfigService service, ObjectNode classNode, |
274 | SubjectFactory subjectFactory) { | 292 | SubjectFactory subjectFactory) { |
275 | - classNode.fieldNames().forEachRemaining(s -> | 293 | + List<String> errorMsgs = new ArrayList<String>(); |
276 | - consumeSubjectJson(service, (ObjectNode) classNode.path(s), | 294 | + classNode.fieldNames().forEachRemaining(s -> { |
277 | - subjectFactory.createSubject(s), | 295 | + List<String> error = consumeSubjectJson(service, (ObjectNode) classNode.path(s), |
278 | - subjectFactory.subjectClassKey())); | 296 | + subjectFactory.createSubject(s), |
297 | + subjectFactory.subjectClassKey()); | ||
298 | + errorMsgs.addAll(error); | ||
299 | + }); | ||
300 | + return errorMsgs; | ||
279 | } | 301 | } |
280 | 302 | ||
281 | - private void consumeSubjectJson(NetworkConfigService service, | 303 | + private List<String> consumeSubjectJson(NetworkConfigService service, |
282 | ObjectNode subjectNode, Object subject, | 304 | ObjectNode subjectNode, Object subject, |
283 | String subjectClassKey) { | 305 | String subjectClassKey) { |
284 | - subjectNode.fieldNames().forEachRemaining(configKey -> | 306 | + List<String> errorMsgs = new ArrayList<String>(); |
285 | - service.applyConfig(subjectClassKey, subject, configKey, subjectNode.path(configKey))); | 307 | + subjectNode.fieldNames().forEachRemaining(configKey -> { |
308 | + try { | ||
309 | + service.applyConfig(subjectClassKey, subject, configKey, subjectNode.path(configKey)); | ||
310 | + } catch (IllegalArgumentException e) { | ||
311 | + errorMsgs.add("Error parsing config " + subjectClassKey + "/" + subject + "/" + configKey); | ||
312 | + } | ||
313 | + }); | ||
314 | + return errorMsgs; | ||
286 | } | 315 | } |
287 | 316 | ||
317 | + private ObjectNode produceErrorJson(List<String> errorMsgs) { | ||
318 | + ObjectMapper mapper = new ObjectMapper(); | ||
319 | + ObjectNode result = mapper.createObjectNode().put("code", 207).putPOJO("message", errorMsgs); | ||
320 | + return result; | ||
321 | + } | ||
288 | 322 | ||
289 | // FIXME: Refactor to allow queued configs to be removed | 323 | // FIXME: Refactor to allow queued configs to be removed |
290 | 324 | ... | ... |
-
Please register or login to post a comment