[ONOS-7703] FabricPipeliner does not remove NextGroup from store when remove NextObjective
Change-Id: Id945443606bd26e87f9e5236e97820bdbbe5b195
diff --git a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricPipeliner.java b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricPipeliner.java
index 61d3824..7cd154b 100644
--- a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricPipeliner.java
+++ b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricPipeliner.java
@@ -169,20 +169,29 @@
return;
}
- // Success, put next group to objective store
- List<PortNumber> portNumbers = Lists.newArrayList();
- nextObjective.next().forEach(treatment ->
- treatment.allInstructions()
- .stream()
- .filter(inst -> inst.type() == Instruction.Type.OUTPUT)
- .map(inst -> (Instructions.OutputInstruction) inst)
- .findFirst()
- .map(Instructions.OutputInstruction::port)
- .ifPresent(portNumbers::add)
- );
- FabricNextGroup nextGroup = new FabricNextGroup(nextObjective.type(),
- portNumbers);
- flowObjectiveStore.putNextGroup(nextObjective.id(), nextGroup);
+ if (nextObjective.op() == Objective.Operation.REMOVE) {
+ if (flowObjectiveStore.getNextGroup(nextObjective.id()) == null) {
+ log.warn("Can not find next obj {} from store", nextObjective.id());
+ return;
+ }
+ flowObjectiveStore.removeNextGroup(nextObjective.id());
+ } else {
+ // Success, put next group to objective store
+ List<PortNumber> portNumbers = Lists.newArrayList();
+ nextObjective.next().forEach(treatment ->
+ treatment.allInstructions()
+ .stream()
+ .filter(inst -> inst.type() == Instruction.Type.OUTPUT)
+ .map(inst -> (Instructions.OutputInstruction) inst)
+ .findFirst()
+ .map(Instructions.OutputInstruction::port)
+ .ifPresent(portNumbers::add)
+ );
+ FabricNextGroup nextGroup = new FabricNextGroup(nextObjective.type(),
+ portNumbers);
+ flowObjectiveStore.putNextGroup(nextObjective.id(), nextGroup);
+ }
+
success(nextObjective);
});
}
diff --git a/pipelines/fabric/src/test/java/org/onosproject/pipelines/fabric/pipeliner/FabricNextPipelinerTest.java b/pipelines/fabric/src/test/java/org/onosproject/pipelines/fabric/pipeliner/FabricNextPipelinerTest.java
index 0c8b509..1aab0d5 100644
--- a/pipelines/fabric/src/test/java/org/onosproject/pipelines/fabric/pipeliner/FabricNextPipelinerTest.java
+++ b/pipelines/fabric/src/test/java/org/onosproject/pipelines/fabric/pipeliner/FabricNextPipelinerTest.java
@@ -17,8 +17,12 @@
package org.onosproject.pipelines.fabric.pipeliner;
import com.google.common.collect.ImmutableList;
+import org.easymock.EasyMock;
import org.junit.Test;
+import org.onlab.junit.TestUtils;
import org.onlab.util.ImmutableByteSequence;
+import org.onosproject.net.behaviour.DefaultNextGroup;
+import org.onosproject.net.behaviour.NextGroup;
import org.onosproject.net.flow.DefaultFlowRule;
import org.onosproject.net.flow.DefaultTrafficSelector;
import org.onosproject.net.flow.DefaultTrafficTreatment;
@@ -27,7 +31,9 @@
import org.onosproject.net.flow.TrafficTreatment;
import org.onosproject.net.flow.criteria.PiCriterion;
import org.onosproject.net.flowobjective.DefaultNextObjective;
+import org.onosproject.net.flowobjective.FlowObjectiveStore;
import org.onosproject.net.flowobjective.NextObjective;
+import org.onosproject.net.flowobjective.Objective;
import org.onosproject.net.group.DefaultGroupBucket;
import org.onosproject.net.group.DefaultGroupDescription;
import org.onosproject.net.group.DefaultGroupKey;
@@ -42,8 +48,10 @@
import org.onosproject.pipelines.fabric.FabricConstants;
import java.util.List;
+import java.util.Map;
import java.util.stream.Collectors;
+import static org.easymock.EasyMock.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
@@ -382,4 +390,42 @@
);
assertEquals(expectedGroup, actualGroup);
}
+
+ /**
+ * Test removing next objective, and expect the next objective data will be removed from the store.
+ */
+ @Test
+ public void testRemoveNextObjective() {
+ // Initialize mock objects
+ NextObjective mockNextObjective = DefaultNextObjective.builder()
+ .fromApp(APP_ID)
+ .withId(NEXT_ID_1)
+ .makePermanent()
+ .withPriority(PRIORITY)
+ .withType(NextObjective.Type.SIMPLE)
+ .remove();
+ FlowObjectiveStore mockFlowObjStore = EasyMock.createNiceMock(FlowObjectiveStore.class);
+ FabricNextPipeliner mockNextPipeliner = EasyMock.createNiceMock(FabricNextPipeliner.class);
+ PipelinerTranslationResult mockResult = PipelinerTranslationResult.builder().build(); // empty result
+ NextGroup mockNextGroup = new DefaultNextGroup(null);
+ expect(mockNextPipeliner.next(mockNextObjective)).andReturn(mockResult).once();
+ expect(mockFlowObjStore.getNextGroup(mockNextObjective.id())).andReturn(mockNextGroup).once();
+ expect(mockFlowObjStore.removeNextGroup(mockNextObjective.id())).andReturn(mockNextGroup).once();
+ replay(mockNextPipeliner, mockFlowObjStore);
+
+ // Initialize the pipeliner
+ FabricPipeliner thePipeliner = new FabricPipeliner();
+ thePipeliner.pipelinerNext = mockNextPipeliner;
+ TestUtils.setField(thePipeliner, "flowObjectiveStore", mockFlowObjStore);
+ // execute removing process
+ thePipeliner.next(mockNextObjective);
+ Map<Objective, FabricPipeliner.PendingInstallObjective> pios =
+ TestUtils.getField(thePipeliner, "pendingInstallObjectives");
+ FabricPipeliner.PendingInstallObjective pio = pios.get(mockNextObjective);
+ pio.callback.accept(null); // applying successful result
+
+ // "removeNextGroup" method should be called once, or failed if not.
+ verify(mockNextPipeliner, mockFlowObjStore);
+
+ }
}