Fix race condition in subnet deletion API

services/subnet_manager/src/main/java/com/futurewei/alcor/subnet/controller/SubnetController.java

@@ -467,18 +467,10 @@ public ResponseId deleteSubnetState(@PathVariable String projectId, @PathVariabl
             }
 
             String rangeId = null;
-            String ipV4RangeId = subnetEntity.getIpV4RangeId();
-            String ipV6RangeId = subnetEntity.getIpV6RangeId();
-            if (ipV4RangeId != null) {
-                rangeId = ipV4RangeId;
+            if (subnetEntity.getIpVersion() == 6) {
+                rangeId = subnetEntity.getIpV6RangeId();
             } else {
-                rangeId = ipV6RangeId;
-            }
-
-            // TODO: check if there is any gateway / non-gateway port for the subnet, waiting for PM new API
-            Boolean checkIfAnyNoneGatewayPortInSubnet = this.subnetService.checkIfAnyPortInSubnet(projectId, subnetId);
-            if (checkIfAnyNoneGatewayPortInSubnet) {
-                throw new HavePortInSubnet();
+                rangeId = subnetEntity.getIpV4RangeId();
             }
 
             // check if subnet bind any router
@@ -494,7 +486,13 @@ public ResponseId deleteSubnetState(@PathVariable String projectId, @PathVariabl
                 logger.warn(e.getMessage());
             }
 
-            // TODO: delete gateway port in port manager. Temporary solution, need PM fix issue
+            // check if there is any non-gateway port for the subnet
+            boolean checkIfAnyNoneGatewayPortInSubnet = this.subnetService.checkIfAnyNonGatewayPortInSubnet(projectId, subnetEntity);
+            if (checkIfAnyNoneGatewayPortInSubnet) {
+                throw new HaveNonGatewayPortInSubnet();
+            }
+
+            // delete gateway port in port manager
             GatewayPortDetail gatewayPortDetail = subnetEntity.getGatewayPortDetail();
             if (gatewayPortDetail != null) {
                 this.subnetToPortManagerService.deleteGatewayPort(projectId, gatewayPortDetail.getGatewayPortId());
@@ -508,7 +506,7 @@ public ResponseId deleteSubnetState(@PathVariable String projectId, @PathVariabl
 
             this.subnetDatabaseService.deleteSubnet(subnetId);
 
-        } catch (ParameterNullOrEmptyException | HavePortInSubnet | SubnetBindRouter e) {
+        } catch (ParameterNullOrEmptyException | HaveNonGatewayPortInSubnet | SubnetBindRouter e) {
             logger.error(e.getMessage());
             throw new Exception(e);
         }

services/subnet_manager/src/main/java/com/futurewei/alcor/subnet/exception/HavePortInSubnet.java → services/subnet_manager/src/main/java/com/futurewei/alcor/subnet/exception/HaveNonGatewayPortInSubnet.java

@@ -18,6 +18,6 @@ free of charge, to any person obtaining a copy of this software and associated d
 import org.springframework.http.HttpStatus;
 import org.springframework.web.bind.annotation.ResponseStatus;
 
-@ResponseStatus(code= HttpStatus.CONFLICT, reason="There is some ports in the subnet, we can Not delete subnet")
-public class HavePortInSubnet extends Exception {
+@ResponseStatus(code= HttpStatus.CONFLICT, reason="There is some customer ports in the subnet, we can Not delete subnet")
+public class HaveNonGatewayPortInSubnet extends Exception {
 }

services/subnet_manager/src/main/java/com/futurewei/alcor/subnet/service/SubnetService.java

@@ -69,7 +69,7 @@ public void fallbackOperation (AtomicReference<RouteWebJson> routeResponseAtomic
     public void deleteSubnetIdInVpc (String subnetId, String projectId, String vpcId) throws Exception;
 
     // check if there is any port in this subnet
-    public boolean checkIfAnyPortInSubnet (String projectId, String subnetId) throws SubnetIdIsNull;
+    public boolean checkIfAnyNonGatewayPortInSubnet(String projectId, SubnetEntity subnetEntity) throws SubnetIdIsNull;
 
     // check if subnet bind any routes
     public boolean checkIfSubnetBindAnyRouter(SubnetEntity subnetEntity);

services/subnet_manager/src/main/java/com/futurewei/alcor/subnet/service/implement/SubnetServiceImp.java

@@ -39,6 +39,7 @@ free of charge, to any person obtaining a copy of this software and associated d
 import com.futurewei.alcor.web.entity.route.*;
 import com.futurewei.alcor.web.entity.subnet.*;
 import com.futurewei.alcor.web.entity.vpc.VpcWebJson;
+import io.netty.util.internal.StringUtil;
 import org.apache.commons.net.util.SubnetUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -304,12 +305,15 @@ public void deleteSubnetIdInVpc(String subnetId, String projectId, String vpcId)
     }
 
     @Override
-    public boolean checkIfAnyPortInSubnet(String projectId, String subnetId) throws SubnetIdIsNull {
-        if (subnetId == null) {
+    public boolean checkIfAnyNonGatewayPortInSubnet(String projectId, SubnetEntity subnetEntity) throws SubnetIdIsNull {
+        if (subnetEntity == null || StringUtil.isNullOrEmpty(subnetEntity.getId())) {
             throw new SubnetIdIsNull();
         }
-        String portManagerServiceUrl = portUrl + "project/" + projectId + "/subnet-port-count/" + subnetId;
-        int  portCount = restTemplate.getForObject(portManagerServiceUrl, Integer.class);
+
+        String portManagerServiceUrl = portUrl + "project/" + projectId + "/subnet-port-count/" + subnetEntity.getId();
+        int portCount = restTemplate.getForObject(portManagerServiceUrl, Integer.class);
+
+        logger.info("[checkIfAnyNonGatewayPortInSubnet]: portCount == " + portCount + " && subnetEntity.getGatewayPortId() = " + subnetEntity.getGatewayPortId());
         if (portCount == 0) {
             return false;
         }
@@ -321,7 +325,7 @@ public boolean checkIfAnyPortInSubnet(String projectId, String subnetId) throws
     public boolean checkIfSubnetBindAnyRouter(SubnetEntity subnetEntity) {
 
         String attachedRouterId = subnetEntity.getAttachedRouterId();
-        if (attachedRouterId == null || attachedRouterId.equals("")){
+        if (attachedRouterId == null || attachedRouterId.equals("")) {
             return false;
         }
 
@@ -330,7 +334,7 @@ public boolean checkIfSubnetBindAnyRouter(SubnetEntity subnetEntity) {
 
     @Override
     @DurationStatistics
-    public boolean checkIfCidrOverlap(String cidr,String projectId, String vpcId) throws FallbackException, ResourceNotFoundException, ResourcePersistenceException, CidrNotWithinNetworkCidr, CidrOverlapWithOtherSubnets {
+    public boolean checkIfCidrOverlap(String cidr, String projectId, String vpcId) throws FallbackException, ResourceNotFoundException, ResourcePersistenceException, CidrNotWithinNetworkCidr, CidrOverlapWithOtherSubnets {
 
         // get vpc and check with vpc cidr
         VpcWebJson vpcWebJson = verifyVpcId(projectId, vpcId);
@@ -341,8 +345,7 @@ public boolean checkIfCidrOverlap(String cidr,String projectId, String vpcId) th
                 throw new CidrNotWithinNetworkCidr();
             }
         }
-
-
+
         // get subnet list and check with subnets cidr
         List<String> subnetIds = vpcWebJson.getNetwork().getSubnets();
         for (String subnetId : subnetIds) {
@@ -552,7 +555,7 @@ public void deleteIPRangeInPIM(String rangeId) {
             return;
         }
 
-        String ipManagerCreateRangeUrl = ipUrl + "range/"+ rangeId;
+        String ipManagerCreateRangeUrl = ipUrl + "range/" + rangeId;
         restTemplate.delete(ipManagerCreateRangeUrl);
     }
 

services/vpc_manager/src/main/java/com/futurewei/alcor/vpcmanager/controller/VpcController.java

@@ -68,7 +68,7 @@ public class VpcController {
      * @return vpc state
      * @throws Exception
      */
-    @Rbac(resource ="vpc")
+    @Rbac(resource = "vpc")
     @FieldFilter(type = VpcEntity.class)
     @RequestMapping(
             method = GET,
@@ -114,7 +114,7 @@ public VpcsWebJson createVpcStateBulk(@PathVariable String projectid, @RequestBo
      * @return vpc state
      * @throws Exception
      */
-    @Rbac(resource ="vpc")
+    @Rbac(resource = "vpc")
     @RequestMapping(
             method = POST,
             value = {"/project/{projectid}/vpcs"})
@@ -194,7 +194,7 @@ public VpcWebJson createVpcState(@PathVariable String projectid, @RequestBody Vp
      * @return vpc state
      * @throws Exception
      */
-    @Rbac(resource ="vpc")
+    @Rbac(resource = "vpc")
     @RequestMapping(
             method = PUT,
             value = {"/project/{projectid}/vpcs/{vpcid}"})
@@ -252,7 +252,7 @@ public VpcWebJson updateVpcStateByVpcId(@PathVariable String projectid, @PathVar
      * @return network id
      * @throws Exception
      */
-    @Rbac(resource ="vpc")
+    @Rbac(resource = "vpc")
     @RequestMapping(
             method = DELETE,
             value = {"/project/{projectid}/vpcs/{vpcid}"})
@@ -290,16 +290,16 @@ public ResponseId deleteVpcStateByVpcId(@PathVariable String projectid, @PathVar
      * @return Map<String, VpcWebResponseObject>
      * @throws Exception
      */
-    @Rbac(resource ="vpc")
+    @Rbac(resource = "vpc")
     @FieldFilter(type = VpcEntity.class)
     @RequestMapping(
             method = GET,
             value = "/project/{projectId}/vpcs")
     @DurationStatistics
     public VpcsWebJson getVpcStatesByProjectId(@PathVariable String projectId) throws Exception {
         Map<String, VpcEntity> vpcStates = null;
-        Map<String, String[]> requestParams = (Map<String, String[]>)request.getAttribute(QUERY_ATTR_HEADER);
-        requestParams = requestParams == null ? request.getParameterMap():requestParams;
+        Map<String, String[]> requestParams = (Map<String, String[]>) request.getAttribute(QUERY_ATTR_HEADER);
+        requestParams = requestParams == null ? request.getParameterMap() : requestParams;
         Map<String, Object[]> queryParams =
                 ControllerUtil.transformUrlPathParams(requestParams, VpcEntity.class);
 
@@ -339,6 +339,7 @@ public Map getVpcCountAndAllVpcStates() throws CacheException {
 
     /**
      * Updates a network with subnet id
+     *
      * @param projectid
      * @param vpcid
      * @param subnetid
@@ -381,11 +382,11 @@ public VpcWebJson addSubnetIdToVpcState(@PathVariable String projectid, @PathVar
         }
 
         return new VpcWebJson(inVpcState);
-
     }
 
     /**
      * delete subnet id in a network
+     *
      * @param projectid
      * @param vpcid
      * @param subnetid
@@ -398,35 +399,19 @@ public VpcWebJson addSubnetIdToVpcState(@PathVariable String projectid, @PathVar
     @DurationStatistics
     public VpcWebJson deleteSubnetIdInVpcState(@PathVariable String projectid, @PathVariable String vpcid, @PathVariable String subnetid) throws Exception {
 
-        VpcEntity inVpcState = new VpcEntity();
+        VpcEntity inVpcState = null;
 
         try {
             RestPreconditionsUtil.verifyParameterNotNullorEmpty(projectid);
             RestPreconditionsUtil.verifyParameterNotNullorEmpty(vpcid);
             RestPreconditionsUtil.verifyParameterNotNullorEmpty(subnetid);
 
-            inVpcState = this.vpcDatabaseService.getByVpcId(vpcid);
-            if (inVpcState == null) {
-                throw new ResourceNotFoundException("Vpc not found : " + vpcid);
-            }
-
-            List<String> subnets = inVpcState.getSubnets();
-            if (subnets == null || !subnets.contains(subnetid)) {
-                return new VpcWebJson(inVpcState);
-            }
-            subnets.remove(subnetid);
-
-            inVpcState.setSubnets(subnets);
-
-            this.vpcDatabaseService.addVpc(inVpcState);
-
-            inVpcState = this.vpcDatabaseService.getByVpcId(vpcid);
+            inVpcState = this.vpcDatabaseService.deleteSubnetIdInVpc(vpcid, subnetid);
 
         } catch (ParameterNullOrEmptyException e) {
             throw new Exception(e);
         }
 
         return new VpcWebJson(inVpcState);
-
     }
 }

services/vpc_manager/src/main/java/com/futurewei/alcor/vpcmanager/dao/VpcRepository.java

@@ -18,6 +18,7 @@ free of charge, to any person obtaining a copy of this software and associated d
 import com.futurewei.alcor.common.db.CacheException;
 import com.futurewei.alcor.common.db.CacheFactory;
 import com.futurewei.alcor.common.db.ICache;
+import com.futurewei.alcor.common.db.Transaction;
 import com.futurewei.alcor.common.db.repo.ICacheRepository;
 import com.futurewei.alcor.common.logging.Logger;
 import com.futurewei.alcor.common.logging.LoggerFactory;
@@ -43,6 +44,14 @@ public ICache<String, VpcEntity> getCache() {
 
     private ICache<String, VpcEntity> cache;
 
+    public Transaction startTransaction() throws CacheException {
+        return cache.getTransaction().start();
+    }
+
+    public void commitTransaction() throws CacheException {
+        cache.getTransaction().commit();
+    }
+
     @Autowired
     public VpcRepository(CacheFactory cacheFactory) {
         cache = cacheFactory.getCache(VpcEntity.class);

services/vpc_manager/src/main/java/com/futurewei/alcor/vpcmanager/service/Impl/VpcDatabaseServiceImpl.java

@@ -18,15 +18,18 @@ free of charge, to any person obtaining a copy of this software and associated d
 import com.futurewei.alcor.common.db.CacheException;
 import com.futurewei.alcor.common.db.ICache;
 import com.futurewei.alcor.common.exception.DatabasePersistenceException;
+import com.futurewei.alcor.common.exception.ResourceNotFoundException;
 import com.futurewei.alcor.common.stats.DurationStatistics;
 import com.futurewei.alcor.vpcmanager.dao.VpcRepository;
 import com.futurewei.alcor.vpcmanager.service.VpcDatabaseService;
 import com.futurewei.alcor.web.entity.vpc.VpcEntity;
+import com.futurewei.alcor.web.entity.vpc.VpcWebJson;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
+import java.util.List;
 import java.util.Map;
 
 @Service
@@ -42,7 +45,7 @@ public class VpcDatabaseServiceImpl implements VpcDatabaseService {
     public VpcEntity getByVpcId(String vpcId) {
         try {
             return this.vpcRepository.findItem(vpcId);
-        }catch (Exception e) {
+        } catch (Exception e) {
             return null;
         }
     }
@@ -75,6 +78,35 @@ public void deleteVpc(String id) throws CacheException {
         this.vpcRepository.deleteItem(id);
     }
 
+    @Override
+    public VpcEntity deleteSubnetIdInVpc(String vpcId, String subnetId) throws ResourceNotFoundException, DatabasePersistenceException, CacheException {
+
+        VpcEntity currentVpcState = null;
+
+        try {
+            this.vpcRepository.startTransaction();
+            currentVpcState = getByVpcId(vpcId);
+            if (currentVpcState == null) {
+                throw new ResourceNotFoundException("Vpc not found : " + vpcId);
+            }
+
+            List<String> subnets = currentVpcState.getSubnets();
+            if (subnets == null || !subnets.contains(subnetId)) {
+                return currentVpcState;
+            }
+
+            subnets.remove(subnetId);
+            currentVpcState.setSubnets(subnets);
+            addVpc(currentVpcState);
+        } catch (ResourceNotFoundException | DatabasePersistenceException | CacheException e) {
+            throw e;
+        } finally {
+            this.vpcRepository.commitTransaction();
+        }
+
+        return currentVpcState;
+    }
+
     @Override
     @DurationStatistics
     public ICache<String, VpcEntity> getCache() {

services/vpc_manager/src/main/java/com/futurewei/alcor/vpcmanager/service/VpcDatabaseService.java

@@ -31,6 +31,7 @@ public interface VpcDatabaseService {
     public Map getAllVpcs (Map<String, Object[]> queryParams) throws CacheException;
     public void addVpc (VpcEntity vpcState) throws DatabasePersistenceException;
     public void deleteVpc (String id) throws CacheException;
+    public VpcEntity deleteSubnetIdInVpc(String vpcId, String subnetId) throws ResourceNotFoundException, DatabasePersistenceException, CacheException;
     public ICache<String, VpcEntity> getCache ();
 
 }

web/src/main/java/com/futurewei/alcor/web/entity/subnet/SubnetEntity.java

@@ -150,6 +150,7 @@ public SubnetEntity(String projectId, String vpcId, String id, String name, Stri
         super(projectId, id, name, null);
         this.vpcId = vpcId;
         this.cidr = cidr;
+        this.ipVersion = 4;
     }
 
     public SubnetEntity(String projectId, String id, String name, String description, String vpcId,