Committed by
Brian O'Connor
Handle exception when installing fails
- Catch an IntentException in InstallCoordinating and Installing - Remove FlowRuleBatchOperationConvertionException because it is unused Change-Id: I99cf07df811ba8489feb75088f83ddc4ebd93c9e
Showing
4 changed files
with
14 additions
and
58 deletions
1 | -/* | ||
2 | - * Copyright 2015 Open Networking Laboratory | ||
3 | - * | ||
4 | - * Licensed under the Apache License, Version 2.0 (the "License"); | ||
5 | - * you may not use this file except in compliance with the License. | ||
6 | - * You may obtain a copy of the License at | ||
7 | - * | ||
8 | - * http://www.apache.org/licenses/LICENSE-2.0 | ||
9 | - * | ||
10 | - * Unless required by applicable law or agreed to in writing, software | ||
11 | - * distributed under the License is distributed on an "AS IS" BASIS, | ||
12 | - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
13 | - * See the License for the specific language governing permissions and | ||
14 | - * limitations under the License. | ||
15 | - */ | ||
16 | -package org.onosproject.net.intent.impl; | ||
17 | - | ||
18 | -import com.google.common.collect.ImmutableList; | ||
19 | -import org.onosproject.net.flow.FlowRuleBatchOperation; | ||
20 | -import org.onosproject.net.intent.IntentException; | ||
21 | - | ||
22 | -import java.util.List; | ||
23 | - | ||
24 | -import static com.google.common.base.Preconditions.checkNotNull; | ||
25 | - | ||
26 | -// TODO: Reconsider error handling and intent exception design. Otherwise, write Javadoc. | ||
27 | -public class FlowRuleBatchOperationConversionException extends IntentException { | ||
28 | - | ||
29 | - private final List<FlowRuleBatchOperation> converted; | ||
30 | - | ||
31 | - public FlowRuleBatchOperationConversionException(List<FlowRuleBatchOperation> converted, Throwable cause) { | ||
32 | - super("exception occurred during IntentInstaller.install()", cause); | ||
33 | - this.converted = ImmutableList.copyOf((checkNotNull(converted))); | ||
34 | - } | ||
35 | - | ||
36 | - public List<FlowRuleBatchOperation> converted() { | ||
37 | - return converted; | ||
38 | - } | ||
39 | -} |
... | @@ -17,6 +17,7 @@ package org.onosproject.net.intent.impl; | ... | @@ -17,6 +17,7 @@ package org.onosproject.net.intent.impl; |
17 | 17 | ||
18 | import org.onosproject.net.flow.FlowRuleOperations; | 18 | import org.onosproject.net.flow.FlowRuleOperations; |
19 | import org.onosproject.net.intent.IntentData; | 19 | import org.onosproject.net.intent.IntentData; |
20 | +import org.onosproject.net.intent.IntentException; | ||
20 | import org.slf4j.Logger; | 21 | import org.slf4j.Logger; |
21 | import org.slf4j.LoggerFactory; | 22 | import org.slf4j.LoggerFactory; |
22 | 23 | ||
... | @@ -48,9 +49,9 @@ class InstallCoordinating implements IntentUpdate { | ... | @@ -48,9 +49,9 @@ class InstallCoordinating implements IntentUpdate { |
48 | try { | 49 | try { |
49 | FlowRuleOperations flowRules = intentManager.coordinate(pending); | 50 | FlowRuleOperations flowRules = intentManager.coordinate(pending); |
50 | return Optional.of(new Installing(intentManager, pending, flowRules)); | 51 | return Optional.of(new Installing(intentManager, pending, flowRules)); |
51 | - } catch (FlowRuleBatchOperationConversionException e) { | 52 | + } catch (IntentException e) { |
52 | - log.warn("Unable to install intent {} due to:", pending.intent().id(), e.getCause()); | 53 | + log.warn("Unable to generate a FlowRuleOperations from intent {} due to:", pending.intent().id(), e); |
53 | - return Optional.of(new InstallingFailed(pending)); //FIXME | 54 | + return Optional.of(new InstallingFailed(pending)); |
54 | } | 55 | } |
55 | } | 56 | } |
56 | } | 57 | } | ... | ... |
... | @@ -17,6 +17,7 @@ package org.onosproject.net.intent.impl; | ... | @@ -17,6 +17,7 @@ package org.onosproject.net.intent.impl; |
17 | 17 | ||
18 | import org.onosproject.net.flow.FlowRuleOperations; | 18 | import org.onosproject.net.flow.FlowRuleOperations; |
19 | import org.onosproject.net.intent.IntentData; | 19 | import org.onosproject.net.intent.IntentData; |
20 | +import org.onosproject.net.intent.IntentException; | ||
20 | import org.slf4j.Logger; | 21 | import org.slf4j.Logger; |
21 | import org.slf4j.LoggerFactory; | 22 | import org.slf4j.LoggerFactory; |
22 | 23 | ||
... | @@ -48,8 +49,10 @@ class Installing implements IntentUpdate { | ... | @@ -48,8 +49,10 @@ class Installing implements IntentUpdate { |
48 | try { | 49 | try { |
49 | intentManager.flowRuleService.apply(flowRules); // FIXME we need to provide a context | 50 | intentManager.flowRuleService.apply(flowRules); // FIXME we need to provide a context |
50 | return Optional.of(new Installed(pending)); | 51 | return Optional.of(new Installed(pending)); |
51 | - } catch (FlowRuleBatchOperationConversionException e) { | 52 | + // What kinds of exceptions are thrown by FlowRuleService.apply()? |
52 | - log.warn("Unable to install intent {} due to:", pending.intent().id(), e.getCause()); | 53 | + // Is IntentException a correct exception abstraction? |
54 | + } catch (IntentException e) { | ||
55 | + log.warn("Unable to install intent {} due to: {}", pending.intent().id(), e); | ||
53 | return Optional.of(new InstallingFailed(pending)); | 56 | return Optional.of(new InstallingFailed(pending)); |
54 | } | 57 | } |
55 | } | 58 | } | ... | ... |
... | @@ -288,14 +288,10 @@ public class IntentManager | ... | @@ -288,14 +288,10 @@ public class IntentManager |
288 | List<Intent> installables = pending.installables(); | 288 | List<Intent> installables = pending.installables(); |
289 | List<List<FlowRuleBatchOperation>> plans = new ArrayList<>(installables.size()); | 289 | List<List<FlowRuleBatchOperation>> plans = new ArrayList<>(installables.size()); |
290 | for (Intent installable : installables) { | 290 | for (Intent installable : installables) { |
291 | - try { | 291 | + registerSubclassInstallerIfNeeded(installable); |
292 | - registerSubclassInstallerIfNeeded(installable); | 292 | + //FIXME need to migrate installers to FlowRuleOperations |
293 | - //FIXME need to migrate installers to FlowRuleOperations | 293 | + // FIXME need to aggregate the FlowRuleOperations across installables |
294 | - // FIXME need to aggregate the FlowRuleOperations across installables | 294 | + plans.add(getInstaller(installable).install(installable)); |
295 | - plans.add(getInstaller(installable).install(installable)); | ||
296 | - } catch (Exception e) { // TODO this should be IntentException | ||
297 | - throw new FlowRuleBatchOperationConversionException(null/*FIXME*/, e); | ||
298 | - } | ||
299 | } | 295 | } |
300 | 296 | ||
301 | return merge(plans).build(new FlowRuleOperationsContext() { // FIXME move this out | 297 | return merge(plans).build(new FlowRuleOperationsContext() { // FIXME move this out |
... | @@ -325,12 +321,7 @@ public class IntentManager | ... | @@ -325,12 +321,7 @@ public class IntentManager |
325 | List<Intent> installables = current.installables(); | 321 | List<Intent> installables = current.installables(); |
326 | List<List<FlowRuleBatchOperation>> plans = new ArrayList<>(); | 322 | List<List<FlowRuleBatchOperation>> plans = new ArrayList<>(); |
327 | for (Intent installable : installables) { | 323 | for (Intent installable : installables) { |
328 | - try { | 324 | + plans.add(getInstaller(installable).uninstall(installable)); |
329 | - plans.add(getInstaller(installable).uninstall(installable)); | ||
330 | - } catch (IntentException e) { | ||
331 | - log.warn("Unable to uninstall intent {} due to:", current.intent().id(), e); | ||
332 | - throw new FlowRuleBatchOperationConversionException(null/*FIXME*/, e); | ||
333 | - } | ||
334 | } | 325 | } |
335 | 326 | ||
336 | return merge(plans).build(new FlowRuleOperationsContext() { | 327 | return merge(plans).build(new FlowRuleOperationsContext() { | ... | ... |
-
Please register or login to post a comment