From b8c2d4fe088057c3e62ba7bb663e9a90077a579e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 18 Aug 2022 14:15:55 +0530 Subject: [PATCH 1/4] server: fix error when dedicatingguestvlanrange for physical nw without vlan range Signed-off-by: Abhishek Kumar --- .../com/cloud/network/NetworkServiceImpl.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index a3a3e4d9702c..ec7485760adb 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -4068,16 +4068,15 @@ public GuestVlanRange dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) { } } - // Verify guest vlans in the range don't belong to a network of a different account for (int i = startVlan; i <= endVlan; i++) { - List allocatedVlans = _dcVnetDao.listAllocatedVnetsInRange(physicalNetwork.getDataCenterId(), physicalNetwork.getId(), startVlan, endVlan); - if (allocatedVlans != null && !allocatedVlans.isEmpty()) { - for (DataCenterVnetVO allocatedVlan : allocatedVlans) { - if (allocatedVlan.getAccountId() != vlanOwner.getAccountId()) { - throw new InvalidParameterValueException("Guest vlan from this range " + allocatedVlan.getVnet() + " is allocated to a different account." - + " Can only dedicate a range which has no allocated vlans or has vlans allocated to the same account "); - } - } + List dataCenterVnet = _dcVnetDao.findVnet(physicalNetwork.getDataCenterId(), physicalNetworkId, Integer.toString(i)); + if (CollectionUtils.isEmpty(dataCenterVnet)) { + throw new InvalidParameterValueException(String.format("Guest vlan %d from this range %s is present in the system for physical network ID: %s", i, vlan, physicalNetwork.getUuid())); + } + // Verify guest vlans in the range don't belong to a network of a different account + if (dataCenterVnet.get(0).getAccountId() != vlanOwner.getAccountId()) { + throw new InvalidParameterValueException("Guest vlan from this range " + dataCenterVnet.get(0).getVnet() + " is allocated to a different account." + + " Can only dedicate a range which has no allocated vlans or has vlans allocated to the same account "); } } From 2651d0116535f37f340ce576316e33f5ad168eea Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 18 Aug 2022 17:25:30 +0530 Subject: [PATCH 2/4] fix Signed-off-by: Abhishek Kumar --- .../src/main/java/com/cloud/network/NetworkServiceImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index ec7485760adb..7475dcbb1aff 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -4071,10 +4071,10 @@ public GuestVlanRange dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) { for (int i = startVlan; i <= endVlan; i++) { List dataCenterVnet = _dcVnetDao.findVnet(physicalNetwork.getDataCenterId(), physicalNetworkId, Integer.toString(i)); if (CollectionUtils.isEmpty(dataCenterVnet)) { - throw new InvalidParameterValueException(String.format("Guest vlan %d from this range %s is present in the system for physical network ID: %s", i, vlan, physicalNetwork.getUuid())); + throw new InvalidParameterValueException(String.format("Guest vlan %d from this range %s is not present in the system for physical network ID: %s", i, vlan, physicalNetwork.getUuid())); } // Verify guest vlans in the range don't belong to a network of a different account - if (dataCenterVnet.get(0).getAccountId() != vlanOwner.getAccountId()) { + if (dataCenterVnet.get(0).getAccountId() != null && dataCenterVnet.get(0).getAccountId() != vlanOwner.getAccountId()) { throw new InvalidParameterValueException("Guest vlan from this range " + dataCenterVnet.get(0).getVnet() + " is allocated to a different account." + " Can only dedicate a range which has no allocated vlans or has vlans allocated to the same account "); } From c30877bfbf843960d543bce1e87d974215262666 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 22 Aug 2022 13:48:53 +0530 Subject: [PATCH 3/4] refactor Signed-off-by: Abhishek Kumar --- .../com/cloud/network/NetworkServiceImpl.java | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 7475dcbb1aff..0cfe2a3211fe 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -370,6 +370,20 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C private Map _configs; + private void verifyDedicatedGuestVlansWithExistingDatacenterVlans(PhysicalNetwork physicalNetwork, Account vlanOwner, int startVlan, int endVlan) { + for (int i = startVlan; i <= endVlan; i++) { + List dataCenterVnet = _dcVnetDao.findVnet(physicalNetwork.getDataCenterId(), physicalNetwork.getId(), Integer.toString(i)); + if (CollectionUtils.isEmpty(dataCenterVnet)) { + throw new InvalidParameterValueException(String.format("Guest vlan %d from this range %d-%d is not present in the system for physical network ID: %s", i, startVlan, endVlan, physicalNetwork.getUuid())); + } + // Verify guest vlans in the range don't belong to a network of a different account + if (dataCenterVnet.get(0).getAccountId() != null && dataCenterVnet.get(0).getAccountId() != vlanOwner.getAccountId()) { + throw new InvalidParameterValueException("Guest vlan from this range " + dataCenterVnet.get(0).getVnet() + " is allocated to a different account." + + " Can only dedicate a range which has no allocated vlans or has vlans allocated to the same account "); + } + } + } + /* Get a list of IPs, classify them by service */ protected Map> getIpToServices(List publicIps, boolean rulesRevoked, boolean includingFirewall) { Map> ipToServices = new HashMap>(); @@ -4068,17 +4082,7 @@ public GuestVlanRange dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) { } } - for (int i = startVlan; i <= endVlan; i++) { - List dataCenterVnet = _dcVnetDao.findVnet(physicalNetwork.getDataCenterId(), physicalNetworkId, Integer.toString(i)); - if (CollectionUtils.isEmpty(dataCenterVnet)) { - throw new InvalidParameterValueException(String.format("Guest vlan %d from this range %s is not present in the system for physical network ID: %s", i, vlan, physicalNetwork.getUuid())); - } - // Verify guest vlans in the range don't belong to a network of a different account - if (dataCenterVnet.get(0).getAccountId() != null && dataCenterVnet.get(0).getAccountId() != vlanOwner.getAccountId()) { - throw new InvalidParameterValueException("Guest vlan from this range " + dataCenterVnet.get(0).getVnet() + " is allocated to a different account." - + " Can only dedicate a range which has no allocated vlans or has vlans allocated to the same account "); - } - } + verifyDedicatedGuestVlansWithExistingDatacenterVlans(physicalNetwork, vlanOwner, startVlan, endVlan); List guestVlanMaps = _accountGuestVlanMapDao.listAccountGuestVlanMapsByPhysicalNetwork(physicalNetworkId); // Verify if vlan range is already dedicated From 8e19aa403f2f3008728b837b92dda598f9586471 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 22 Aug 2022 16:53:15 +0530 Subject: [PATCH 4/4] fix test Signed-off-by: Abhishek Kumar --- .../network/DedicateGuestVlanRangesTest.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/test/java/com/cloud/network/DedicateGuestVlanRangesTest.java b/server/src/test/java/com/cloud/network/DedicateGuestVlanRangesTest.java index 8687465f3c69..726f99e9a01d 100644 --- a/server/src/test/java/com/cloud/network/DedicateGuestVlanRangesTest.java +++ b/server/src/test/java/com/cloud/network/DedicateGuestVlanRangesTest.java @@ -29,9 +29,10 @@ import java.util.List; import java.util.UUID; -import com.cloud.user.User; -import junit.framework.Assert; - +import org.apache.cloudstack.api.command.admin.network.DedicateGuestVlanRangeCmd; +import org.apache.cloudstack.api.command.admin.network.ListDedicatedGuestVlanRangesCmd; +import org.apache.cloudstack.api.command.admin.network.ReleaseDedicatedGuestVlanRangeCmd; +import org.apache.cloudstack.context.CallContext; import org.apache.log4j.Logger; import org.junit.After; import org.junit.Before; @@ -39,11 +40,6 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import org.apache.cloudstack.api.command.admin.network.DedicateGuestVlanRangeCmd; -import org.apache.cloudstack.api.command.admin.network.ListDedicatedGuestVlanRangesCmd; -import org.apache.cloudstack.api.command.admin.network.ReleaseDedicatedGuestVlanRangeCmd; -import org.apache.cloudstack.context.CallContext; - import com.cloud.dc.DataCenterVnetVO; import com.cloud.dc.dao.DataCenterVnetDao; import com.cloud.network.dao.AccountGuestVlanMapDao; @@ -54,10 +50,13 @@ import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; +import com.cloud.user.User; import com.cloud.user.UserVO; import com.cloud.user.dao.AccountDao; import com.cloud.utils.db.TransactionLegacy; +import junit.framework.Assert; + public class DedicateGuestVlanRangesTest { private static final Logger s_logger = Logger.getLogger(DedicateGuestVlanRangesTest.class); @@ -275,7 +274,7 @@ void runDedicateGuestVlanRangeAllocatedVlans() throws Exception { DataCenterVnetVO dataCenter = new DataCenterVnetVO("2-5", 1L, 1L); dataCenter.setAccountId(1L); dataCenterList.add(dataCenter); - when(networkService._dcVnetDao.listAllocatedVnetsInRange(anyLong(), anyLong(), anyInt(), anyInt())).thenReturn(dataCenterList); + when(networkService._dcVnetDao.findVnet(anyLong(), anyLong(), anyString())).thenReturn(dataCenterList); try { networkService.dedicateGuestVlanRange(dedicateGuestVlanRangesCmd); @@ -298,7 +297,8 @@ void runDedicateGuestVlanRangeDedicatedRange() throws Exception { when(networkService._physicalNetworkDao.findById(anyLong())).thenReturn(physicalNetwork); - when(networkService._dcVnetDao.listAllocatedVnetsInRange(anyLong(), anyLong(), anyInt(), anyInt())).thenReturn(null); + DataCenterVnetVO dataCenterVnetVO = new DataCenterVnetVO("2-5", 1L, 1L); + when(networkService._dcVnetDao.findVnet(anyLong(), anyLong(), anyString())).thenReturn(List.of(dataCenterVnetVO)); List guestVlanMaps = new ArrayList(); AccountGuestVlanMapVO accountGuestVlanMap = new AccountGuestVlanMapVO(1L, 1L);