Brian O'Connor
Committed by Gerrit Code Review

Initial implementation of CORRUPT state (ONOS-1060)

- Added CORRUPT state to state machine and event type
- Simplified phases using new request field
- Improved null-safety by using Optionals

Change-Id: I1d576b719765b5664aef73477ee04593e8acc4fd
Showing 17 changed files with 169 additions and 198 deletions
......@@ -22,16 +22,12 @@ import org.onosproject.store.Timestamp;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.onosproject.net.intent.IntentState.FAILED;
import static org.onosproject.net.intent.IntentState.INSTALLED;
import static org.onosproject.net.intent.IntentState.INSTALLING;
import static org.onosproject.net.intent.IntentState.PURGE_REQ;
import static org.onosproject.net.intent.IntentState.WITHDRAWING;
import static org.onosproject.net.intent.IntentState.WITHDRAWN;
import static org.onosproject.net.intent.IntentState.*;
/**
* A wrapper class that contains an intents, its state, and other metadata for
......@@ -44,6 +40,7 @@ public class IntentData { //FIXME need to make this "immutable"
private final Intent intent;
private final IntentState request; //TODO perhaps we want a full fledged object for requests
private IntentState state;
private Timestamp version;
private NodeId origin;
......@@ -60,6 +57,7 @@ public class IntentData { //FIXME need to make this "immutable"
public IntentData(Intent intent, IntentState state, Timestamp version) {
this.intent = intent;
this.state = state;
this.request = state;
this.version = version;
}
......@@ -73,6 +71,7 @@ public class IntentData { //FIXME need to make this "immutable"
intent = intentData.intent;
state = intentData.state;
request = intentData.request;
version = intentData.version;
origin = intentData.origin;
installables = intentData.installables;
......@@ -81,6 +80,7 @@ public class IntentData { //FIXME need to make this "immutable"
// kryo constructor
protected IntentData() {
intent = null;
request = null;
}
/**
......@@ -101,6 +101,10 @@ public class IntentData { //FIXME need to make this "immutable"
return state;
}
public IntentState request() {
return request;
}
/**
* Returns the intent key.
*
......@@ -175,7 +179,7 @@ public class IntentData { //FIXME need to make this "immutable"
* @return list of installable intents
*/
public List<Intent> installables() {
return installables;
return installables != null ? installables : Collections.emptyList();
}
/**
......@@ -240,6 +244,12 @@ public class IntentData { //FIXME need to make this "immutable"
}
return true;
case CORRUPT:
if (currentState == CORRUPT) {
return false;
}
return true;
case PURGE_REQ:
return true;
......
......@@ -34,7 +34,8 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> {
INSTALLED,
/**
* Signifies that an intent has failed compilation or installation.
* Signifies that an intent has failed compilation and that it cannot
* be satisfied by the network at this time.
*/
FAILED,
......@@ -49,6 +50,13 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> {
WITHDRAWN,
/**
* Signifies that an intent has failed installation or withdrawal, but
* still hold some or all of its resources.
* (e.g. link reservations, flow rules on the data plane, etc.)
*/
CORRUPT,
/**
* Signifies that an intent has been purged from the system.
*/
PURGED
......@@ -115,6 +123,9 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> {
case FAILED:
type = Type.FAILED;
break;
case CORRUPT:
type = Type.CORRUPT;
break;
case PURGE_REQ:
type = Type.PURGED;
break;
......
......@@ -89,10 +89,19 @@ public enum IntentState {
WITHDRAWN,
/**
* Signifies that the intent has failed compiling, installing or
* recompiling states.
* Signifies that the intent has failed to be installed and cannot be
* satisfied given current network conditions. But, the framework will
* reattempt to install it when network conditions change until it is
* withdrawn by an application.
*/
FAILED, //TODO consider renaming to UNSAT.
FAILED, //TODO consider renaming to UNSATISFIABLE
/**
* Signifies that an intent has failed either installation or withdrawal,
* and still hold some or all of its resources.
* (e.g. link reservations, flow rules on the data plane, etc.)
*/
CORRUPT, //TODO consider renaming to ERROR
/**
* Indicates that the intent should be purged from the database.
......
......@@ -15,15 +15,7 @@
*/
package org.onosproject.net.intent.impl;
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 com.google.common.collect.ImmutableList;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
......@@ -56,17 +48,21 @@ import org.onosproject.net.intent.impl.phase.IntentProcessPhase;
import org.onosproject.net.intent.impl.phase.IntentWorker;
import org.slf4j.Logger;
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.Optional;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.stream.Collectors;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.concurrent.Executors.newFixedThreadPool;
import static java.util.concurrent.Executors.newSingleThreadExecutor;
import static org.onlab.util.Tools.groupedThreads;
import static org.onosproject.net.intent.IntentState.FAILED;
import static org.onosproject.net.intent.IntentState.INSTALLED;
import static org.onosproject.net.intent.IntentState.INSTALL_REQ;
import static org.onosproject.net.intent.IntentState.WITHDRAWN;
import static org.onosproject.net.intent.IntentState.WITHDRAW_REQ;
import static org.onosproject.net.intent.IntentState.*;
import static org.onosproject.net.intent.impl.phase.IntentProcessPhase.newInitialPhase;
import static org.slf4j.LoggerFactory.getLogger;
......@@ -360,7 +356,7 @@ public class IntentManager
}
@Override
public void apply(IntentData toUninstall, IntentData toInstall) {
public void apply(Optional<IntentData> toUninstall, Optional<IntentData> toInstall) {
IntentManager.this.apply(toUninstall, toInstall);
}
}
......@@ -370,12 +366,13 @@ public class IntentManager
REMOVE
}
private void applyIntentData(IntentData data,
private void applyIntentData(Optional<IntentData> intentData,
FlowRuleOperations.Builder builder,
Direction direction) {
if (data == null) {
if (!intentData.isPresent()) {
return;
}
IntentData data = intentData.get();
List<Intent> intentsToApply = data.installables();
if (!intentsToApply.stream().allMatch(x -> x instanceof FlowRuleIntent)) {
......@@ -411,7 +408,7 @@ public class IntentManager
}
private void apply(IntentData toUninstall, IntentData toInstall) {
private void apply(Optional<IntentData> toUninstall, Optional<IntentData> toInstall) {
// need to consider if FlowRuleIntent is only one as installable intent or not
FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
......@@ -421,29 +418,45 @@ public class IntentManager
FlowRuleOperations operations = builder.build(new FlowRuleOperationsContext() {
@Override
public void onSuccess(FlowRuleOperations ops) {
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);
if (toInstall.isPresent()) {
IntentData installData = toInstall.get();
log.debug("Completed installing: {}", installData.key());
installData.setState(INSTALLED);
store.write(installData);
} else if (toUninstall.isPresent()) {
IntentData uninstallData = toUninstall.get();
log.debug("Completed withdrawing: {}", uninstallData.key());
switch (uninstallData.request()) {
case INSTALL_REQ:
uninstallData.setState(FAILED);
break;
case WITHDRAW_REQ:
default: //TODO "default" case should not happen
uninstallData.setState(WITHDRAWN);
break;
}
store.write(uninstallData);
}
}
@Override
public void onError(FlowRuleOperations ops) {
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 (toInstall.isPresent()) {
IntentData installData = toInstall.get();
log.warn("Failed installation: {} {} on {}",
installData.key(), installData.intent(), ops);
installData.setState(CORRUPT);
store.write(installData);
}
// if toUninstall was cause of error, then CORRUPT (another job will clean this up)
if (toUninstall.isPresent()) {
IntentData uninstallData = toUninstall.get();
log.warn("Failed withdrawal: {} {} on {}",
uninstallData.key(), uninstallData.intent(), ops);
uninstallData.setState(CORRUPT);
store.write(uninstallData);
}
}
});
......
......@@ -19,6 +19,7 @@ import org.onosproject.net.intent.Intent;
import org.onosproject.net.intent.IntentData;
import java.util.List;
import java.util.Optional;
/**
* A collection of methods to process an intent.
......@@ -38,8 +39,8 @@ public interface IntentProcessor {
List<Intent> compile(Intent intent, List<Intent> previousInstallables);
/**
* @param toUninstall Intent data describing flows to uninstall. May be null.
* @param toInstall Intent data describing flows to install. May be null.
* @param toUninstall Intent data describing flows to uninstall.
* @param toInstall Intent data describing flows to install.
*/
void apply(IntentData toUninstall, IntentData toInstall);
void apply(Optional<IntentData> toUninstall, Optional<IntentData> toInstall);
}
......
......@@ -15,46 +15,59 @@
*/
package org.onosproject.net.intent.impl.phase;
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;
import static com.google.common.base.Preconditions.checkNotNull;
/**
* Represents a phase where an intent is being compiled.
* Represents a phase where an intent is being compiled or recompiled.
*/
final class Compiling implements IntentProcessPhase {
class Compiling implements IntentProcessPhase {
private static final Logger log = LoggerFactory.getLogger(Compiling.class);
private final IntentProcessor processor;
private final IntentData data;
private final Optional<IntentData> stored;
/**
* Creates an compiling phase.
* Creates a intent recompiling phase.
*
* @param processor intent processor that does work for compiling
* @param data intent data containing an intent to be compiled
* @param processor intent processor that does work for recompiling
* @param data intent data containing an intent to be recompiled
* @param stored intent data stored in the store
*/
Compiling(IntentProcessor processor, IntentData data) {
Compiling(IntentProcessor processor, IntentData data, Optional<IntentData> stored) {
this.processor = checkNotNull(processor);
this.data = checkNotNull(data);
this.stored = checkNotNull(stored);
}
@Override
public Optional<IntentProcessPhase> execute() {
try {
data.setInstallables(processor.compile(data.intent(), null));
return Optional.of(new Installing(processor, data, null));
List<Intent> compiled = processor.compile(data.intent(),
//TODO consider passing an optional here in the future
stored.isPresent() ? stored.get().installables() : null);
data.setInstallables(compiled);
return Optional.of(new Installing(processor, data, stored));
} catch (IntentException e) {
log.debug("Unable to compile intent {} due to: {}", data.intent(), e);
return Optional.of(new CompileFailed(data));
if (stored.isPresent() && !stored.get().installables().isEmpty()) {
// removing orphaned flows and deallocating resources
data.setInstallables(stored.get().installables());
return Optional.of(new Withdrawing(processor, data));
} else {
return Optional.of(new Failed(data));
}
}
}
}
......
......@@ -17,17 +17,28 @@ package org.onosproject.net.intent.impl.phase;
import org.onosproject.net.intent.IntentData;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.onosproject.net.intent.IntentState.CORRUPT;
/**
* Represents a phase where the compile has failed.
* A class representing errors removing or installing intents.
*/
public class CompileFailed extends AbstractFailed {
public class Corrupt extends FinalIntentProcessPhase {
private final IntentData intentData;
/**
* Create an instance with the specified data.
*
* @param intentData intentData
*/
public CompileFailed(IntentData intentData) {
super(intentData);
Corrupt(IntentData intentData) {
this.intentData = checkNotNull(intentData);
this.intentData.setState(CORRUPT);
}
@Override
public IntentData data() {
return intentData;
}
}
......
......@@ -21,10 +21,9 @@ import static com.google.common.base.Preconditions.checkNotNull;
import static org.onosproject.net.intent.IntentState.FAILED;
/**
* A common parent class of a class representing failure
* as IntentUpdate subclass.
* Represents a phase where the compile has failed.
*/
abstract class AbstractFailed extends FinalIntentProcessPhase {
public class Failed extends FinalIntentProcessPhase {
private final IntentData intentData;
......@@ -33,7 +32,7 @@ abstract class AbstractFailed extends FinalIntentProcessPhase {
*
* @param intentData intentData
*/
AbstractFailed(IntentData intentData) {
Failed(IntentData intentData) {
this.intentData = checkNotNull(intentData);
this.intentData.setState(FAILED);
}
......
......@@ -47,10 +47,6 @@ final class InstallRequest implements IntentProcessPhase {
@Override
public Optional<IntentProcessPhase> execute() {
if (!stored.isPresent() || stored.get().installables() == null || stored.get().installables().isEmpty()) {
return Optional.of(new Compiling(processor, data));
} else {
return Optional.of(new Recompiling(processor, data, stored.get()));
}
return Optional.of(new Compiling(processor, data, stored));
}
}
......
......@@ -18,6 +18,8 @@ package org.onosproject.net.intent.impl.phase;
import org.onosproject.net.intent.IntentData;
import org.onosproject.net.intent.impl.IntentProcessor;
import java.util.Optional;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.onosproject.net.intent.IntentState.INSTALLING;
......@@ -28,7 +30,7 @@ class Installing extends FinalIntentProcessPhase {
private final IntentProcessor processor;
private final IntentData data;
private final IntentData stored;
private final Optional<IntentData> stored;
/**
* Create an installing phase.
......@@ -37,16 +39,16 @@ class Installing extends FinalIntentProcessPhase {
* @param data intent data containing an intent to be installed
* @param stored intent data already stored
*/
Installing(IntentProcessor processor, IntentData data, IntentData stored) {
Installing(IntentProcessor processor, IntentData data, Optional<IntentData> stored) {
this.processor = checkNotNull(processor);
this.data = checkNotNull(data);
this.stored = checkNotNull(stored);
this.data.setState(INSTALLING);
this.stored = stored;
}
@Override
public void preExecute() {
processor.apply(stored, data);
processor.apply(stored, Optional.of(data));
}
@Override
......
......@@ -20,8 +20,6 @@ import org.onosproject.net.intent.impl.IntentProcessor;
import java.util.Optional;
import static org.onlab.util.Tools.isNullOrEmpty;
/**
* Represents a phase of processing an intent.
*/
......@@ -46,20 +44,16 @@ public interface IntentProcessPhase {
*/
static IntentProcessPhase newInitialPhase(IntentProcessor processor,
IntentData data, IntentData current) {
switch (data.state()) {
switch (data.request()) {
case INSTALL_REQ:
return new InstallRequest(processor, data, Optional.ofNullable(current));
case WITHDRAW_REQ:
if (current == null || isNullOrEmpty(current.installables())) {
return new Withdrawn(data);
} else {
return new WithdrawRequest(processor, data, current);
}
return new WithdrawRequest(processor, data, Optional.ofNullable(current));
case PURGE_REQ:
return new PurgeRequest(data, current);
default:
// illegal state
return new CompileFailed(data);
return new Failed(data);
}
}
......
/*
* 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.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;
import static com.google.common.base.Preconditions.checkNotNull;
/**
* Represents a phase where an intent is being recompiled.
*/
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;
/**
* Creates a intent recompiling phase.
*
* @param processor intent processor that does work for recompiling
* @param data intent data containing an intent to be recompiled
* @param stored intent data stored in the store
*/
Recompiling(IntentProcessor processor, IntentData data, IntentData stored) {
this.processor = checkNotNull(processor);
this.data = checkNotNull(data);
this.stored = checkNotNull(stored);
}
@Override
public Optional<IntentProcessPhase> execute() {
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));
}
}
}
/*
* 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;
/**
* Represent a phase where the install has failed.
*/
class ReplaceFailed extends AbstractFailed {
/**
* Create an instance with the specified data.
*
* @param intentData intentData
*/
ReplaceFailed(IntentData intentData) {
super(intentData);
}
}
......@@ -29,7 +29,7 @@ final class WithdrawRequest implements IntentProcessPhase {
private final IntentProcessor processor;
private final IntentData data;
private final IntentData stored;
private final Optional<IntentData> stored;
/**
* Creates a withdraw request phase.
......@@ -39,7 +39,7 @@ final class WithdrawRequest implements IntentProcessPhase {
* @param intentData intent data to be processed
* @param stored intent data stored in the store
*/
WithdrawRequest(IntentProcessor processor, IntentData intentData, IntentData stored) {
WithdrawRequest(IntentProcessor processor, IntentData intentData, Optional<IntentData> stored) {
this.processor = checkNotNull(processor);
this.data = checkNotNull(intentData);
this.stored = checkNotNull(stored);
......@@ -50,7 +50,18 @@ final class WithdrawRequest implements IntentProcessPhase {
//TODO perhaps we want to validate that the pending and current are the
// same version i.e. they are the same
// Note: this call is not just the symmetric version of submit
data.setInstallables(stored.installables());
if (!stored.isPresent() || stored.get().installables().isEmpty()) {
switch (data.request()) {
case INSTALL_REQ:
return Optional.of(new Failed(data));
case WITHDRAW_REQ:
default: //TODO "default" case should not happen
return Optional.of(new Withdrawn(data));
}
}
data.setInstallables(stored.get().installables());
return Optional.of(new Withdrawing(processor, data));
}
}
......
......@@ -18,6 +18,8 @@ package org.onosproject.net.intent.impl.phase;
import org.onosproject.net.intent.IntentData;
import org.onosproject.net.intent.impl.IntentProcessor;
import java.util.Optional;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.onosproject.net.intent.IntentState.WITHDRAWING;
......@@ -38,13 +40,12 @@ class Withdrawing extends FinalIntentProcessPhase {
Withdrawing(IntentProcessor processor, IntentData data) {
this.processor = checkNotNull(processor);
this.data = checkNotNull(data);
this.data.setState(WITHDRAWING);
}
@Override
protected void preExecute() {
processor.apply(data, null);
processor.apply(Optional.of(data), Optional.empty());
}
@Override
......
......@@ -65,9 +65,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.onlab.junit.TestTools.assertAfter;
import static org.onlab.util.Tools.delay;
import static org.onosproject.net.intent.IntentState.FAILED;
import static org.onosproject.net.intent.IntentState.INSTALLED;
import static org.onosproject.net.intent.IntentState.WITHDRAWN;
import static org.onosproject.net.intent.IntentState.*;
import static org.onosproject.net.intent.IntentTestsMocks.MockFlowRule;
import static org.onosproject.net.intent.IntentTestsMocks.MockIntent;
......@@ -243,7 +241,7 @@ public class IntentManagerTest {
public void verifyState() {
// verify that all intents are parked and the batch operation is unblocked
Set<IntentState> parked = Sets.newHashSet(INSTALLED, WITHDRAWN, FAILED);
Set<IntentState> parked = Sets.newHashSet(INSTALLED, WITHDRAWN, FAILED, CORRUPT);
for (Intent i : service.getIntents()) {
IntentState state = service.getIntentState(i.key());
assertTrue("Intent " + i.id() + " is in invalid state " + state,
......@@ -395,7 +393,7 @@ public class IntentManagerTest {
final Long id = MockIntent.nextId();
flowRuleService.setFuture(false);
MockIntent intent = new MockIntent(id);
listener.setLatch(1, Type.FAILED);
listener.setLatch(1, Type.CORRUPT);
listener.setLatch(1, Type.INSTALL_REQ);
service.submit(intent);
listener.await(Type.INSTALL_REQ);
......@@ -403,7 +401,7 @@ public class IntentManagerTest {
delay(100);
flowRuleService.setFuture(false);
service.withdraw(intent);
listener.await(Type.FAILED);
listener.await(Type.CORRUPT);
verifyState();
}
......@@ -462,6 +460,7 @@ public class IntentManagerTest {
* Tests an intent with no installer.
*/
@Test
@Ignore //FIXME corrupt or failed?
public void intentWithoutInstaller() {
MockIntent intent = new MockIntent(MockIntent.nextId());
listener.setLatch(1, Type.INSTALL_REQ);
......
......@@ -121,7 +121,7 @@ public class CompilingTest {
expect(processor.compile(input, null)).andReturn(Arrays.asList(compiled));
replay(processor);
Compiling sut = new Compiling(processor, pending);
Compiling sut = new Compiling(processor, pending, Optional.empty());
Optional<IntentProcessPhase> output = sut.execute();
......@@ -139,11 +139,11 @@ public class CompilingTest {
expect(processor.compile(input, null)).andThrow(new IntentCompilationException());
replay(processor);
Compiling sut = new Compiling(processor, pending);
Compiling sut = new Compiling(processor, pending, Optional.empty());
Optional<IntentProcessPhase> output = sut.execute();
verify(processor);
assertThat(output.get(), is(instanceOf(CompileFailed.class)));
assertThat(output.get(), is(instanceOf(Failed.class)));
}
}
......