Fixed to not to create gateway group if it is already exists

Also changed some names shorter.

Change-Id: Iaa8aa5ac378fc168e79c9e238090ca817af42261

@@ -331,7 +331,7 @@ public class OpenstackIcmpHandler {
     private Map<DeviceId, PortNumber> getExternalInfo() {
         Map<DeviceId, PortNumber> externalInfoMap = Maps.newHashMap();
         gatewayService.getGatewayDeviceIds().forEach(deviceId ->
-                externalInfoMap.putIfAbsent(deviceId, gatewayService.getGatewayExternalPort(deviceId)));
+                externalInfoMap.putIfAbsent(deviceId, gatewayService.getUplinkPort(deviceId)));
         return externalInfoMap;
     }
 }

@@ -147,11 +147,11 @@ public class OpenstackPnatHandler implements Runnable {
 
         ScalableGatewayService gatewayService = getService(ScalableGatewayService.class);
         GatewayNode gatewayNode = gatewayService.getGatewayNode(deviceId);
-        if (gatewayNode.getGatewayExternalInterfaceName() == null) {
+        if (gatewayNode.getUplinkIntf() == null) {
             log.error(EXTERNAL_PORT_NULL, deviceId.toString());
             return;
         }
-        treatment.setOutput(gatewayService.getGatewayExternalPort(deviceId));
+        treatment.setOutput(gatewayService.getUplinkPort(deviceId));
 
         ethernet.resetChecksum();
 

@@ -468,7 +468,7 @@ public class OpenstackRoutingManager implements OpenstackRoutingService {
                         DeviceId deviceId = pkt.receivedFrom().deviceId();
                         Port port = null;
                         port = deviceService.getPort(deviceId,
-                                gatewayService.getGatewayExternalPort(deviceId));
+                                gatewayService.getUplinkPort(deviceId));
                         if (port != null) {
                             OpenstackPort openstackPort = getOpenstackPort(ethernet.getSourceMAC(),
                                     Ip4Address.valueOf(iPacket.getSourceAddress()));

@@ -188,7 +188,7 @@ public class OpenstackRoutingRulePopulator {
 
         tBuilder.setIpSrc(externalIp);
         gatewayService.getGatewayNodes().forEach(node -> {
-            tBuilder.setOutput(gatewayService.getGatewayExternalPort(node.getGatewayDeviceId()));
+            tBuilder.setOutput(gatewayService.getUplinkPort(node.getGatewayDeviceId()));
             ForwardingObjective fo = DefaultForwardingObjective.builder()
                     .withSelector(sBuilder.build())
                     .withTreatment(tBuilder.build())
@@ -380,7 +380,7 @@ public class OpenstackRoutingRulePopulator {
         StreamSupport.stream(deviceService.getDevices().spliterator(), false)
                 .filter(d -> isTypeOf(d.id(), OpenstackNodeService.NodeType.COMPUTE))
                 .forEach(d -> populateRuleToGatewayBySgw(d.id(),
-                        gatewayService.getGroupIdForGatewayLoadBalance(d.id()), vni));
+                        gatewayService.getGatewayGroupId(d.id()), vni));
     }
 
     private void populateRuleToGatewayBySgw(DeviceId deviceId, GroupId groupId, long vni) {
@@ -548,7 +548,7 @@ public class OpenstackRoutingRulePopulator {
     }
 
     private PortNumber getExternalPortNum(DeviceId deviceId) {
-        return checkNotNull(gatewayService.getGatewayExternalPort(deviceId), PORTNOTNULL);
+        return checkNotNull(gatewayService.getUplinkPort(deviceId), PORTNOTNULL);
     }
 
     /**

@@ -28,13 +28,12 @@ import static com.google.common.base.Preconditions.checkNotNull;
  */
 public final class GatewayNode {
     private final DeviceId gatewayDeviceId;
-    private final String gatewayExternalInterfaceName;
+    private final String uplinkIntf;
     private final Ip4Address dataIpAddress;
 
-    private GatewayNode(DeviceId gatewayDeviceId, String gatewayExternalInterfaceName,
-                        Ip4Address dataIpAddress) {
+    private GatewayNode(DeviceId gatewayDeviceId, String uplinkIntf, Ip4Address dataIpAddress) {
         this.gatewayDeviceId = gatewayDeviceId;
-        this.gatewayExternalInterfaceName = gatewayExternalInterfaceName;
+        this.uplinkIntf = uplinkIntf;
         this.dataIpAddress = dataIpAddress;
     }
 
@@ -52,8 +51,8 @@ public final class GatewayNode {
      *
      * @return The gateway`s interface name
      */
-    public String getGatewayExternalInterfaceName() {
-        return gatewayExternalInterfaceName;
+    public String getUplinkIntf() {
+        return uplinkIntf;
     }
 
     /**
@@ -74,8 +73,7 @@ public final class GatewayNode {
         if (obj instanceof GatewayNode) {
             GatewayNode that = (GatewayNode) obj;
             if (Objects.equals(gatewayDeviceId, that.gatewayDeviceId) &&
-                    Objects.equals(gatewayExternalInterfaceName,
-                                   that.gatewayExternalInterfaceName) &&
+                    Objects.equals(uplinkIntf, that.uplinkIntf) &&
                     Objects.equals(dataIpAddress, that.dataIpAddress)) {
                 return true;
             }
@@ -85,17 +83,15 @@ public final class GatewayNode {
 
     @Override
     public int hashCode() {
-        return Objects.hash(gatewayDeviceId,
-                            gatewayExternalInterfaceName,
-                            dataIpAddress);
+        return Objects.hash(gatewayDeviceId, uplinkIntf, dataIpAddress);
     }
 
     @Override
     public String toString() {
         return MoreObjects.toStringHelper(getClass())
-                .add("deviceId", gatewayDeviceId)
-                .add("externalPort", gatewayExternalInterfaceName)
-                .add("dataIp", dataIpAddress)
+                .add("gatewayDeviceId", gatewayDeviceId)
+                .add("uplinkInterface", uplinkIntf)
+                .add("dataIpAddress", dataIpAddress)
                 .toString();
     }
 
@@ -114,7 +110,7 @@ public final class GatewayNode {
     public static final class Builder {
 
         private DeviceId gatewayDeviceId;
-        private String gatewayExternalInterfaceName;
+        private String uplinkIntf;
         private Ip4Address dataIpAddress;
 
         /**
@@ -129,13 +125,13 @@ public final class GatewayNode {
         }
 
         /**
-         * Sets the gateway`s interface name.
+         * Sets the gateway`s uplink interface name.
          *
          * @param name The gateway`s interface name
          * @return Builder object
          */
-        public Builder gatewayExternalInterfaceName(String name) {
-            this.gatewayExternalInterfaceName = name;
+        public Builder uplinkIntf(String name) {
+            this.uplinkIntf = name;
             return this;
         }
 
@@ -156,8 +152,8 @@ public final class GatewayNode {
          * @return GatewayNode object
          */
         public GatewayNode build() {
-            return new GatewayNode(checkNotNull(gatewayDeviceId), checkNotNull(gatewayExternalInterfaceName),
-                    checkNotNull(dataIpAddress));
+            return new GatewayNode(checkNotNull(gatewayDeviceId), checkNotNull(uplinkIntf),
+                                   checkNotNull(dataIpAddress));
         }
     }
 }

@@ -41,7 +41,7 @@ public class GatewayNodeConfig extends Config<ApplicationId> {
     public static final String NODES = "nodes";
     public static final String BRIDGE_ID = "bridgeId";
     public static final String DATAPLANE_IP = "dataPlaneIp";
-    public static final String EXTERNAL_INTERFACE_NAME = "gatewayExternalInterfaceName";
+    public static final String UPLINK_INTERFACE_NAME = "uplinkInterface";
 
     /**
      * Returns the set of nodes read from network config.
@@ -61,7 +61,7 @@ public class GatewayNodeConfig extends Config<ApplicationId> {
             try {
                 nodes.add(new GatewayNode.Builder()
                         .gatewayDeviceId(DeviceId.deviceId(jsonNode.path(BRIDGE_ID).asText()))
-                        .gatewayExternalInterfaceName(jsonNode.path(EXTERNAL_INTERFACE_NAME).asText())
+                        .uplinkIntf(jsonNode.path(UPLINK_INTERFACE_NAME).asText())
                         .dataIpAddress(Ip4Address.valueOf(jsonNode.path(DATAPLANE_IP).asText())).build());
             } catch (IllegalArgumentException | NullPointerException e) {
                 log.error("Failed to read {}", e.toString());
@@ -86,7 +86,7 @@ public class GatewayNodeConfig extends Config<ApplicationId> {
         ObjectNode objectNode = (ObjectNode) jsonNode;
         return isString(objectNode, BRIDGE_ID, MANDATORY)
                 && isIpAddress(objectNode, DATAPLANE_IP, MANDATORY)
-                && isString(objectNode, EXTERNAL_INTERFACE_NAME, MANDATORY);
+                && isString(objectNode, UPLINK_INTERFACE_NAME, MANDATORY);
     }
 
 }

@@ -35,20 +35,21 @@ public interface ScalableGatewayService {
     GatewayNode getGatewayNode(DeviceId deviceId);
 
     /**
-     * Returns the gateway`s port number with the given device identifier.
+     * Returns the uplink port number of the gateway with the supplied device ID.
      *
-     * @param deviceId The gateway node deviceId
-     * @return The external interface port number
+     * @param deviceId the gateway node device id
+     * @return the external interface port number
      */
-    PortNumber getGatewayExternalPort(DeviceId deviceId);
+    PortNumber getUplinkPort(DeviceId deviceId);
 
     /**
      * Returns group id for gateway load balance.
+     * If the group does not exist in the supplied source device, creates one.
      *
      * @param srcDeviceId source device id
      * @return The group id
      */
-    GroupId getGroupIdForGatewayLoadBalance(DeviceId srcDeviceId);
+    GroupId getGatewayGroupId(DeviceId srcDeviceId);
 
     /**
      * Returns the list of gateway node information with the given device identifier.

@@ -56,7 +56,7 @@ public class ScalableGatewayAddCommand extends AbstractShellCommand {
         GatewayNode gatewayNode = GatewayNode.builder()
                 .gatewayDeviceId(DeviceId.deviceId(deviceId))
                 .dataIpAddress(Ip4Address.valueOf(ipAddress))
-                .gatewayExternalInterfaceName(interfaceName)
+                .uplinkIntf(interfaceName)
                 .build();
         if (service.addGatewayNode(gatewayNode)) {
             print(SUCCESS);

@@ -35,6 +35,6 @@ public class ScalableGatewayListCommand extends AbstractShellCommand {
         service.getGatewayNodes().forEach(node -> print(FORMAT,
                 node.getGatewayDeviceId().toString(),
                 node.getDataIpAddress().toString(),
-                node.getGatewayExternalInterfaceName().toString()));
+                node.getUplinkIntf().toString()));
     }
 }

@@ -16,6 +16,7 @@
 
 package org.onosproject.scalablegateway.impl;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
@@ -25,6 +26,7 @@ import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
 import org.onlab.util.KryoNamespace;
+import org.onlab.util.Tools;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
 
@@ -43,13 +45,14 @@ import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceService;
 import org.onosproject.net.driver.DriverService;
 import org.onosproject.net.group.Group;
-import org.onosproject.net.group.GroupDescription;
+import org.onosproject.net.group.GroupKey;
 import org.onosproject.net.group.GroupService;
 import org.onosproject.scalablegateway.api.GatewayNode;
 import org.onosproject.scalablegateway.api.GatewayNodeConfig;
 import org.onosproject.scalablegateway.api.ScalableGatewayService;
 
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 
 import org.onosproject.store.serializers.KryoNamespaces;
@@ -60,7 +63,7 @@ import org.onosproject.store.service.Versioned;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import static org.onosproject.net.AnnotationKeys.PORT_NAME;
 
 /**
  * Manages gateway node for gateway scalability.
@@ -74,11 +77,6 @@ public class ScalableGatewayManager implements ScalableGatewayService {
     private ApplicationId appId;
     private static final String APP_ID = "org.onosproject.scalablegateway";
     private static final String APP_NAME = "scalablegateway";
-    private static final String GATEWAYNODE_CAN_NOT_BE_NULL = "The gateway node can not be null";
-    private static final String PORT_CAN_NOT_BE_NULL = "The port can not be null";
-    private static final String FAIL_ADD_GATEWAY = "Adding process is failed as existing deivce id";
-    private static final String FAIL_REMOVE_GATEWAY = "Removing process is failed as unknown deivce id";
-    private static final String PORT_NAME = "portName";
     private static final String GATEWAYNODE_MAP_NAME = "gatewaynode-map";
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
@@ -106,7 +104,7 @@ public class ScalableGatewayManager implements ScalableGatewayService {
     private SelectGroupHandler selectGroupHandler;
 
     private final NetworkConfigListener configListener = new InternalConfigListener();
-    private InternalDeviceListener internalDeviceListener = new InternalDeviceListener();
+    private final InternalDeviceListener internalDeviceListener = new InternalDeviceListener();
 
     private final ConfigFactory configFactory =
             new ConfigFactory(SubjectFactories.APP_SUBJECT_FACTORY, GatewayNodeConfig.class, APP_NAME) {
@@ -115,10 +113,10 @@ public class ScalableGatewayManager implements ScalableGatewayService {
                     return new GatewayNodeConfig();
                 }
             };
-    private ConsistentMap<DeviceId, GatewayNode> gatewayNodeMap; // Map<GatewayNode`s Id, GatewayNode object>
+
+    private ConsistentMap<DeviceId, GatewayNode> gatewayNodeMap; // Map<GatewayNode Id, GatewayNode object>
     private static final KryoNamespace.Builder GATEWAYNODE_SERIALIZER = KryoNamespace.newBuilder()
             .register(KryoNamespaces.API)
-            .register(DeviceId.class)
             .register(GatewayNode.class);
 
     @Activate
@@ -141,8 +139,6 @@ public class ScalableGatewayManager implements ScalableGatewayService {
 
     @Deactivate
     protected void deactivate() {
-        gatewayNodeMap.clear();
-
         deviceService.removeListener(internalDeviceListener);
         configService.removeListener(configListener);
 
@@ -151,32 +147,44 @@ public class ScalableGatewayManager implements ScalableGatewayService {
 
     @Override
     public GatewayNode getGatewayNode(DeviceId deviceId) {
-        return checkNotNull(gatewayNodeMap.get(deviceId).value(), GATEWAYNODE_CAN_NOT_BE_NULL);
+        GatewayNode gatewayNode = gatewayNodeMap.get(deviceId).value();
+        if (gatewayNode == null) {
+            log.warn("Gateway with device ID {} does not exist");
+            return null;
+        }
+        return gatewayNode;
     }
 
     @Override
-    public PortNumber getGatewayExternalPort(DeviceId deviceId) {
-        GatewayNode gatewayNode = checkNotNull(gatewayNodeMap.get(deviceId).value(), GATEWAYNODE_CAN_NOT_BE_NULL);
-        String externalInterfaceName = gatewayNode.getGatewayExternalInterfaceName();
-        Optional<Port> port = deviceService.getPorts(deviceId)
-                .stream()
-                .filter(p -> p.annotations().value(PORT_NAME).equals(externalInterfaceName))
-                .findFirst();
+    public PortNumber getUplinkPort(DeviceId deviceId) {
+        GatewayNode gatewayNode = gatewayNodeMap.get(deviceId).value();
+        if (gatewayNode == null) {
+            log.warn("Gateway with device ID {} does not exist");
+            return null;
+        }
 
+        Optional<Port> port = deviceService.getPorts(deviceId).stream()
+                .filter(p -> Objects.equals(
+                        p.annotations().value(PORT_NAME),
+                        gatewayNode.getUplinkIntf()))
+                .findFirst();
         if (!port.isPresent()) {
-            log.error("Cannot find port {} in gateway device {}", externalInterfaceName, deviceId);
+            log.warn("Cannot find uplink interface from gateway {}", deviceId);
             return null;
         }
-
         return port.get().number();
     }
 
     @Override
-    public GroupId getGroupIdForGatewayLoadBalance(DeviceId srcDeviceId) {
-        GroupDescription description = selectGroupHandler.createSelectGroupInVxlan(srcDeviceId, getGatewayNodes());
-        groupService.addGroup(description);
-        Group group = groupService.getGroup(description.deviceId(), description.appCookie());
-        return group != null ? group.id() : null;
+    public synchronized GroupId getGatewayGroupId(DeviceId srcDeviceId) {
+        GroupKey groupKey = selectGroupHandler.getGroupKey(srcDeviceId);
+        Group group = groupService.getGroup(srcDeviceId, groupKey);
+        if (group == null) {
+            log.info("Created gateway group for {}", srcDeviceId);
+            return selectGroupHandler.createGatewayGroup(srcDeviceId, getGatewayNodes());
+        } else {
+            return group.id();
+        }
     }
 
     @Override
@@ -185,9 +193,8 @@ public class ScalableGatewayManager implements ScalableGatewayService {
         gatewayNodeMap.values()
                 .stream()
                 .map(Versioned::value)
-                .forEach(gatewayNode -> gatewayNodeList.add(gatewayNode));
+                .forEach(gatewayNodeList::add);
         return gatewayNodeList;
-
     }
 
     @Override
@@ -198,7 +205,6 @@ public class ScalableGatewayManager implements ScalableGatewayService {
                 .map(Versioned::value)
                 .forEach(gatewayNode -> deviceIdList.add(gatewayNode.getGatewayDeviceId()));
         return deviceIdList;
-
     }
 
     @Override
@@ -206,7 +212,7 @@ public class ScalableGatewayManager implements ScalableGatewayService {
         Versioned<GatewayNode> existingNode = gatewayNodeMap.putIfAbsent(
                 gatewayNode.getGatewayDeviceId(), gatewayNode);
         if (existingNode == null) {
-            updateGatewayLoadBalance(gatewayNode, true);
+            updateGatewayGroup(gatewayNode, true);
             log.info("Added {} to gateway pool", gatewayNode);
             return true;
         } else {
@@ -218,16 +224,22 @@ public class ScalableGatewayManager implements ScalableGatewayService {
     public boolean deleteGatewayNode(GatewayNode gatewayNode) {
         boolean result = gatewayNodeMap.remove(gatewayNode.getGatewayDeviceId(), gatewayNode);
         if (result) {
-            updateGatewayLoadBalance(gatewayNode, false);
+            updateGatewayGroup(gatewayNode, false);
+            log.info("Deleted gateway with device ID {}", gatewayNode.getGatewayDeviceId());
         }
         return result;
     }
 
-    private void updateGatewayLoadBalance(GatewayNode gatewayNode, boolean nodeInsertion) {
-        deviceService.getAvailableDevices().forEach(device ->
-                groupService.getGroups(device.id(), appId).forEach(group ->
-                        selectGroupHandler.updateBucketToSelectGroupInVxlan(device.id(), group.appCookie(),
-                                Lists.newArrayList(gatewayNode), nodeInsertion)));
+    private void updateGatewayGroup(GatewayNode gatewayNode, boolean isInsert) {
+        Tools.stream(deviceService.getAvailableDevices()).forEach(device -> {
+            Tools.stream(groupService.getGroups(device.id(), appId)).forEach(group -> {
+                selectGroupHandler.updateGatewayGroupBuckets(
+                        device.id(),
+                        ImmutableList.of(gatewayNode),
+                        isInsert);
+                log.trace("Updated gateway group on {}", device.id());
+            });
+        });
     }
 
     private class InternalConfigListener implements NetworkConfigListener {
@@ -257,7 +269,9 @@ public class ScalableGatewayManager implements ScalableGatewayService {
         public void event(DeviceEvent deviceEvent) {
             if (deviceEvent.type() == DeviceEvent.Type.DEVICE_SUSPENDED ||
                     deviceEvent.type() == DeviceEvent.Type.DEVICE_REMOVED) {
-                deleteGatewayNode(getGatewayNode(deviceEvent.subject().id()));
+                DeviceId deviceId = deviceEvent.subject().id();
+                deleteGatewayNode(getGatewayNode(deviceId));
+                log.warn("Gateway with device ID {} is disconnected", deviceId);
             }
         }
     }
@@ -269,8 +283,7 @@ public class ScalableGatewayManager implements ScalableGatewayService {
             return;
         }
 
-        config.gatewayNodes().forEach(gatewayNode -> addGatewayNode(gatewayNode));
-
+        config.gatewayNodes().forEach(this::addGatewayNode);
         log.info("ScalableGateway configured");
     }
 }

@@ -19,6 +19,8 @@ package org.onosproject.scalablegateway.impl;
 import com.google.common.collect.Lists;
 import org.onlab.packet.Ip4Address;
 import org.onosproject.core.ApplicationId;
+import org.onosproject.core.DefaultGroupId;
+import org.onosproject.core.GroupId;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Port;
 import org.onosproject.net.PortNumber;
@@ -36,7 +38,6 @@ import org.onosproject.net.flow.instructions.ExtensionTreatment;
 import org.onosproject.net.flow.instructions.ExtensionTreatmentType;
 import org.onosproject.net.group.DefaultGroupDescription;
 import org.onosproject.net.group.DefaultGroupKey;
-import org.onosproject.net.group.Group;
 import org.onosproject.net.group.GroupBucket;
 import org.onosproject.net.group.GroupBuckets;
 import org.onosproject.net.group.GroupDescription;
@@ -48,6 +49,7 @@ import org.slf4j.LoggerFactory;
 
 import java.util.List;
 
+import static org.onosproject.net.AnnotationKeys.PORT_NAME;
 import static org.onosproject.net.group.DefaultGroupBucket.createSelectGroupBucket;
 
 /**
@@ -59,7 +61,6 @@ public class SelectGroupHandler {
 
     private static final String TUNNEL_DESTINATION = "tunnelDst";
     private static final String PORTNAME_PREFIX_TUNNEL = "vxlan";
-    private static final String PORTNAME = "portName";
 
     private final GroupService groupService;
     private final DeviceService deviceService;
@@ -89,45 +90,61 @@ public class SelectGroupHandler {
      * @param nodeList gateway node list for bucket action
      * @return created select type group description
      */
-    public GroupDescription createSelectGroupInVxlan(DeviceId srcDeviceId, List<GatewayNode> nodeList) {
+    public GroupId createGatewayGroup(DeviceId srcDeviceId, List<GatewayNode> nodeList) {
         List<GroupBucket> bucketList = generateBucketsForSelectGroup(srcDeviceId, nodeList);
-        GroupKey key = generateGroupKey(srcDeviceId, nodeList);
-        return new DefaultGroupDescription(srcDeviceId, GroupDescription.Type.SELECT,
-                new GroupBuckets(bucketList), key, null, appId);
+        GroupId groupId = getGroupId(srcDeviceId);
+        GroupDescription groupDescription = new DefaultGroupDescription(
+                srcDeviceId,
+                GroupDescription.Type.SELECT,
+                new GroupBuckets(bucketList),
+                getGroupKey(srcDeviceId),
+                groupId.id(),
+                appId);
+
+        groupService.addGroup(groupDescription);
+        return groupId;
     }
 
-    private GroupKey generateGroupKey(DeviceId srcDeviceId, List<GatewayNode> nodeList) {
-        String cookie = srcDeviceId.toString();
-        for (GatewayNode node : nodeList) {
-            cookie = cookie.concat(node.getGatewayDeviceId().toString());
-        }
-        return new DefaultGroupKey(cookie.getBytes());
+    /**
+     * Returns unique group key with supplied source device ID as a hash.
+     *
+     * @param srcDeviceId source device id
+     * @return group key
+     */
+    public GroupKey getGroupKey(DeviceId srcDeviceId) {
+        return new DefaultGroupKey(srcDeviceId.toString().getBytes());
+    }
 
+    private GroupId getGroupId(DeviceId srcDeviceId) {
+        return new DefaultGroupId(srcDeviceId.toString().hashCode());
     }
 
     /**
      * Updates groupBuckets in select type group.
      *
-     * @param deviceId target device id for group description
-     * @param oldAppCookie group key for target group
+     * @param deviceId target device id to update the group
      * @param nodeList updated gateway node list for bucket action
-     * @param nodeInsertion update type(add or remove)
+     * @param isInsert update type(add or remove)
      * @return result of process
      */
-    public boolean updateBucketToSelectGroupInVxlan(DeviceId deviceId, GroupKey oldAppCookie,
-                                                    List<GatewayNode> nodeList, boolean nodeInsertion) {
+    public void updateGatewayGroupBuckets(DeviceId deviceId,
+                                          List<GatewayNode> nodeList,
+                                          boolean isInsert) {
         List<GroupBucket> bucketList = generateBucketsForSelectGroup(deviceId, nodeList);
-
-        GroupKey newAppCookie = generateGroupKey(deviceId, nodeList);
-        if (nodeInsertion) {
-            groupService.addBucketsToGroup(deviceId, oldAppCookie,
-                    new GroupBuckets(bucketList), newAppCookie, appId);
+        GroupKey groupKey = getGroupKey(deviceId);
+        if (isInsert) {
+            groupService.addBucketsToGroup(
+                    deviceId,
+                    groupKey,
+                    new GroupBuckets(bucketList),
+                    groupKey, appId);
         } else {
-            groupService.removeBucketsFromGroup(deviceId, oldAppCookie,
-                    new GroupBuckets(bucketList), newAppCookie, appId);
+            groupService.removeBucketsFromGroup(
+                    deviceId,
+                    groupKey,
+                    new GroupBuckets(bucketList),
+                    groupKey, appId);
         }
-        Group group = groupService.getGroup(deviceId, newAppCookie);
-        return group != null ? true : false;
     }
 
     private List<GroupBucket> generateBucketsForSelectGroup(DeviceId deviceId, List<GatewayNode> nodeList) {
@@ -175,7 +192,7 @@ public class SelectGroupHandler {
      */
     private PortNumber getTunnelPort(DeviceId deviceId) {
         Port port = deviceService.getPorts(deviceId).stream()
-                .filter(p -> p.annotations().value(PORTNAME).equals(PORTNAME_PREFIX_TUNNEL))
+                .filter(p -> p.annotations().value(PORT_NAME).equals(PORTNAME_PREFIX_TUNNEL))
                 .findAny().orElse(null);
 
         if (port == null) {