ONOS-895: Group manager implementation

Change-Id: Ie183f722fa39012f8de056961715c325e2388e63
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
new file mode 100644
index 0000000..f54f85e
--- /dev/null
+++ b/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
@@ -0,0 +1,366 @@
+/*
+ * 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.group.impl;
+
+import static org.slf4j.LoggerFactory.getLogger;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Set;
+
+import org.apache.felix.scr.annotations.Activate;
+import org.apache.felix.scr.annotations.Component;
+import org.apache.felix.scr.annotations.Deactivate;
+import org.apache.felix.scr.annotations.Reference;
+import org.apache.felix.scr.annotations.ReferenceCardinality;
+import org.apache.felix.scr.annotations.Service;
+import org.onosproject.core.ApplicationId;
+import org.onosproject.event.AbstractListenerRegistry;
+import org.onosproject.event.EventDeliveryService;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.group.Group;
+import org.onosproject.net.group.GroupBuckets;
+import org.onosproject.net.group.GroupDescription;
+import org.onosproject.net.group.GroupEvent;
+import org.onosproject.net.group.GroupKey;
+import org.onosproject.net.group.GroupListener;
+import org.onosproject.net.group.GroupOperation;
+import org.onosproject.net.group.GroupOperations;
+import org.onosproject.net.group.GroupProvider;
+import org.onosproject.net.group.GroupProviderRegistry;
+import org.onosproject.net.group.GroupProviderService;
+import org.onosproject.net.group.GroupService;
+import org.onosproject.net.group.GroupStore;
+import org.onosproject.net.group.GroupStore.UpdateType;
+import org.onosproject.net.group.GroupStoreDelegate;
+import org.onosproject.net.provider.AbstractProviderRegistry;
+import org.onosproject.net.provider.AbstractProviderService;
+import org.slf4j.Logger;
+
+import com.google.common.collect.Sets;
+
+/**
+ * Provides implementation of the group service APIs.
+ */
+@Component(immediate = true)
+@Service
+public class GroupManager
+        extends AbstractProviderRegistry<GroupProvider, GroupProviderService>
+        implements GroupService, GroupProviderRegistry {
+
+    private final Logger log = getLogger(getClass());
+
+    private final AbstractListenerRegistry<GroupEvent, GroupListener>
+                listenerRegistry = new AbstractListenerRegistry<>();
+    private final GroupStoreDelegate delegate = new InternalGroupStoreDelegate();
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected GroupStore store;
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected EventDeliveryService eventDispatcher;
+
+    @Activate
+    public void activate() {
+        store.setDelegate(delegate);
+        eventDispatcher.addSink(GroupEvent.class, listenerRegistry);
+        log.info("Started");
+    }
+
+    @Deactivate
+    public void deactivate() {
+        store.unsetDelegate(delegate);
+        eventDispatcher.removeSink(GroupEvent.class);
+        log.info("Stopped");
+    }
+
+    /**
+     * Create a group in the specified device with the provided parameters.
+     *
+     * @param groupDesc group creation parameters
+     *
+     */
+    @Override
+    public void addGroup(GroupDescription groupDesc) {
+        store.storeGroupDescription(groupDesc);
+    }
+
+    /**
+     * Return a group object associated to an application cookie.
+     *
+     * NOTE1: The presence of group object in the system does not
+     * guarantee that the "group" is actually created in device.
+     * GROUP_ADDED notification would confirm the creation of
+     * this group in data plane.
+     *
+     * @param deviceId device identifier
+     * @param appCookie application cookie to be used for lookup
+     * @return group associated with the application cookie or
+     *               NULL if Group is not found for the provided cookie
+     */
+    @Override
+    public Group getGroup(DeviceId deviceId, GroupKey appCookie) {
+        return store.getGroup(deviceId, appCookie);
+    }
+
+    /**
+     * Append buckets to existing group. The caller can optionally
+     * associate a new cookie during this updation. GROUP_UPDATED or
+     * GROUP_UPDATE_FAILED notifications would be provided along with
+     * cookie depending on the result of the operation on the device.
+     *
+     * @param deviceId device identifier
+     * @param oldCookie cookie to be used to retrieve the existing group
+     * @param buckets immutable list of group bucket to be added
+     * @param newCookie immutable cookie to be used post update operation
+     * @param appId Application Id
+     */
+    @Override
+    public void addBucketsToGroup(DeviceId deviceId,
+                           GroupKey oldCookie,
+                           GroupBuckets buckets,
+                           GroupKey newCookie,
+                           ApplicationId appId) {
+        store.updateGroupDescription(deviceId,
+                                     oldCookie,
+                                     UpdateType.ADD,
+                                     buckets,
+                                     newCookie);
+    }
+
+    /**
+     * Remove buckets from existing group. The caller can optionally
+     * associate a new cookie during this updation. GROUP_UPDATED or
+     * GROUP_UPDATE_FAILED notifications would be provided along with
+     * cookie depending on the result of the operation on the device.
+     *
+     * @param deviceId device identifier
+     * @param oldCookie cookie to be used to retrieve the existing group
+     * @param buckets immutable list of group bucket to be removed
+     * @param newCookie immutable cookie to be used post update operation
+     * @param appId Application Id
+     */
+    @Override
+    public void removeBucketsFromGroup(DeviceId deviceId,
+                                GroupKey oldCookie,
+                                GroupBuckets buckets,
+                                GroupKey newCookie,
+                                ApplicationId appId) {
+        store.updateGroupDescription(deviceId,
+                                     oldCookie,
+                                     UpdateType.REMOVE,
+                                     buckets,
+                                     newCookie);
+    }
+
+    /**
+     * Delete a group associated to an application cookie.
+     * GROUP_DELETED or GROUP_DELETE_FAILED notifications would be
+     * provided along with cookie depending on the result of the
+     * operation on the device.
+     *
+     * @param deviceId device identifier
+     * @param appCookie application cookie to be used for lookup
+     * @param appId Application Id
+     */
+    @Override
+    public void removeGroup(DeviceId deviceId,
+                            GroupKey appCookie,
+                            ApplicationId appId) {
+        store.deleteGroupDescription(deviceId, appCookie);
+    }
+
+    /**
+     * Retrieve all groups created by an application in the specified device
+     * as seen by current controller instance.
+     *
+     * @param deviceId device identifier
+     * @param appId application id
+     * @return collection of immutable group objects created by the application
+     */
+    @Override
+    public Iterable<Group> getGroups(DeviceId deviceId,
+                                     ApplicationId appId) {
+        return store.getGroups(deviceId);
+    }
+
+    /**
+     * Adds the specified group listener.
+     *
+     * @param listener group listener
+     */
+    @Override
+    public void addListener(GroupListener listener) {
+        listenerRegistry.addListener(listener);
+    }
+
+    /**
+     * Removes the specified group listener.
+     *
+     * @param listener group listener
+     */
+    @Override
+    public void removeListener(GroupListener listener) {
+        listenerRegistry.removeListener(listener);
+    }
+
+    @Override
+    protected GroupProviderService createProviderService(GroupProvider provider) {
+        return new InternalGroupProviderService(provider);
+    }
+
+    private class InternalGroupStoreDelegate implements GroupStoreDelegate {
+        @Override
+        public void notify(GroupEvent event) {
+            final Group group = event.subject();
+            GroupProvider groupProvider =
+                    getProvider(group.deviceId());
+            GroupOperations groupOps = null;
+            switch (event.type()) {
+            case GROUP_ADD_REQUESTED:
+                GroupOperation groupAddOp = GroupOperation.
+                        createAddGroupOperation(group.id(),
+                                                group.type(),
+                                                group.buckets());
+                groupOps = new GroupOperations(
+                                          Arrays.asList(groupAddOp));
+                groupProvider.performGroupOperation(group.deviceId(), groupOps);
+                break;
+
+            case GROUP_UPDATE_REQUESTED:
+                GroupOperation groupModifyOp = GroupOperation.
+                        createModifyGroupOperation(group.id(),
+                                                group.type(),
+                                                group.buckets());
+                groupOps = new GroupOperations(
+                                   Arrays.asList(groupModifyOp));
+                groupProvider.performGroupOperation(group.deviceId(), groupOps);
+                break;
+
+            case GROUP_REMOVE_REQUESTED:
+                GroupOperation groupDeleteOp = GroupOperation.
+                        createDeleteGroupOperation(group.id(),
+                                                group.type());
+                groupOps = new GroupOperations(
+                                   Arrays.asList(groupDeleteOp));
+                groupProvider.performGroupOperation(group.deviceId(), groupOps);
+                break;
+
+            case GROUP_ADDED:
+            case GROUP_UPDATED:
+            case GROUP_REMOVED:
+                eventDispatcher.post(event);
+                break;
+
+            default:
+                break;
+            }
+        }
+    }
+
+    private class InternalGroupProviderService
+            extends AbstractProviderService<GroupProvider>
+            implements GroupProviderService {
+
+        protected InternalGroupProviderService(GroupProvider provider) {
+            super(provider);
+        }
+
+        @Override
+        public void groupOperationFailed(GroupOperation operation) {
+            // TODO Auto-generated method stub
+
+        }
+
+        private void groupMissing(Group group) {
+            checkValidity();
+            GroupProvider gp = getProvider(group.deviceId());
+            switch (group.state()) {
+                case PENDING_DELETE:
+                    store.removeGroupEntry(group);
+                    break;
+                case ADDED:
+                case PENDING_ADD:
+                    GroupOperation groupAddOp = GroupOperation.
+                                    createAddGroupOperation(group.id(),
+                                                            group.type(),
+                                                            group.buckets());
+                    GroupOperations groupOps = new GroupOperations(
+                                              Arrays.asList(groupAddOp));
+                    gp.performGroupOperation(group.deviceId(), groupOps);
+                    break;
+                default:
+                    log.debug("Group {} has not been installed.", group);
+                    break;
+            }
+        }
+
+
+        private void extraneousGroup(Group group) {
+            log.debug("Group {} is on switch but not in store.", group);
+            checkValidity();
+            store.addOrUpdateExtraneousGroupEntry(group);
+        }
+
+        private void groupAdded(Group group) {
+            checkValidity();
+
+            log.trace("Group {}", group);
+            store.addOrUpdateGroupEntry(group);
+        }
+
+        @Override
+        public void pushGroupMetrics(DeviceId deviceId,
+                                     Collection<Group> groupEntries) {
+            boolean deviceInitialAuditStatus =
+                    store.deviceInitialAuditStatus(deviceId);
+            Set<Group> southboundGroupEntries =
+                    Sets.newHashSet(groupEntries);
+            Set<Group> storedGroupEntries =
+                    Sets.newHashSet(store.getGroups(deviceId));
+            Set<Group> extraneousStoredEntries =
+                    Sets.newHashSet(store.getExtraneousGroups(deviceId));
+
+            for (Iterator<Group> it = southboundGroupEntries.iterator(); it.hasNext();) {
+                Group group = it.next();
+                if (storedGroupEntries.remove(group)) {
+                    // we both have the group, let's update some info then.
+                    groupAdded(group);
+                    it.remove();
+                }
+            }
+            for (Group group : southboundGroupEntries) {
+                // there are groups in the switch that aren't in the store
+                extraneousStoredEntries.remove(group);
+                extraneousGroup(group);
+            }
+            for (Group group : storedGroupEntries) {
+                // there are groups in the store that aren't in the switch
+                groupMissing(group);
+            }
+            for (Group group : extraneousStoredEntries) {
+                // there are groups in the extraneous store that
+                // aren't in the switch
+                store.removeExtraneousGroupEntry(group);
+            }
+
+            if (!deviceInitialAuditStatus) {
+                store.deviceInitialAuditCompleted(deviceId);
+            }
+        }
+    }
+}
diff --git a/core/net/src/main/java/org/onosproject/net/group/impl/package-info.java b/core/net/src/main/java/org/onosproject/net/group/impl/package-info.java
new file mode 100644
index 0000000..641ab44
--- /dev/null
+++ b/core/net/src/main/java/org/onosproject/net/group/impl/package-info.java
@@ -0,0 +1,20 @@
+/*
+ * 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.
+ */
+
+/**
+ * Core subsystem for group state.
+ */
+package org.onosproject.net.group.impl;
\ No newline at end of file
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
new file mode 100644
index 0000000..2e1bd21
--- /dev/null
+++ b/core/net/src/test/java/org/onosproject/net/group/impl/GroupManagerTest.java
@@ -0,0 +1,400 @@
+/*
+ * 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.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;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.packet.MacAddress;
+import org.onosproject.core.ApplicationId;
+import org.onosproject.core.DefaultApplicationId;
+import org.onosproject.core.DefaultGroupId;
+import org.onosproject.core.GroupId;
+import org.onosproject.event.impl.TestEventDispatcher;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.PortNumber;
+import org.onosproject.net.flow.DefaultTrafficTreatment;
+import org.onosproject.net.flow.TrafficTreatment;
+import org.onosproject.net.group.DefaultGroup;
+import org.onosproject.net.group.DefaultGroupBucket;
+import org.onosproject.net.group.DefaultGroupDescription;
+import org.onosproject.net.group.Group;
+import org.onosproject.net.group.GroupBucket;
+import org.onosproject.net.group.GroupBuckets;
+import org.onosproject.net.group.GroupDescription;
+import org.onosproject.net.group.GroupEvent;
+import org.onosproject.net.group.GroupKey;
+import org.onosproject.net.group.GroupListener;
+import org.onosproject.net.group.GroupOperation;
+import org.onosproject.net.group.GroupOperations;
+import org.onosproject.net.group.GroupProvider;
+import org.onosproject.net.group.GroupProviderRegistry;
+import org.onosproject.net.group.GroupProviderService;
+import org.onosproject.net.group.GroupService;
+import org.onosproject.net.group.StoredGroupEntry;
+import org.onosproject.net.provider.AbstractProvider;
+import org.onosproject.net.provider.ProviderId;
+import org.onosproject.store.trivial.impl.SimpleGroupStore;
+
+import com.google.common.collect.Iterables;
+
+/**
+ * Test codifying the group service & group provider service contracts.
+ */
+public class GroupManagerTest {
+
+    private static final ProviderId PID = new ProviderId("of", "groupfoo");
+    private static final DeviceId DID = DeviceId.deviceId("of:001");
+
+    private GroupManager mgr;
+    private GroupService groupService;
+    private GroupProviderRegistry providerRegistry;
+    private TestGroupListener internalListener = new TestGroupListener();
+    private GroupListener listener = internalListener;
+    private TestGroupProvider internalProvider;
+    private GroupProvider provider;
+    private GroupProviderService providerService;
+    private ApplicationId appId;
+
+    @Before
+    public void setUp() {
+        mgr = new GroupManager();
+        groupService = mgr;
+        mgr.store = new SimpleGroupStore();
+        mgr.eventDispatcher = new TestEventDispatcher();
+        providerRegistry = mgr;
+
+        mgr.activate();
+        mgr.addListener(listener);
+
+        internalProvider = new TestGroupProvider(PID);
+        provider = internalProvider;
+        providerService = providerRegistry.register(provider);
+        appId = new DefaultApplicationId(2, "org.groupmanager.test");
+        assertTrue("provider should be registered",
+                   providerRegistry.getProviders().contains(provider.id()));
+    }
+
+    @After
+    public void tearDown() {
+        providerRegistry.unregister(provider);
+        assertFalse("provider should not be registered",
+                    providerRegistry.getProviders().contains(provider.id()));
+        mgr.removeListener(listener);
+        mgr.deactivate();
+        mgr.eventDispatcher = null;
+    }
+
+    private class TestGroupKey implements GroupKey {
+        private String groupId;
+
+        public TestGroupKey(String id) {
+            this.groupId = id;
+        }
+
+        public String id() {
+            return this.groupId;
+        }
+
+        @Override
+        public int hashCode() {
+            return groupId.hashCode();
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (obj instanceof TestGroupKey) {
+                return this.groupId.equals(((TestGroupKey) obj).id());
+            }
+            return false;
+        }
+    }
+
+    /**
+     * Tests group service north bound and south bound interfaces.
+     * The following operations are tested:
+     * a)Tests group creation before the device group AUDIT completes
+     * b)Tests initial device group AUDIT process
+     * c)Tests deletion process of any extraneous groups
+     * d)Tests execution of any pending group creation requests
+     * after the device group AUDIT completes
+     * e)Tests re-apply process of any missing groups
+     * f)Tests event notifications after receiving confirmation for
+     * any operations from data plane
+     * g)Tests group bucket modifications (additions and deletions)
+     * h)Tests group deletion
+     */
+    @Test
+    public void testGroupService() {
+        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);
+        internalProvider.validate(DID, null);
+        assertEquals(null, groupService.getGroup(DID, key));
+        assertEquals(0, Iterables.size(groupService.getGroups(DID, appId)));
+
+        // 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);
+        // First group metrics would trigger the device audit completion
+        // post which all pending group requests are also executed.
+        Group createdGroup = groupService.getGroup(DID, key);
+        int createdGroupId = createdGroup.id().id();
+        assertNotEquals(gId1.id(), createdGroupId);
+        assertNotEquals(gId2.id(), createdGroupId);
+        List<GroupOperation> expectedGroupOps = Arrays.asList(
+                            GroupOperation.createDeleteGroupOperation(gId1,
+                                                          Group.Type.SELECT),
+                            GroupOperation.createAddGroupOperation(
+                                           createdGroup.id(),
+                                           Group.Type.SELECT,
+                                           groupBuckets));
+        internalProvider.validate(DID, expectedGroupOps);
+
+        group1 = createSouthboundGroupEntry(gId1,
+                                            Arrays.asList(ports1),
+                                            0);
+        group2 = createSouthboundGroupEntry(gId2,
+                                            Arrays.asList(ports2),
+                                            0);
+        groupEntries = Arrays.asList(group1, group2);
+        providerService.pushGroupMetrics(DID, groupEntries);
+        expectedGroupOps = Arrays.asList(
+                GroupOperation.createDeleteGroupOperation(gId1,
+                                                          Group.Type.SELECT),
+                GroupOperation.createDeleteGroupOperation(gId2,
+                                                          Group.Type.SELECT),
+                GroupOperation.createAddGroupOperation(createdGroup.id(),
+                                                       Group.Type.SELECT,
+                                                       groupBuckets));
+        internalProvider.validate(DID, expectedGroupOps);
+
+        createdGroup = new DefaultGroup(createdGroup.id(),
+                                        DID,
+                                        Group.Type.SELECT,
+                                        groupBuckets);
+        groupEntries = Arrays.asList(createdGroup);
+        providerService.pushGroupMetrics(DID, groupEntries);
+        internalListener.validateEvent(Arrays.asList(GroupEvent.Type.GROUP_ADDED));
+
+        // Test group add bucket operations
+        TestGroupKey addKey = new TestGroupKey("group1AddBuckets");
+        PortNumber[] addPorts = {PortNumber.portNumber(51),
+                                 PortNumber.portNumber(52)};
+        outPorts.clear();
+        outPorts.addAll(Arrays.asList(addPorts));
+        List<GroupBucket> addBuckets = 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);
+            addBuckets.add(DefaultGroupBucket.createSelectGroupBucket(
+                                                        tBuilder.build()));
+            buckets.add(DefaultGroupBucket.createSelectGroupBucket(
+                                                        tBuilder.build()));
+        }
+        GroupBuckets groupAddBuckets = new GroupBuckets(addBuckets);
+        groupService.addBucketsToGroup(DID,
+                                       key,
+                                       groupAddBuckets,
+                                       addKey,
+                                       appId);
+        GroupBuckets updatedBuckets = new GroupBuckets(buckets);
+        expectedGroupOps = Arrays.asList(
+               GroupOperation.createModifyGroupOperation(createdGroup.id(),
+                                                         Group.Type.SELECT,
+                                                         updatedBuckets));
+        internalProvider.validate(DID, expectedGroupOps);
+        Group existingGroup = groupService.getGroup(DID, addKey);
+        groupEntries = Arrays.asList(existingGroup);
+        providerService.pushGroupMetrics(DID, groupEntries);
+        internalListener.validateEvent(Arrays.asList(GroupEvent.Type.GROUP_UPDATED));
+
+        // Test group remove bucket operations
+        TestGroupKey removeKey = new TestGroupKey("group1RemoveBuckets");
+        PortNumber[] removePorts = {PortNumber.portNumber(31),
+                                 PortNumber.portNumber(32)};
+        outPorts.clear();
+        outPorts.addAll(Arrays.asList(removePorts));
+        List<GroupBucket> removeBuckets = 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);
+            removeBuckets.add(DefaultGroupBucket.createSelectGroupBucket(
+                                                        tBuilder.build()));
+            buckets.remove(DefaultGroupBucket.createSelectGroupBucket(
+                                                        tBuilder.build()));
+        }
+        GroupBuckets groupRemoveBuckets = new GroupBuckets(removeBuckets);
+        groupService.removeBucketsFromGroup(DID,
+                                            addKey,
+                                            groupRemoveBuckets,
+                                            removeKey,
+                                            appId);
+        updatedBuckets = new GroupBuckets(buckets);
+        expectedGroupOps = Arrays.asList(
+               GroupOperation.createModifyGroupOperation(createdGroup.id(),
+                                                         Group.Type.SELECT,
+                                                         updatedBuckets));
+        internalProvider.validate(DID, expectedGroupOps);
+        existingGroup = groupService.getGroup(DID, removeKey);
+        groupEntries = Arrays.asList(existingGroup);
+        providerService.pushGroupMetrics(DID, groupEntries);
+        internalListener.validateEvent(Arrays.asList(GroupEvent.Type.GROUP_UPDATED));
+
+        // Test group remove operations
+        groupService.removeGroup(DID, removeKey, appId);
+        expectedGroupOps = Arrays.asList(
+             GroupOperation.createDeleteGroupOperation(createdGroup.id(),
+                                                       Group.Type.SELECT));
+        internalProvider.validate(DID, expectedGroupOps);
+        groupEntries = Collections.emptyList();
+        providerService.pushGroupMetrics(DID, groupEntries);
+        internalListener.validateEvent(Arrays.asList(GroupEvent.Type.GROUP_REMOVED));
+    }
+
+    private Group createSouthboundGroupEntry(GroupId gId,
+                                             List<PortNumber> ports,
+                                             long referenceCount) {
+        List<PortNumber> outPorts = new ArrayList<PortNumber>();
+        outPorts.addAll(ports);
+
+        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);
+        StoredGroupEntry group = new DefaultGroup(
+                            gId, DID, Group.Type.SELECT, groupBuckets);
+        group.setReferenceCount(referenceCount);
+        return group;
+    }
+
+    private static class TestGroupListener implements GroupListener {
+        final List<GroupEvent> events = new ArrayList<>();
+
+        @Override
+        public void event(GroupEvent event) {
+            events.add(event);
+        }
+
+        public void validateEvent(List<GroupEvent.Type> expectedEvents) {
+            int i = 0;
+            System.err.println("events :" + events);
+            for (GroupEvent e : events) {
+                assertEquals("unexpected event", expectedEvents.get(i), e.type());
+                i++;
+            }
+            assertEquals("mispredicted number of events",
+                         expectedEvents.size(), events.size());
+            events.clear();
+        }
+    }
+
+    private class TestGroupProvider
+                extends AbstractProvider implements GroupProvider {
+        DeviceId lastDeviceId;
+        List<GroupOperation> groupOperations = new ArrayList<GroupOperation>();
+
+        protected TestGroupProvider(ProviderId id) {
+            super(id);
+        }
+
+        @Override
+        public void performGroupOperation(DeviceId deviceId,
+                                          GroupOperations groupOps) {
+            lastDeviceId = deviceId;
+            groupOperations.addAll(groupOps.operations());
+        }
+
+        public void validate(DeviceId expectedDeviceId,
+                             List<GroupOperation> expectedGroupOps) {
+            if (expectedGroupOps == null) {
+                assertTrue("events generated", groupOperations.isEmpty());
+                return;
+            }
+
+            assertEquals(lastDeviceId, expectedDeviceId);
+            assertTrue((this.groupOperations.containsAll(expectedGroupOps) &&
+                    expectedGroupOps.containsAll(groupOperations)));
+
+            groupOperations.clear();
+            lastDeviceId = null;
+        }
+
+    }
+
+}
+
+