ONOS-1404 - Rerouting delayed due to flows being removed out of order
ONOS-1409 - handle all links to a switch going away and coming back
Change-Id: Ib7216530fa8eef41b1088c5122a40bf9265481ca
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
index ddccb53..610b030 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
@@ -15,7 +15,15 @@
*/
package org.onosproject.net.intent.impl;
-import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -48,15 +56,7 @@
import org.onosproject.net.intent.impl.phase.IntentWorker;
import org.slf4j.Logger;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.EnumSet;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
-import java.util.stream.Collectors;
+import com.google.common.collect.ImmutableList;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.concurrent.Executors.newFixedThreadPool;
@@ -359,97 +359,92 @@
}
@Override
- public void install(IntentData data) {
- IntentManager.this.install(data);
+ public void apply(IntentData toUninstall, IntentData toInstall) {
+ IntentManager.this.apply(toUninstall, toInstall);
}
+ }
- @Override
- public void uninstall(IntentData data) {
- IntentManager.this.uninstall(data);
+ private enum Direction {
+ ADD,
+ REMOVE
+ }
+
+ private void applyIntentData(IntentData data,
+ FlowRuleOperations.Builder builder,
+ Direction direction) {
+ if (data != null) {
+ List<Intent> intentsToApply = data.installables();
+ if (!intentsToApply.stream().allMatch(x -> x instanceof FlowRuleIntent)) {
+ throw new IllegalStateException("installable intents must be FlowRuleIntent");
+ }
+
+ if (direction == Direction.ADD) {
+ trackerService.addTrackedResources(data.key(), data.intent().resources());
+ intentsToApply.forEach(installable ->
+ trackerService.addTrackedResources(data.key(), installable.resources()));
+ } else {
+ trackerService.removeTrackedResources(data.key(), data.intent().resources());
+ intentsToApply.forEach(installable ->
+ trackerService.removeTrackedResources(data.intent().key(),
+ installable.resources()));
+ }
+
+ // FIXME do FlowRuleIntents have stages??? Can we do uninstall work in parallel? I think so.
+ builder.newStage();
+
+ List<Collection<FlowRule>> stages = intentsToApply.stream()
+ .map(x -> (FlowRuleIntent) x)
+ .map(FlowRuleIntent::flowRules)
+ .collect(Collectors.toList());
+
+ for (Collection<FlowRule> rules : stages) {
+ if (direction == Direction.ADD) {
+ rules.forEach(builder::add);
+ } else {
+ rules.forEach(builder::remove);
+ }
+ }
}
}
- private void install(IntentData data) {
+ private void apply(IntentData toUninstall, IntentData toInstall) {
// need to consider if FlowRuleIntent is only one as installable intent or not
- List<Intent> installables = data.installables();
- if (!installables.stream().allMatch(x -> x instanceof FlowRuleIntent)) {
- throw new IllegalStateException("installable intents must be FlowRuleIntent");
- }
-
- trackerService.addTrackedResources(data.key(), data.intent().resources());
- installables.forEach(installable ->
- trackerService.addTrackedResources(data.key(), installable.resources()));
-
- List<Collection<FlowRule>> stages = installables.stream()
- .map(x -> (FlowRuleIntent) x)
- .map(FlowRuleIntent::flowRules)
- .collect(Collectors.toList());
FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
- for (Collection<FlowRule> rules : stages) {
- rules.forEach(builder::add);
- builder.newStage();
- }
+ applyIntentData(toUninstall, builder, Direction.REMOVE);
+ applyIntentData(toInstall, builder, Direction.ADD);
FlowRuleOperations operations = builder.build(new FlowRuleOperationsContext() {
@Override
public void onSuccess(FlowRuleOperations ops) {
- log.debug("Completed installing: {}", data.key());
- data.setState(INSTALLED);
- store.write(data);
+ if (toInstall != null) {
+ log.debug("Completed installing: {}", toInstall.key());
+ //FIXME state depends on install.... we might want to pass this in.
+ toInstall.setState(INSTALLED);
+ store.write(toInstall);
+ } else if (toUninstall != null) {
+ log.debug("Completed withdrawing: {}", toUninstall.key());
+ //FIXME state depends on install.... we might want to pass this in.
+ toUninstall.setState(WITHDRAWN);
+ store.write(toUninstall);
+ }
}
@Override
public void onError(FlowRuleOperations ops) {
- log.warn("Failed installation: {} {} on {}", data.key(), data.intent(), ops);
- data.setState(FAILED);
- store.write(data);
+ if (toInstall != null) {
+ log.warn("Failed installation: {} {} on {}", toInstall.key(), toInstall.intent(), ops);
+ //FIXME
+ toInstall.setState(FAILED);
+ store.write(toInstall);
+ }
+ // if toInstall was cause of error, then recompile (manage/increment counter, when exceeded -> CORRUPT)
+ // if toUninstall was cause of error, then CORRUPT (another job will lean this up)
}
});
flowRuleService.apply(operations);
}
- private void uninstall(IntentData data) {
- List<Intent> installables = data.installables();
- if (!installables.stream().allMatch(x -> x instanceof FlowRuleIntent)) {
- throw new IllegalStateException("installable intents must be FlowRuleIntent");
- }
-
- trackerService.removeTrackedResources(data.key(), data.intent().resources());
- installables.forEach(installable ->
- trackerService.removeTrackedResources(data.intent().key(),
- installable.resources()));
-
- List<Collection<FlowRule>> stages = installables.stream()
- .map(x -> (FlowRuleIntent) x)
- .map(FlowRuleIntent::flowRules)
- .collect(Collectors.toList());
-
- FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
- for (Collection<FlowRule> rules : stages) {
- rules.forEach(builder::remove);
- builder.newStage();
- }
-
- FlowRuleOperations operations = builder.build(new FlowRuleOperationsContext() {
- @Override
- public void onSuccess(FlowRuleOperations ops) {
- log.debug("Completed withdrawing: {}", data.key());
- data.setState(WITHDRAWN);
- data.setInstallables(Collections.emptyList());
- store.write(data);
- }
-
- @Override
- public void onError(FlowRuleOperations ops) {
- log.warn("Failed withdraw: {}", data.key());
- data.setState(FAILED);
- store.write(data);
- }
- });
-
- flowRuleService.apply(operations);
- }
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentProcessor.java b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentProcessor.java
index d946804..766c91fb 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentProcessor.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentProcessor.java
@@ -38,16 +38,8 @@
List<Intent> compile(Intent intent, List<Intent> previousInstallables);
/**
- * Installs an intent included in the specified intent data.
- *
- * @param data intent data containing an intent to be installed
+ * @param toUninstall Intent data describing flows to uninstall. May be null.
+ * @param toInstall Intent data describing flows to install. May be null.
*/
- void install(IntentData data);
-
- /**
- * Uninstalls an intent included in the specified intent data.
- *
- * @param data intent data containing an intent to be uninstalled
- */
- void uninstall(IntentData data);
+ void apply(IntentData toUninstall, IntentData toInstall);
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Compiling.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Compiling.java
index 060ae53..fe2986f 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Compiling.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Compiling.java
@@ -50,7 +50,7 @@
public Optional<IntentProcessPhase> execute() {
try {
data.setInstallables(processor.compile(data.intent(), null));
- return Optional.of(new Installing(processor, data));
+ return Optional.of(new Installing(processor, data, null));
} catch (IntentException e) {
log.debug("Unable to compile intent {} due to: {}", data.intent(), e);
return Optional.of(new CompileFailed(data));
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Installing.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Installing.java
index de042e5..f743abe 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Installing.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Installing.java
@@ -28,6 +28,7 @@
private final IntentProcessor processor;
private final IntentData data;
+ private final IntentData stored;
/**
* Create an installing phase.
@@ -35,15 +36,16 @@
* @param processor intent processor that does work for installing
* @param data intent data containing an intent to be installed
*/
- Installing(IntentProcessor processor, IntentData data) {
+ Installing(IntentProcessor processor, IntentData data, IntentData stored) {
this.processor = checkNotNull(processor);
this.data = checkNotNull(data);
this.data.setState(INSTALLING);
+ this.stored = stored;
}
@Override
public void preExecute() {
- processor.install(data);
+ processor.apply(stored, data);
}
@Override
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Recompiling.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Recompiling.java
index 110f42f..9e27070 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Recompiling.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Recompiling.java
@@ -17,7 +17,10 @@
import org.onosproject.net.intent.Intent;
import org.onosproject.net.intent.IntentData;
+import org.onosproject.net.intent.IntentException;
import org.onosproject.net.intent.impl.IntentProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.List;
import java.util.Optional;
@@ -29,6 +32,8 @@
*/
class Recompiling implements IntentProcessPhase {
+ private static final Logger log = LoggerFactory.getLogger(Recompiling.class);
+
private final IntentProcessor processor;
private final IntentData data;
private final IntentData stored;
@@ -48,8 +53,14 @@
@Override
public Optional<IntentProcessPhase> execute() {
- List<Intent> compiled = processor.compile(data.intent(), stored.installables());
- data.setInstallables(compiled);
- return Optional.of(new Replacing(processor, data, stored));
+ try {
+ List<Intent> compiled = processor.compile(data.intent(), stored.installables());
+ data.setInstallables(compiled);
+ return Optional.of(new Installing(processor, data, stored));
+ } catch (IntentException e) {
+ log.debug("Unable to recompile intent {} due to: {}", data.intent(), e);
+ // FIXME we need to removed orphaned flows and deallocate resources
+ return Optional.of(new CompileFailed(data));
+ }
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Replacing.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Replacing.java
deleted file mode 100644
index 911139c..0000000
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Replacing.java
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Copyright 2015 Open Networking Laboratory
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.onosproject.net.intent.impl.phase;
-
-import org.onosproject.net.intent.IntentData;
-import org.onosproject.net.intent.IntentException;
-import org.onosproject.net.intent.impl.IntentProcessor;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.Optional;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-
-/**
- * Represents a phase to replace an intent.
- */
-final class Replacing implements IntentProcessPhase {
-
- private static final Logger log = LoggerFactory.getLogger(Replacing.class);
-
- private final IntentProcessor processor;
- private final IntentData data;
- private final IntentData stored;
-
- /**
- * Creates a replacing phase.
- *
- * @param processor intent processor that does work for replacing
- * @param data intent data containing an intent to be replaced
- * @param stored intent data stored in the store
- */
- Replacing(IntentProcessor processor, IntentData data, IntentData stored) {
- this.processor = checkNotNull(processor);
- this.data = checkNotNull(data);
- this.stored = checkNotNull(stored);
- }
-
- @Override
- public Optional<IntentProcessPhase> execute() {
- try {
- processor.uninstall(stored);
- return Optional.of(new Installing(processor, data));
- } catch (IntentException e) {
- log.warn("Unable to generate a FlowRuleOperations from intent {} due to:", data.intent().id(), e);
- return Optional.of(new ReplaceFailed(data));
- }
- }
-}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Withdrawing.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Withdrawing.java
index 9351a96..052cfb5 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Withdrawing.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Withdrawing.java
@@ -44,7 +44,7 @@
@Override
protected void preExecute() {
- processor.uninstall(data);
+ processor.apply(data, null);
}
@Override
diff --git a/core/net/src/test/java/org/onosproject/net/intent/impl/phase/ReplacingTest.java b/core/net/src/test/java/org/onosproject/net/intent/impl/phase/ReplacingTest.java
deleted file mode 100644
index e646a3f..0000000
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/phase/ReplacingTest.java
+++ /dev/null
@@ -1,158 +0,0 @@
-/*
- * Copyright 2015 Open Networking Laboratory
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.onosproject.net.intent.impl.phase;
-
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.onosproject.TestApplicationId;
-import org.onosproject.core.ApplicationId;
-import org.onosproject.core.IdGenerator;
-import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.DefaultLink;
-import org.onosproject.net.DefaultPath;
-import org.onosproject.net.Link;
-import org.onosproject.net.Path;
-import org.onosproject.net.flow.DefaultTrafficSelector;
-import org.onosproject.net.flow.DefaultTrafficTreatment;
-import org.onosproject.net.flow.TrafficSelector;
-import org.onosproject.net.flow.TrafficTreatment;
-import org.onosproject.net.intent.Intent;
-import org.onosproject.net.intent.IntentData;
-import org.onosproject.net.intent.MockIdGenerator;
-import org.onosproject.net.intent.PathIntent;
-import org.onosproject.net.intent.PointToPointIntent;
-import org.onosproject.net.intent.impl.IntentInstallationException;
-import org.onosproject.net.intent.impl.IntentProcessor;
-import org.onosproject.net.provider.ProviderId;
-import org.onosproject.store.Timestamp;
-
-import java.util.Arrays;
-import java.util.List;
-import java.util.Optional;
-
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.expectLastCall;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertThat;
-import static org.onosproject.net.DeviceId.deviceId;
-import static org.onosproject.net.Link.Type.DIRECT;
-import static org.onosproject.net.PortNumber.portNumber;
-import static org.onosproject.net.intent.IntentState.INSTALLED;
-import static org.onosproject.net.intent.IntentState.INSTALL_REQ;
-
-/**
- * Unit tests for InstallingCoordinating phase.
- */
-public class ReplacingTest {
-
- private final ApplicationId appId = new TestApplicationId("test");
- private final ProviderId pid = new ProviderId("of", "test");
- private final TrafficSelector selector = DefaultTrafficSelector.emptySelector();
- private final TrafficTreatment treatment = DefaultTrafficTreatment.emptyTreatment();
- private final ConnectPoint cp1 = new ConnectPoint(deviceId("1"), portNumber(1));
- private final ConnectPoint cp2 = new ConnectPoint(deviceId("1"), portNumber(2));
- private final ConnectPoint cp3 = new ConnectPoint(deviceId("2"), portNumber(1));
- private final ConnectPoint cp4 = new ConnectPoint(deviceId("2"), portNumber(2));
-
- private final List<Link> links = Arrays.asList(new DefaultLink(pid, cp2, cp4, DIRECT));
- private final Path path = new DefaultPath(pid, links, 10);
-
- private PointToPointIntent input;
- private PathIntent compiled;
-
- private IdGenerator idGenerator;
- private IntentProcessor processor;
- private Timestamp version;
-
- @Before
- public void setUp() {
- processor = createMock(IntentProcessor.class);
- version = createMock(Timestamp.class);
-
- idGenerator = new MockIdGenerator();
-
- Intent.bindIdGenerator(idGenerator);
-
- // Intent creation should be placed after binding an ID generator
- input = PointToPointIntent.builder()
- .appId(appId)
- .selector(selector)
- .treatment(treatment)
- .ingressPoint(cp1)
- .egressPoint(cp3)
- .build();
- compiled = PathIntent.builder()
- .appId(appId)
- .selector(selector)
- .treatment(treatment)
- .path(path)
- .build();
- }
-
-
- @After
- public void tearDown() {
- Intent.unbindIdGenerator(idGenerator);
- }
-
- /**
- * Tests a next phase when no exception occurs.
- */
- @Test
- public void testMoveToNextPhaseWithoutError() {
- IntentData pending = new IntentData(input, INSTALL_REQ, version);
- pending.setInstallables(Arrays.asList(compiled));
-
- IntentData current = new IntentData(input, INSTALLED, version);
- current.setInstallables(Arrays.asList(compiled));
-
- processor.uninstall(current);
- replay(processor);
-
- Replacing sut = new Replacing(processor, pending, current);
-
- Optional<IntentProcessPhase> executed = sut.execute();
-
- verify(processor);
- assertThat(executed.get(), is(instanceOf(Installing.class)));
- }
-
- /**
- * Tests a next phase when IntentInstallationException occurs.
- */
- @Test
- public void testWhenIntentInstallExceptionOccurs() {
- IntentData pending = new IntentData(input, INSTALL_REQ, version);
- pending.setInstallables(Arrays.asList(compiled));
-
- IntentData current = new IntentData(input, INSTALLED, version);
-
- processor.uninstall(current);
- expectLastCall().andThrow(new IntentInstallationException());
- replay(processor);
-
- Replacing sut = new Replacing(processor, pending, current);
-
- Optional<IntentProcessPhase> executed = sut.execute();
-
- verify(processor);
- assertThat(executed.get(), is(instanceOf(ReplaceFailed.class)));
- }
-}