ONOS-944: Implemented a Group operation failure handler in GroupManger and SimpleGroupStore.
Change-Id: Ib3be4d534ceff04af2dad0c062fd4cd63d49ee82
diff --git a/core/api/src/main/java/org/onosproject/net/group/GroupEvent.java b/core/api/src/main/java/org/onosproject/net/group/GroupEvent.java
index d9d8a97..45fbb3e 100644
--- a/core/api/src/main/java/org/onosproject/net/group/GroupEvent.java
+++ b/core/api/src/main/java/org/onosproject/net/group/GroupEvent.java
@@ -41,6 +41,21 @@
*/
GROUP_UPDATED,
+ /**
+ * Signifies that a request to create Group has failed.
+ */
+ GROUP_ADD_FAILED,
+
+ /**
+ * Signifies that a request to remove Group has failed.
+ */
+ GROUP_REMOVE_FAILED,
+
+ /**
+ * Signifies that a request to update Group has failed.
+ */
+ GROUP_UPDATE_FAILED,
+
// internal event between Manager <-> Store
/*
@@ -55,6 +70,8 @@
* Signifies that a request to delete Group has been added to the store.
*/
GROUP_REMOVE_REQUESTED,
+
+
}
/**
diff --git a/core/api/src/main/java/org/onosproject/net/group/GroupProviderService.java b/core/api/src/main/java/org/onosproject/net/group/GroupProviderService.java
index 26c7bb1..076de49 100644
--- a/core/api/src/main/java/org/onosproject/net/group/GroupProviderService.java
+++ b/core/api/src/main/java/org/onosproject/net/group/GroupProviderService.java
@@ -29,9 +29,10 @@
/**
* Notifies core if any failure from data plane during group operations.
*
+ * @param deviceId the device ID
* @param operation offended group operation
*/
- void groupOperationFailed(GroupOperation operation);
+ void groupOperationFailed(DeviceId deviceId, GroupOperation operation);
/**
* Pushes the collection of group detected in the data plane along
diff --git a/core/api/src/main/java/org/onosproject/net/group/GroupStore.java b/core/api/src/main/java/org/onosproject/net/group/GroupStore.java
index 2fc7030..6803b58 100644
--- a/core/api/src/main/java/org/onosproject/net/group/GroupStore.java
+++ b/core/api/src/main/java/org/onosproject/net/group/GroupStore.java
@@ -143,4 +143,12 @@
* @return initial group audit status
*/
boolean deviceInitialAuditStatus(DeviceId deviceId);
+
+ /**
+ * Indicates the group operations failed.
+ *
+ * @param deviceId the device ID
+ * @param operation the group operation failed
+ */
+ void groupOperationFailed(DeviceId deviceId, GroupOperation operation);
}
diff --git a/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java b/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
index f54f85e..4cb21e5 100644
--- a/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
+++ b/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
@@ -263,6 +263,9 @@
case GROUP_ADDED:
case GROUP_UPDATED:
case GROUP_REMOVED:
+ case GROUP_ADD_FAILED:
+ case GROUP_UPDATE_FAILED:
+ case GROUP_REMOVE_FAILED:
eventDispatcher.post(event);
break;
@@ -281,9 +284,9 @@
}
@Override
- public void groupOperationFailed(GroupOperation operation) {
- // TODO Auto-generated method stub
-
+ public void groupOperationFailed(DeviceId deviceId,
+ GroupOperation operation) {
+ store.groupOperationFailed(deviceId, operation);
}
private void groupMissing(Group group) {
diff --git a/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java b/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java
index 2e1bd21..2e18430 100644
--- a/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java
@@ -15,11 +15,6 @@
*/
package org.onosproject.net.group.impl;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertTrue;
-
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -61,6 +56,8 @@
import com.google.common.collect.Iterables;
+import static org.junit.Assert.*;
+
/**
* Test codifying the group service & group provider service contracts.
*/
@@ -317,6 +314,90 @@
internalListener.validateEvent(Arrays.asList(GroupEvent.Type.GROUP_REMOVED));
}
+ /**
+ * Test GroupOperationFailure function in Group Manager.
+ * a)GroupAddFailure
+ * b)GroupUpdateFailure
+ * c)GroupRemoteFailure
+ */
+ @Test
+ public void testGroupOperationFailure() {
+ PortNumber[] ports1 = {PortNumber.portNumber(31),
+ PortNumber.portNumber(32)};
+ PortNumber[] ports2 = {PortNumber.portNumber(41),
+ PortNumber.portNumber(42)};
+ // Test Group creation before AUDIT process
+ TestGroupKey key = new TestGroupKey("group1BeforeAudit");
+ List<GroupBucket> buckets = new ArrayList<GroupBucket>();
+ List<PortNumber> outPorts = new ArrayList<PortNumber>();
+ outPorts.addAll(Arrays.asList(ports1));
+ outPorts.addAll(Arrays.asList(ports2));
+ for (PortNumber portNumber: outPorts) {
+ TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+ tBuilder.setOutput(portNumber)
+ .setEthDst(MacAddress.valueOf("00:00:00:00:00:02"))
+ .setEthSrc(MacAddress.valueOf("00:00:00:00:00:01"))
+ .pushMpls()
+ .setMpls(106);
+ buckets.add(DefaultGroupBucket.createSelectGroupBucket(
+ tBuilder.build()));
+ }
+ GroupBuckets groupBuckets = new GroupBuckets(buckets);
+ GroupDescription newGroupDesc = new DefaultGroupDescription(DID,
+ Group.Type.SELECT,
+ groupBuckets,
+ key,
+ appId);
+ groupService.addGroup(newGroupDesc);
+
+ // Test initial group audit process
+ GroupId gId1 = new DefaultGroupId(1);
+ Group group1 = createSouthboundGroupEntry(gId1,
+ Arrays.asList(ports1),
+ 0);
+ GroupId gId2 = new DefaultGroupId(2);
+ // Non zero reference count will make the group manager to queue
+ // the extraneous groups until reference count is zero.
+ Group group2 = createSouthboundGroupEntry(gId2,
+ Arrays.asList(ports2),
+ 2);
+ List<Group> groupEntries = Arrays.asList(group1, group2);
+ providerService.pushGroupMetrics(DID, groupEntries);
+ Group createdGroup = groupService.getGroup(DID, key);
+
+ // Group Add failure test
+ GroupOperation groupAddOp = GroupOperation.
+ createAddGroupOperation(createdGroup.id(),
+ createdGroup.type(),
+ createdGroup.buckets());
+ providerService.groupOperationFailed(DID, groupAddOp);
+ internalListener.validateEvent(Arrays.asList(GroupEvent.Type.GROUP_ADD_FAILED));
+
+ // Group Mod failure test
+ groupService.addGroup(newGroupDesc);
+ createdGroup = groupService.getGroup(DID, key);
+ assertNotNull(createdGroup);
+
+ GroupOperation groupModOp = GroupOperation.
+ createModifyGroupOperation(createdGroup.id(),
+ createdGroup.type(),
+ createdGroup.buckets());
+ providerService.groupOperationFailed(DID, groupModOp);
+ internalListener.validateEvent(Arrays.asList(GroupEvent.Type.GROUP_UPDATE_FAILED));
+
+ // Group Delete failure test
+ groupService.addGroup(newGroupDesc);
+ createdGroup = groupService.getGroup(DID, key);
+ assertNotNull(createdGroup);
+
+ GroupOperation groupDelOp = GroupOperation.
+ createDeleteGroupOperation(createdGroup.id(),
+ createdGroup.type());
+ providerService.groupOperationFailed(DID, groupDelOp);
+ internalListener.validateEvent(Arrays.asList(GroupEvent.Type.GROUP_REMOVE_FAILED));
+
+ }
+
private Group createSouthboundGroupEntry(GroupId gId,
List<PortNumber> ports,
long referenceCount) {
diff --git a/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleGroupStore.java b/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleGroupStore.java
index 8c7de08..9258dcc 100644
--- a/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleGroupStore.java
+++ b/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleGroupStore.java
@@ -43,6 +43,7 @@
import org.onosproject.net.group.GroupEvent;
import org.onosproject.net.group.GroupEvent.Type;
import org.onosproject.net.group.GroupKey;
+import org.onosproject.net.group.GroupOperation;
import org.onosproject.net.group.GroupStore;
import org.onosproject.net.group.GroupStoreDelegate;
import org.onosproject.net.group.StoredGroupEntry;
@@ -474,6 +475,41 @@
}
@Override
+ public void groupOperationFailed(DeviceId deviceId, GroupOperation operation) {
+
+ StoredGroupEntry existing = (groupEntriesById.get(
+ deviceId) != null) ?
+ groupEntriesById.get(deviceId).get(operation.groupId()) :
+ null;
+
+ if (existing == null) {
+ log.warn("No group entry with ID {} found ", operation.groupId());
+ return;
+ }
+
+ switch (operation.opType()) {
+ case ADD:
+ notifyDelegate(new GroupEvent(Type.GROUP_ADD_FAILED, existing));
+ break;
+ case MODIFY:
+ notifyDelegate(new GroupEvent(Type.GROUP_UPDATE_FAILED, existing));
+ break;
+ case DELETE:
+ notifyDelegate(new GroupEvent(Type.GROUP_REMOVE_FAILED, existing));
+ break;
+ default:
+ log.warn("Unknown group operation type {}", operation.opType());
+ }
+
+ ConcurrentMap<GroupKey, StoredGroupEntry> keyTable =
+ getGroupKeyTable(existing.deviceId());
+ ConcurrentMap<GroupId, StoredGroupEntry> idTable =
+ getGroupIdTable(existing.deviceId());
+ idTable.remove(existing.id());
+ keyTable.remove(existing.appCookie());
+ }
+
+ @Override
public void addOrUpdateExtraneousGroupEntry(Group group) {
ConcurrentMap<GroupId, Group> extraneousIdTable =
getExtraneousGroupIdTable(group.deviceId());
@@ -497,4 +533,6 @@
return FluentIterable.from(
getExtraneousGroupIdTable(deviceId).values());
}
+
+
}
diff --git a/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleGroupStoreTest.java b/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleGroupStoreTest.java
index 277e1ca..33d7118 100644
--- a/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleGroupStoreTest.java
+++ b/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleGroupStoreTest.java
@@ -40,6 +40,7 @@
import org.onosproject.net.group.GroupDescription;
import org.onosproject.net.group.GroupEvent;
import org.onosproject.net.group.GroupKey;
+import org.onosproject.net.group.GroupOperation;
import org.onosproject.net.group.GroupStore.UpdateType;
import org.onosproject.net.group.GroupStoreDelegate;
@@ -129,6 +130,18 @@
createdGroupId = event.subject().id();
assertEquals(Group.GroupState.PENDING_DELETE,
event.subject().state());
+ } else if (expectedEvent == GroupEvent.Type.GROUP_ADD_FAILED) {
+ createdGroupId = event.subject().id();
+ assertEquals(Group.GroupState.PENDING_ADD,
+ event.subject().state());
+ } else if (expectedEvent == GroupEvent.Type.GROUP_UPDATE_FAILED) {
+ createdGroupId = event.subject().id();
+ assertEquals(Group.GroupState.PENDING_UPDATE,
+ event.subject().state());
+ } else if (expectedEvent == GroupEvent.Type.GROUP_REMOVE_FAILED) {
+ createdGroupId = event.subject().id();
+ assertEquals(Group.GroupState.PENDING_DELETE,
+ event.subject().state());
}
}
@@ -310,6 +323,92 @@
simpleGroupStore.unsetDelegate(removeGroupEntryDelegate);
+
+ }
+
+ @Test
+ public void testGroupOperationFailure() {
+
+ simpleGroupStore.deviceInitialAuditCompleted(D1);
+
+ ApplicationId appId =
+ new DefaultApplicationId(2, "org.groupstore.test");
+ TestGroupKey key = new TestGroupKey("group1");
+ PortNumber[] ports = {PortNumber.portNumber(31),
+ PortNumber.portNumber(32)};
+ List<PortNumber> outPorts = new ArrayList<PortNumber>();
+ outPorts.add(ports[0]);
+ outPorts.add(ports[1]);
+
+ List<GroupBucket> buckets = new ArrayList<GroupBucket>();
+ for (PortNumber portNumber: outPorts) {
+ TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+ tBuilder.setOutput(portNumber)
+ .setEthDst(MacAddress.valueOf("00:00:00:00:00:02"))
+ .setEthSrc(MacAddress.valueOf("00:00:00:00:00:01"))
+ .pushMpls()
+ .setMpls(106);
+ buckets.add(DefaultGroupBucket.createSelectGroupBucket(
+ tBuilder.build()));
+ }
+ GroupBuckets groupBuckets = new GroupBuckets(buckets);
+ GroupDescription groupDesc = new DefaultGroupDescription(
+ D1,
+ Group.Type.SELECT,
+ groupBuckets,
+ key,
+ appId);
+ InternalGroupStoreDelegate checkStoreGroupDelegate =
+ new InternalGroupStoreDelegate(key,
+ groupBuckets,
+ GroupEvent.Type.GROUP_ADD_REQUESTED);
+ simpleGroupStore.setDelegate(checkStoreGroupDelegate);
+ // Testing storeGroup operation
+ simpleGroupStore.storeGroupDescription(groupDesc);
+ simpleGroupStore.unsetDelegate(checkStoreGroupDelegate);
+
+ // Testing Group add operation failure
+ Group createdGroup = simpleGroupStore.getGroup(D1, key);
+ checkStoreGroupDelegate.verifyGroupId(createdGroup.id());
+
+ GroupOperation groupAddOp = GroupOperation.
+ createAddGroupOperation(createdGroup.id(),
+ createdGroup.type(),
+ createdGroup.buckets());
+ InternalGroupStoreDelegate checkGroupAddFailureDelegate =
+ new InternalGroupStoreDelegate(key,
+ groupBuckets,
+ GroupEvent.Type.GROUP_ADD_FAILED);
+ simpleGroupStore.setDelegate(checkGroupAddFailureDelegate);
+ simpleGroupStore.groupOperationFailed(D1, groupAddOp);
+
+
+ // Testing Group modify operation failure
+ simpleGroupStore.unsetDelegate(checkGroupAddFailureDelegate);
+ GroupOperation groupModOp = GroupOperation.
+ createModifyGroupOperation(createdGroup.id(),
+ createdGroup.type(),
+ createdGroup.buckets());
+ InternalGroupStoreDelegate checkGroupModFailureDelegate =
+ new InternalGroupStoreDelegate(key,
+ groupBuckets,
+ GroupEvent.Type.GROUP_UPDATE_FAILED);
+ simpleGroupStore.setDelegate(checkGroupModFailureDelegate);
+ simpleGroupStore.groupOperationFailed(D1, groupModOp);
+
+ // Testing Group modify operation failure
+ simpleGroupStore.unsetDelegate(checkGroupModFailureDelegate);
+ GroupOperation groupDelOp = GroupOperation.
+ createDeleteGroupOperation(createdGroup.id(),
+ createdGroup.type());
+ InternalGroupStoreDelegate checkGroupDelFailureDelegate =
+ new InternalGroupStoreDelegate(key,
+ groupBuckets,
+ GroupEvent.Type.GROUP_REMOVE_FAILED);
+ simpleGroupStore.setDelegate(checkGroupDelFailureDelegate);
+ simpleGroupStore.groupOperationFailed(D1, groupDelOp);
+
+
}
}
diff --git a/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java b/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
index 1c9f801..0bd147d 100644
--- a/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
+++ b/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
@@ -282,8 +282,10 @@
} else {
GroupOperation operation =
pendingGroupOperations.get(pendingGroupId);
+ DeviceId deviceId = DeviceId.deviceId(Dpid.uri(dpid));
if (operation != null) {
- providerService.groupOperationFailed(operation);
+ providerService.groupOperationFailed(deviceId,
+ operation);
pendingGroupOperations.remove(pendingGroupId);
pendingXidMaps.remove(pendingGroupId);
log.warn("Received an group mod error {}", msg);
diff --git a/providers/openflow/group/src/test/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProviderTest.java b/providers/openflow/group/src/test/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProviderTest.java
index dd6d9ff..402e0ac 100644
--- a/providers/openflow/group/src/test/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProviderTest.java
+++ b/providers/openflow/group/src/test/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProviderTest.java
@@ -177,7 +177,7 @@
}
@Override
- public void groupOperationFailed(GroupOperation operation) {
+ public void groupOperationFailed(DeviceId deviceId, GroupOperation operation) {
this.failedOperation = operation;
}