Updated Branches: refs/heads/4.2 d3b690238 -> 4fdb177bb
Cloudstack-2150 DB table entries of phisical network is not proper.Shows Duplicate entries Cloudstack-2980 Adding a VLAN range that overlaps with two existing ranges results in inconsistent DB entries Signed-off-by: Abhinandan Prateek <aprat...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/4fdb177b Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/4fdb177b Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/4fdb177b Branch: refs/heads/4.2 Commit: 4fdb177bb602bd3a81ef62b0b754bc93f8e1dc93 Parents: d3b6902 Author: Bharat Kumar <bharat.ku...@citrix.com> Authored: Wed Jul 17 13:05:11 2013 +0530 Committer: Abhinandan Prateek <aprat...@apache.org> Committed: Wed Jul 17 15:10:52 2013 +0530 ---------------------------------------------------------------------- .../src/com/cloud/dc/dao/DataCenterDao.java | 2 +- .../src/com/cloud/dc/dao/DataCenterDaoImpl.java | 4 +- .../src/com/cloud/dc/dao/DataCenterVnetDao.java | 4 +- .../com/cloud/dc/dao/DataCenterVnetDaoImpl.java | 28 +++- .../com/cloud/network/NetworkServiceImpl.java | 160 +++++++------------ .../network/UpdatePhysicalNetworkTest.java | 11 +- 6 files changed, 87 insertions(+), 122 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4fdb177b/engine/schema/src/com/cloud/dc/dao/DataCenterDao.java ---------------------------------------------------------------------- diff --git a/engine/schema/src/com/cloud/dc/dao/DataCenterDao.java b/engine/schema/src/com/cloud/dc/dao/DataCenterDao.java index ed6e696..e5e2cdd 100755 --- a/engine/schema/src/com/cloud/dc/dao/DataCenterDao.java +++ b/engine/schema/src/com/cloud/dc/dao/DataCenterDao.java @@ -70,7 +70,7 @@ public interface DataCenterDao extends GenericDao<DataCenterVO, Long> { int countZoneVlans(long dcId, boolean onlyCountAllocated); - void addVnet(long dcId, long physicalNetworkId, int start, int end); + void addVnet(long dcId, long physicalNetworkId, List<String> vnets); void deleteVnet(long physicalNetworkId); List<DataCenterVnetVO> listAllocatedVnets(long physicalNetworkId); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4fdb177b/engine/schema/src/com/cloud/dc/dao/DataCenterDaoImpl.java ---------------------------------------------------------------------- diff --git a/engine/schema/src/com/cloud/dc/dao/DataCenterDaoImpl.java b/engine/schema/src/com/cloud/dc/dao/DataCenterDaoImpl.java index 503306f..47b5522 100755 --- a/engine/schema/src/com/cloud/dc/dao/DataCenterDaoImpl.java +++ b/engine/schema/src/com/cloud/dc/dao/DataCenterDaoImpl.java @@ -268,8 +268,8 @@ public class DataCenterDaoImpl extends GenericDaoBase<DataCenterVO, Long> implem } @Override - public void addVnet(long dcId, long physicalNetworkId, int start, int end) { - _vnetAllocDao.add(dcId, physicalNetworkId, start, end); + public void addVnet(long dcId, long physicalNetworkId, List<String> vnets) { + _vnetAllocDao.add(dcId, physicalNetworkId, vnets); } @Override http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4fdb177b/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDao.java ---------------------------------------------------------------------- diff --git a/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDao.java b/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDao.java index 778498d..e2e6b79 100644 --- a/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDao.java +++ b/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDao.java @@ -29,7 +29,7 @@ public interface DataCenterVnetDao extends GenericDao<DataCenterVnetVO, Long> { public int countZoneVlans(long dcId, boolean onlyCountAllocated); public List<DataCenterVnetVO> findVnet(long dcId, long physicalNetworkId, String vnet); - public void add(long dcId, long physicalNetworkId, int start, int end); + public void add(long dcId, long physicalNetworkId, List<String> vnets); public void delete(long physicalNetworkId); @@ -46,4 +46,6 @@ public interface DataCenterVnetDao extends GenericDao<DataCenterVnetVO, Long> { public int countVnetsAllocatedToAccount(long dcId, long accountId); public int countVnetsDedicatedToAccount(long dcId, long accountId); + + List<String> listVnetsByPhysicalNetworkAndDataCenter(long dcId, long physicalNetworkId); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4fdb177b/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDaoImpl.java ---------------------------------------------------------------------- diff --git a/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDaoImpl.java b/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDaoImpl.java index e97f2c6..ced2982 100755 --- a/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDaoImpl.java +++ b/engine/schema/src/com/cloud/dc/dao/DataCenterVnetDaoImpl.java @@ -64,6 +64,7 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long private final GenericSearchBuilder<DataCenterVnetVO, Integer> countVnetsAllocatedToAccount; protected GenericSearchBuilder<DataCenterVnetVO, Integer> countVnetsDedicatedToAccount; protected SearchBuilder<AccountGuestVlanMapVO> AccountGuestVlanMapSearch; + protected GenericSearchBuilder<DataCenterVnetVO, String> ListAllVnetSearch; @Inject protected AccountGuestVlanMapDao _accountGuestVlanMapDao; @@ -112,15 +113,15 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long } @DB - public void add(long dcId, long physicalNetworkId, int start, int end) { + public void add(long dcId, long physicalNetworkId, List<String> vnets) { String insertVnet = "INSERT INTO `cloud`.`op_dc_vnet_alloc` (vnet, data_center_id, physical_network_id) VALUES ( ?, ?, ?)"; Transaction txn = Transaction.currentTxn(); try { txn.start(); PreparedStatement stmt = txn.prepareAutoCloseStatement(insertVnet); - for (int i = start; i <= end; i++) { - stmt.setString(1, String.valueOf(i)); + for (int i =0; i <= vnets.size()-1; i++) { + stmt.setString(1, vnets.get(i)); stmt.setLong(2, dcId); stmt.setLong(3, physicalNetworkId); stmt.addBatch(); @@ -128,11 +129,7 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long stmt.executeBatch(); txn.commit(); } catch (SQLException e) { - if (!e.getMessage().contains("Duplicate")){ - txn.rollback(); - txn.close(); - throw new CloudRuntimeException("Exception caught adding vnet ", e); - } + throw new CloudRuntimeException(e.getMessage()); } } @@ -228,6 +225,14 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long } @Override + public List<String> listVnetsByPhysicalNetworkAndDataCenter(long dcId, long physicalNetworkId){ + SearchCriteria<String> sc = ListAllVnetSearch.create(); + sc.setParameters("dc", dcId ); + sc.setParameters("physicalNetworkId", physicalNetworkId); + return customSearch(sc, null); + } + + @Override public boolean configure(String name, Map<String, Object> params) throws ConfigurationException { boolean result = super.configure(name, params); @@ -314,5 +319,12 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long countVnetsAllocatedToAccount.and("accountId", countVnetsAllocatedToAccount.entity().getAccountId(), SearchCriteria.Op.EQ); countVnetsAllocatedToAccount.select(null, Func.COUNT, countVnetsAllocatedToAccount.entity().getId()); countVnetsAllocatedToAccount.done(); + + ListAllVnetSearch = createSearchBuilder(String.class); + ListAllVnetSearch.select(null, Func.NATIVE, ListAllVnetSearch.entity().getVnet()); + ListAllVnetSearch.and("dc", ListAllVnetSearch.entity().getDataCenterId(), Op.EQ); + ListAllVnetSearch.and("physicalNetworkId", ListAllVnetSearch.entity().getPhysicalNetworkId(), Op.EQ); + ListAllVnetSearch.done(); + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4fdb177b/server/src/com/cloud/network/NetworkServiceImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/NetworkServiceImpl.java b/server/src/com/cloud/network/NetworkServiceImpl.java index f8f6044..b4ac326 100755 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -2497,8 +2497,14 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { pNetwork = _physicalNetworkDao.persist(pNetwork); // Add vnet entries for the new zone if zone type is Advanced + + List <String> vnets = new ArrayList<String>(); + for (Integer i= vnetStart; i<= vnetEnd; i++ ) { + vnets.add(i.toString()); + } + if (vnetRange != null) { - _dcDao.addVnet(zone.getId(), pNetwork.getId(), vnetStart, vnetEnd); + _dcDao.addVnet(zone.getId(), pNetwork.getId(), vnets); } // add VirtualRouter as the default network service provider @@ -2548,7 +2554,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { @Override @DB @ActionEvent(eventType = EventTypes.EVENT_PHYSICAL_NETWORK_UPDATE, eventDescription = "updating physical network", async = true) - public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state, String removeVlan) { + public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRange, String state, String removeVlan) { // verify input parameters PhysicalNetworkVO network = _physicalNetworkDao.findById(id); @@ -2565,7 +2571,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { ex.addProxyObject(String.valueOf(network.getDataCenterId()), "dataCenterId"); throw ex; } - if (newVnetRangeString != null) { + if (newVnetRange!= null) { if (zone.getNetworkType() == NetworkType.Basic || (zone.getNetworkType() == NetworkType.Advanced && zone.isSecurityGroupEnabled())) { throw new InvalidParameterValueException("Can't add vnet range to the physical network in the zone that supports " + zone.getNetworkType() + " network, Security Group enabled: " @@ -2575,7 +2581,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { if (removeVlan != null){ List<Integer> tokens = processVlanRange(network,removeVlan); - boolean result = removeVlanRange(network, tokens.get(0), tokens.get(1)); + removeVlanRange(network, tokens.get(0), tokens.get(1)); } if (tags != null && tags.size() > 1) { @@ -2607,121 +2613,63 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { boolean AddVnet = true; List<Pair<Integer, Integer>> vnetsToAdd = new ArrayList<Pair<Integer, Integer>>(); - if (newVnetRangeString != null) { - Integer newStartVnet = 0; - Integer newEndVnet = 0; - List<Integer> tokens = processVlanRange(network, newVnetRangeString); - newStartVnet = tokens.get(0); - newEndVnet = tokens.get(1); - Integer j=0; - List <Pair <Integer,Integer>> existingRanges = network.getVnet(); - if (!existingRanges.isEmpty()) { - for (; j < existingRanges.size(); j++){ - int existingStartVnet = existingRanges.get(j).first(); - int existingEndVnet = existingRanges.get(j).second(); - - // check if vnet is being extended - if (newStartVnet.intValue() >= existingStartVnet & newEndVnet.intValue() <= existingEndVnet) { - throw new InvalidParameterValueException("The vlan range you trying to add already exists."); - } - - if (newStartVnet < existingStartVnet & newEndVnet+1 >= existingStartVnet & newEndVnet <= existingEndVnet) { - vnetsToAdd.add(new Pair<Integer, Integer>(newStartVnet, existingStartVnet - 1)); - existingRanges.get(j).first(newStartVnet); - AddVnet = false; - break; + List<Integer> tokens = null; + List<String> add_Vnet = null; + if (newVnetRange != null) { + tokens = processVlanRange(network, newVnetRange); + HashSet<String> vnetsInDb = new HashSet<String>(); + vnetsInDb.addAll(_datacneter_vnet.listVnetsByPhysicalNetworkAndDataCenter(network.getDataCenterId(), id)); + HashSet<String> tempVnets = new HashSet<String>(); + tempVnets.addAll(vnetsInDb); + for (Integer i = tokens.get(0); i <= tokens.get(1); i++) { + tempVnets.add(i.toString()); + } + tempVnets.removeAll(vnetsInDb); + if (tempVnets.isEmpty()) { + throw new InvalidParameterValueException("The vlan range you are trying to add already exists."); + } + vnetsInDb.addAll(tempVnets); + add_Vnet = new ArrayList<String>(); + add_Vnet.addAll(tempVnets); + List<String> sortedList = new ArrayList<String>(vnetsInDb); + Collections.sort(sortedList, new Comparator<String>() { + public int compare(String s1, String s2) { + return Integer.valueOf(s1).compareTo(Integer.valueOf(s2)); } - - else if (newStartVnet > existingStartVnet & newStartVnet-1 <= existingEndVnet & newEndVnet >= existingEndVnet) { - vnetsToAdd.add(new Pair<Integer, Integer>(existingEndVnet + 1, newEndVnet)); - existingRanges.get(j).second(newEndVnet); - AddVnet = false; - break; - } - - else if (newStartVnet< existingStartVnet & newEndVnet > existingEndVnet){ - vnetsToAdd.add(new Pair<Integer, Integer>(newStartVnet,existingStartVnet-1)); - vnetsToAdd.add(new Pair<Integer, Integer>(existingEndVnet+1,newEndVnet)); - existingRanges.get(j).first(newStartVnet); - existingRanges.get(j).second(newEndVnet); - break; + }); + //build the vlan string form the allocated vlan list. + String vnetRange = ""; + String startvnet = sortedList.get(0); + String endvnet = ""; + for ( int i =0; i < sortedList.size()-1; i++ ) { + if (Integer.valueOf(sortedList.get(i+1)) - Integer.valueOf(sortedList.get(i)) > 1) { + endvnet = sortedList.get(i); + vnetRange=vnetRange + startvnet+"-"+endvnet+";"; + startvnet = sortedList.get(i+1); } } - + endvnet = sortedList.get(sortedList.size()-1); + vnetRange=vnetRange + startvnet+"-"+endvnet+";"; + vnetRange = vnetRange.substring(0,vnetRange.length()-1); + network.setVnet(vnetRange); } - if (AddVnet){ - vnetsToAdd.add(new Pair<Integer, Integer>(newStartVnet, newEndVnet)); - existingRanges.add(new Pair<Integer, Integer>(newStartVnet,newEndVnet)); - } - - Map <Integer,Integer> vnetMap = new HashMap<Integer, Integer>(existingRanges.size()); - Map <Integer, Integer> IndexMap = new HashMap<Integer, Integer>(existingRanges.size()); - for (int i=0; i< existingRanges.size(); i++){ - vnetMap.put(existingRanges.get(i).first(),existingRanges.get(i).second()); - IndexMap.put(existingRanges.get(i).first(),i); - } - - Integer value; - Integer index; - String vnetString = ""; - for (int i=0; i < existingRanges.size(); i++){ - value = vnetMap.get((existingRanges.get(i).second()+1)); - if (value != null) { - vnetMap.remove((existingRanges.get(i).second()+1)); - vnetMap.remove(existingRanges.get(i).first()); - vnetMap.put(existingRanges.get(i).first(),value); - existingRanges.add(new Pair<Integer,Integer>(existingRanges.get(i).first(),value)); - index = IndexMap.get(existingRanges.get(i).second()+1); - existingRanges.get(index).first(-1); - existingRanges.get(index).second(-1); - existingRanges.get(i).first(-1); - existingRanges.get(i).second(-1); - } - value = vnetMap.get((existingRanges.get(i).second())); - if (value != null && ( (existingRanges.get(i).second()) != (existingRanges.get(i).first()) )) { - vnetMap.remove((existingRanges.get(i).second())); - vnetMap.remove(existingRanges.get(i).first()); - vnetMap.put(existingRanges.get(i).first(),value); - existingRanges.add(new Pair<Integer,Integer>(existingRanges.get(i).first(),value)); - index = IndexMap.get(existingRanges.get(i).second()); - existingRanges.get(index).first(-1); - existingRanges.get(index).second(-1); - existingRanges.get(i).first(-1); - existingRanges.get(i).second(-1); - } - } - - - - if (newVnetRangeString != null) { - for (Pair<Integer,Integer> vnetRange : existingRanges ){ - value=vnetMap.get(vnetRange.first()); - if (value != null){ - vnetString = vnetString+vnetRange.first().toString()+"-"+value.toString()+";"; - } - } - if (vnetString.length() > 0 && vnetString.charAt(vnetString.length()-1)==';') { - vnetString = vnetString.substring(0, vnetString.length()-1); - } - network.setVnet(vnetString); - } - - for (Pair<Integer, Integer> vnetToAdd : vnetsToAdd) { - s_logger.debug("Adding vnet range " + vnetToAdd.first() + "-" + vnetToAdd.second() + " for the physicalNetwork id= " + id + " and zone id=" + network.getDataCenterId() + Transaction txn = Transaction.currentTxn(); + txn.start(); + if (add_Vnet != null) { + s_logger.debug("Adding vnet range " + tokens.get(0).toString() + "-" + tokens.get(1).toString() + " for the physicalNetwork id= " + id + " and zone id=" + network.getDataCenterId() + " as a part of updatePhysicalNetwork call"); - _dcDao.addVnet(network.getDataCenterId(), network.getId(), vnetToAdd.first(), vnetToAdd.second()); - } + _dcDao.addVnet(network.getDataCenterId(), network.getId(), add_Vnet); } - _physicalNetworkDao.update(id, network); + txn.commit(); return network; } - private List<Integer> processVlanRange(PhysicalNetworkVO network, String removeVlan) { + private List<Integer> processVlanRange(PhysicalNetworkVO network, String vlan) { Integer StartVnet; Integer EndVnet; - String[] VnetRange = removeVlan.split("-"); + String[] VnetRange = vlan.split("-"); // Init with [min,max] of VLAN. Actually 0x000 and 0xFFF are reserved by IEEE, shoudn't be used. long minVnet = MIN_VLAN_ID; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4fdb177b/server/test/com/cloud/network/UpdatePhysicalNetworkTest.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/network/UpdatePhysicalNetworkTest.java b/server/test/com/cloud/network/UpdatePhysicalNetworkTest.java index 25c023e..e3fc36a 100644 --- a/server/test/com/cloud/network/UpdatePhysicalNetworkTest.java +++ b/server/test/com/cloud/network/UpdatePhysicalNetworkTest.java @@ -18,6 +18,7 @@ package com.cloud.network; import com.cloud.capacity.CapacityManagerImpl; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.DataCenterVnetDao; import com.cloud.network.NetworkServiceImpl; import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkVO; @@ -40,16 +41,18 @@ import static org.mockito.Mockito.when; public class UpdatePhysicalNetworkTest { private PhysicalNetworkDao _physicalNetworkDao = mock(PhysicalNetworkDao.class); + private DataCenterVnetDao _DatacenterVnetDao = mock(DataCenterVnetDao.class); private DataCenterDao _datacenterDao = mock(DataCenterDao.class); private DataCenterVO datacentervo = mock(DataCenterVO.class); private PhysicalNetworkVO physicalNetworkVO = mock(PhysicalNetworkVO.class); - List<Pair<Integer,Integer>> existingRange = new ArrayList<Pair<Integer, Integer>>(); + List<String> existingRange = new ArrayList<String>(); ArgumentCaptor<String> argumentCaptor = ArgumentCaptor.forClass(String.class); public NetworkServiceImpl setUp() { NetworkServiceImpl networkService = new NetworkServiceImpl(); ((NetworkServiceImpl)networkService)._dcDao= _datacenterDao; networkService._physicalNetworkDao = _physicalNetworkDao; + networkService._datacneter_vnet = _DatacenterVnetDao; return networkService; } @@ -57,15 +60,15 @@ public class UpdatePhysicalNetworkTest { public void updatePhysicalNetworkTest(){ Transaction txn = Transaction.open("updatePhysicalNetworkTest"); NetworkServiceImpl networkService = setUp(); - existingRange.add(new Pair<Integer, Integer>(520, 524)); + existingRange.add("524"); when(_physicalNetworkDao.findById(anyLong())).thenReturn(physicalNetworkVO); when(_datacenterDao.findById(anyLong())).thenReturn(datacentervo); when(_physicalNetworkDao.update(anyLong(), any(physicalNetworkVO.getClass()))).thenReturn(true); - when(physicalNetworkVO.getVnet()).thenReturn(existingRange); + when(_DatacenterVnetDao.listVnetsByPhysicalNetworkAndDataCenter(anyLong(), anyLong())).thenReturn(existingRange); networkService.updatePhysicalNetwork(1l, null, null, "525-530", null, null); txn.close("updatePhysicalNetworkTest"); verify(physicalNetworkVO).setVnet(argumentCaptor.capture()); - assertEquals("520-530", argumentCaptor.getValue()); + assertEquals("524-530", argumentCaptor.getValue()); } }
