From 86939e7f9dae71c09c98e7bc7f53c5b37bbad4ce Mon Sep 17 00:00:00 2001 From: Spaceman1984 <49917670+Spaceman1984@users.noreply.github.com> Date: Wed, 12 Aug 2020 09:59:12 +0200 Subject: [PATCH] server: Fixed private gateway can't be deleted (#4016) When the static route service is not available on the VPC and a static route is created, the static route is created in a revoked state. Currently, the UI doesn't distinguish between active or revoked static routes. This PR adds the missing state filter to the list routes command and only lists active routes in the UI. It also ignores revoked routes when the private gateway is being removed but clears out the inactive routes before the gateway is removed. Fixes #2908 --- .../command/user/vpc/CreateStaticRouteCmd.java | 2 +- .../command/user/vpc/ListStaticRoutesCmd.java | 7 +++++++ .../cloud/network/vpc/dao/StaticRouteDao.java | 2 ++ .../network/vpc/dao/StaticRouteDaoImpl.java | 9 +++++++++ .../com/cloud/network/vpc/VpcManagerImpl.java | 18 +++++++++++++++++- ui/scripts/vpc.js | 3 ++- 6 files changed, 38 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreateStaticRouteCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreateStaticRouteCmd.java index 622143fa38d..849d1ecbc0b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreateStaticRouteCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreateStaticRouteCmd.java @@ -109,7 +109,7 @@ public class CreateStaticRouteCmd extends BaseAsyncCreateCmd { routeResponse.setResponseName(getCommandName()); } finally { if (!success || route == null) { - _vpcService.revokeStaticRoute(getEntityId()); + _entityMgr.remove(StaticRoute.class, getEntityId()); throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create static route"); } } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListStaticRoutesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListStaticRoutesCmd.java index 3dba84c8a72..3ad2f444f69 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListStaticRoutesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListStaticRoutesCmd.java @@ -48,6 +48,9 @@ public class ListStaticRoutesCmd extends BaseListTaggedResourcesCmd { @Parameter(name = ApiConstants.GATEWAY_ID, type = CommandType.UUID, entityType = PrivateGatewayResponse.class, description = "list static routes by gateway id") private Long gatewayId; + @Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "list static routes by state") + private String state; + public Long getId() { return id; } @@ -60,6 +63,10 @@ public class ListStaticRoutesCmd extends BaseListTaggedResourcesCmd { return gatewayId; } + public String getState() { + return state; + } + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// diff --git a/engine/schema/src/main/java/com/cloud/network/vpc/dao/StaticRouteDao.java b/engine/schema/src/main/java/com/cloud/network/vpc/dao/StaticRouteDao.java index 07c5ee160bd..750b477bdc9 100644 --- a/engine/schema/src/main/java/com/cloud/network/vpc/dao/StaticRouteDao.java +++ b/engine/schema/src/main/java/com/cloud/network/vpc/dao/StaticRouteDao.java @@ -30,6 +30,8 @@ public interface StaticRouteDao extends GenericDao { List listByVpcId(long vpcId); + List listByGatewayId(long gatewayId); + long countRoutesByGateway(long gatewayId); } diff --git a/engine/schema/src/main/java/com/cloud/network/vpc/dao/StaticRouteDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/vpc/dao/StaticRouteDaoImpl.java index 596cf983b20..671bf450758 100644 --- a/engine/schema/src/main/java/com/cloud/network/vpc/dao/StaticRouteDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/vpc/dao/StaticRouteDaoImpl.java @@ -62,6 +62,7 @@ public class StaticRouteDaoImpl extends GenericDaoBase impl RoutesByGatewayCount = createSearchBuilder(Long.class); RoutesByGatewayCount.select(null, Func.COUNT, RoutesByGatewayCount.entity().getId()); RoutesByGatewayCount.and("gatewayId", RoutesByGatewayCount.entity().getVpcGatewayId(), Op.EQ); + RoutesByGatewayCount.and("state", RoutesByGatewayCount.entity().getState(), Op.EQ); RoutesByGatewayCount.done(); } @@ -91,10 +92,18 @@ public class StaticRouteDaoImpl extends GenericDaoBase impl return listBy(sc); } + @Override + public List listByGatewayId(long gatewayId) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("gatewayId", gatewayId); + return listBy(sc); + } + @Override public long countRoutesByGateway(long gatewayId) { SearchCriteria sc = RoutesByGatewayCount.create(); sc.setParameters("gatewayId", gatewayId); + sc.setParameters("state", "Active"); return customSearch(sc, null).get(0); } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 620e5510dda..ccb8eea8517 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -2027,7 +2027,10 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis } } - // 2) Delete private gateway from the DB + // 2) Clean up any remaining routes + cleanUpRoutesByGatewayId(gatewayId); + + // 3) Delete private gateway from the DB return deletePrivateGatewayFromTheDB(gateway); } finally { @@ -2037,6 +2040,13 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis } } + private void cleanUpRoutesByGatewayId(long gatewayId){ + List routes = _staticRouteDao.listByGatewayId(gatewayId); + for (StaticRouteVO route: routes){ + _staticRouteDao.remove(route.getId()); + } + } + @DB protected boolean deletePrivateGatewayFromTheDB(final PrivateGateway gateway) { // check if there are ips allocted in the network @@ -2329,6 +2339,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis final List permittedAccounts = new ArrayList(); final Map tags = cmd.getTags(); final Long projectId = cmd.getProjectId(); + final String state = cmd.getState(); final Ternary domainIdRecursiveListProject = new Ternary(domainId, isRecursive, null); @@ -2344,6 +2355,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); sb.and("vpcId", sb.entity().getVpcId(), SearchCriteria.Op.EQ); sb.and("vpcGatewayId", sb.entity().getVpcGatewayId(), SearchCriteria.Op.EQ); + sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); if (tags != null && !tags.isEmpty()) { final SearchBuilder tagSearch = _resourceTagDao.createSearchBuilder(); @@ -2371,6 +2383,10 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis sc.addAnd("vpcGatewayId", Op.EQ, gatewayId); } + if (state != null) { + sc.addAnd("state", Op.EQ, state); + } + if (tags != null && !tags.isEmpty()) { int count = 0; sc.setJoinParameters("tagSearch", "resourceType", ResourceObjectType.StaticRoute.toString()); diff --git a/ui/scripts/vpc.js b/ui/scripts/vpc.js index f7fb47805cc..2b3b494466f 100644 --- a/ui/scripts/vpc.js +++ b/ui/scripts/vpc.js @@ -2652,7 +2652,8 @@ url: createURL('listStaticRoutes'), data: { gatewayid: args.context.vpcGateways[0].id, - listAll: true + listAll: true, + state: "Active" }, success: function(json) { var items = json.liststaticroutesresponse.staticroute;