Martin Mucha has uploaded a new change for review. Change subject: core: move allocated macs to related pool when its created. ......................................................................
core: move allocated macs to related pool when its created. If there's some data center in system without configured own mac pool then allocated macs are registered in global pool. When this data center configuration is changed and new pool is created for it, macs related to it have to leave global mac pool and move to newly created one. Change-Id: Ia935ce8e8f63b6849a4bd63768b2d45fec61c1c8 Bug-Url: https://bugzilla.redhat.com/1078844 Signed-off-by: Martin Mucha <mmu...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/DataCenterScope.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerOriginal.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerRanges.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerStrategy.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/macPoolManager/ScopedMacPoolManagerTest.java 5 files changed, 134 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/73/26973/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/DataCenterScope.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/DataCenterScope.java index 09cf956..6b9736d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/DataCenterScope.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/DataCenterScope.java @@ -73,7 +73,7 @@ final List<StoragePool> storagePools = DbFacade.getInstance().getStoragePoolDao().getAll(); for (StoragePool storagePool : storagePools) { final Guid storagePoolId = storagePool.getId(); - List<String> macsForDataCenter = DbFacade.getInstance().getVmNicDao().getAllMacsByDataCenter(storagePoolId); + List<String> macsForDataCenter = allMacsForDataCenter(storagePoolId); final String macPoolRanges = storagePool.getMacPoolRanges(); if (macPoolRanges != null) { @@ -82,6 +82,10 @@ fillPool(getPoolInternal(storagePoolId), macsForDataCenter); } + } + + private List<String> allMacsForDataCenter(Guid storagePoolId) { + return DbFacade.getInstance().getVmNicDao().getAllMacsByDataCenter(storagePoolId); } private void fillPool(MacPoolManagerStrategy pool, List<String> macsForDataCenter) { @@ -140,21 +144,48 @@ protected boolean modifyPoolInternal(Guid scopeGuid, String ranges) { final boolean scopeExist = scopedPools.containsKey(scopeGuid); if (!scopeExist) { - return createPool(scopeGuid, ranges); + return createsNewPoolUponModifyPoolRequest(scopeGuid, ranges); } final boolean poolDefinitionRemoved = rangesDefinitionIsEmpty(ranges); if (poolDefinitionRemoved) { - return removePool(scopeGuid); + final boolean removed = removePoolInternal(scopeGuid); + return removed; } + return modifyExistingPoolSettings(scopeGuid, ranges); + } + + private boolean modifyExistingPoolSettings(Guid scopeGuid, String ranges) { final MacPoolManagerStrategy currentPool = getPool(scopeGuid); - final MacPoolManagerStrategy newPool = createAndInitPool(ranges); - currentPool.copyContent(newPool); - scopedPools.put(scopeGuid, newPool); //replace currentPool + if (currentPool.getMacPoolRanges().equals(ranges)) { +// ranges definition did not change, do nothing. + return false; + } else { + final MacPoolManagerStrategy newPool = createAndInitPool(ranges); - return true; + currentPool.copyContent(newPool); + scopedPools.put(scopeGuid, newPool); //replace currentPool + + return true; + } + } + + private boolean createsNewPoolUponModifyPoolRequest(Guid scopeGuid, String ranges) { + final boolean modified = createPoolInternal(scopeGuid, ranges); + + final MacPoolManagerStrategy alteredPool = getPool(scopeGuid); + for (String mac : allMacsForDataCenter(scopeGuid)) { + if (defaultPool.isMacInUse(mac)) { + defaultPool.freeMac(mac); + } else { + log.warn("Mac address, which should be allocated in global pool is not used there."); + } + + alteredPool.addMac(mac); + } + return modified; } /** diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerOriginal.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerOriginal.java index 5b2b93b..5839aa7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerOriginal.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerOriginal.java @@ -65,6 +65,11 @@ } @Override + public String getMacPoolRanges() { + return ranges; + } + + @Override public void initialize() { lockObj.writeLock().lock(); try { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerRanges.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerRanges.java index e17c66f..22b9518 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerRanges.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerRanges.java @@ -35,6 +35,11 @@ } @Override + public String getMacPoolRanges() { + return rangesString; + } + + @Override public void initialize() { if (initialized) { //this should rather throw an exception, but that is rather complicated to handle in jUnit. diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerStrategy.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerStrategy.java index 77801f0..90fc6dc 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerStrategy.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macPoolManager/MacPoolManagerStrategy.java @@ -24,5 +24,8 @@ void copyContent(MacPoolManagerStrategy macPoolManagerStrategy); - + /** + * @return ranges definition for this pool. + */ + String getMacPoolRanges(); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/macPoolManager/ScopedMacPoolManagerTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/macPoolManager/ScopedMacPoolManagerTest.java index aa147f2..fd63465 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/macPoolManager/ScopedMacPoolManagerTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/macPoolManager/ScopedMacPoolManagerTest.java @@ -230,9 +230,89 @@ errorCollector.checkThat("provided mac should not be used in returned pool", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).getPool().isMacInUse(macAddress2), is(false)); errorCollector.checkThat("provided mac should be used in returned pool", ScopedMacPoolManager.scopeFor().vmNic(vmNic2).getPool().isMacInUse(macAddress2), is(true)); - errorCollector.checkThat("provided mac should be used in returned pool", ScopedMacPoolManager.defaultScope().isMacInUse(macAddress1), is(false)); - errorCollector.checkThat("provided mac should be used in returned pool", ScopedMacPoolManager.defaultScope().isMacInUse(macAddress2), is(false)); + errorCollector.checkThat("provided mac should not be used in returned pool", ScopedMacPoolManager.defaultScope().isMacInUse(macAddress1), is(false)); + errorCollector.checkThat("provided mac should not be used in returned pool", ScopedMacPoolManager.defaultScope().isMacInUse(macAddress2), is(false)); + } + @Test + //test whether allocated mac get moved to global pool when it's scoped pool is removed. + public void testAllocatedMacMigrationOnDataCenterScopeRemoval() throws Exception { + //mock data center + final StoragePool storagePool1 = new StoragePool(); + storagePool1.setMacPoolRanges("00:1a:4a:15:c0:00-00:1a:4a:15:c0:09"); + storagePool1.setId(Guid.newGuid()); + //mock first vm to be in first data center + final VM vm1 = new VM(); + vm1.setId(Guid.newGuid()); + vm1.setStoragePoolId(storagePool1.getId()); + + //mock first vmNic + final String macAddress1 = "00:1a:4a:15:c0:fe"; + final VmNic vmNic1 = new VmNic(); + vmNic1.setMacAddress(macAddress1); + vmNic1.setVmId(vm1.getId()); + + when(vmDaoMock.get(eq(vm1.getId()))).thenReturn(vm1); + + when(storagePoolDaoMock.getAll()).thenReturn(Arrays.asList(new StoragePool[] { storagePool1 })); + when(vmNicDaoMock.getAll()).thenReturn(Arrays.asList(new VmNic[] { vmNic1 })); + + //mock querying for existing mac addresses for given data center. + when(vmNicDaoMock.getAllMacsByDataCenter(eq(storagePool1.getId()))).thenReturn(Collections.singletonList(macAddress1)); + + ScopedMacPoolManager.initialize(); + + //assertions: macAddress1 is referenced from scoped pool, not from global one. + errorCollector.checkThat("scoped pool for this nic should exist", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).isScopeExist(), is(true)); + errorCollector.checkThat("default scope should not be returned for this vmNic", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).getPool(), CoreMatchers.not(equalTo(ScopedMacPoolManager.defaultScope()))); + errorCollector.checkThat("provided mac should be used in returned pool", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).getPool().isMacInUse(macAddress1), is(true)); + errorCollector.checkThat("provided mac should not be used in returned pool", ScopedMacPoolManager.defaultScope().isMacInUse(macAddress1), is(false)); + + ScopedMacPoolManager.scopeFor().vmNic(vmNic1).removePool(); + errorCollector.checkThat("scoped pool for this nic should not exist", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).isScopeExist(), is(false)); + errorCollector.checkThat("default scope should be returned for this vmNic", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).getPool(), equalTo(ScopedMacPoolManager.defaultScope())); + errorCollector.checkThat("provided mac should not be used in returned pool", ScopedMacPoolManager.defaultScope().isMacInUse(macAddress1), is(true)); + } + + @Test + //test whether allocated mac get moved to global pool when it's scoped pool is removed. + public void testAllocatedMacMigrationOnDataCenterScopeCreation() throws Exception { + //mock data center + final StoragePool storagePool1 = new StoragePool(); +// storagePool1.setMacPoolRanges("00:1a:4a:15:c0:00-00:1a:4a:15:c0:09"); + storagePool1.setId(Guid.newGuid()); + + //mock first vm to be in first data center + final VM vm1 = new VM(); + vm1.setId(Guid.newGuid()); + vm1.setStoragePoolId(storagePool1.getId()); + + //mock first vmNic + final String macAddress1 = "00:1a:4a:15:c0:fe"; + final VmNic vmNic1 = new VmNic(); + vmNic1.setMacAddress(macAddress1); + vmNic1.setVmId(vm1.getId()); + + when(vmDaoMock.get(eq(vm1.getId()))).thenReturn(vm1); + + when(storagePoolDaoMock.getAll()).thenReturn(Arrays.asList(new StoragePool[] { storagePool1 })); + when(vmNicDaoMock.getAll()).thenReturn(Arrays.asList(new VmNic[] { vmNic1 })); + + //mock querying for existing mac addresses for given data center. + when(vmNicDaoMock.getAllMacsByDataCenter(eq(storagePool1.getId()))).thenReturn(Collections.singletonList(macAddress1)); + + ScopedMacPoolManager.initialize(); + + //assertions: macAddress1 is referenced from scoped pool, not from global one. + errorCollector.checkThat("scoped pool for this nic should exist", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).isScopeExist(), is(false)); + errorCollector.checkThat("default scope should not be returned for this vmNic", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).getPool(), equalTo(ScopedMacPoolManager.defaultScope())); + errorCollector.checkThat("provided mac should not be used in returned pool", ScopedMacPoolManager.defaultScope().isMacInUse(macAddress1), is(true)); + + ScopedMacPoolManager.scopeFor().vmNic(vmNic1).modifyPool("00:1a:4a:15:c0:00-00:1a:4a:15:c0:09"); + errorCollector.checkThat("scoped pool for this nic should not exist", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).isScopeExist(), is(true)); + errorCollector.checkThat("default scope should be returned for this vmNic", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).getPool(), CoreMatchers.not(equalTo(ScopedMacPoolManager.defaultScope()))); + errorCollector.checkThat("provided mac should be used in returned pool", ScopedMacPoolManager.scopeFor().vmNic(vmNic1).getPool().isMacInUse(macAddress1), is(true)); + errorCollector.checkThat("provided mac should not be used in returned pool", ScopedMacPoolManager.defaultScope().isMacInUse(macAddress1), is(false)); } } -- To view, visit http://gerrit.ovirt.org/26973 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia935ce8e8f63b6849a4bd63768b2d45fec61c1c8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
