diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java index d2f08260aa3..0fedf746fa6 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java @@ -20,7 +20,6 @@ package org.apache.cloudstack.storage.image.manager; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -180,28 +179,14 @@ public class ImageStoreProviderManagerImpl implements ImageStoreProviderManager, @Override public DataStore getImageStoreWithFreeCapacity(List imageStores) { - if (imageStores.size() > 1) { - imageStores.sort(new Comparator() { // Sort data stores based on free capacity - @Override - public int compare(DataStore store1, DataStore store2) { - return Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store1), - _statsCollector.imageStoreCurrentFreeCapacity(store2)); - } - }); - for (DataStore imageStore : imageStores) { - // Return image store if used percentage is less then threshold value i.e. 90%. - if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) { - return imageStore; - } - } - } else if (imageStores.size() == 1) { - if (_statsCollector.imageStoreHasEnoughCapacity(imageStores.get(0))) { - return imageStores.get(0); + imageStores.sort((store1, store2) -> Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store2), + _statsCollector.imageStoreCurrentFreeCapacity(store1))); + for (DataStore imageStore : imageStores) { + if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) { + return imageStore; } } - - // No store with space found - logger.error(String.format("Can't find an image storage in zone with less than %d usage", + logger.error(String.format("Could not find an image storage in zone with less than %d usage", Math.round(_statsCollector.getImageStoreCapacityThreshold() * 100))); return null; } @@ -209,23 +194,11 @@ public class ImageStoreProviderManagerImpl implements ImageStoreProviderManager, @Override public List orderImageStoresOnFreeCapacity(List imageStores) { List stores = new ArrayList<>(); - if (imageStores.size() > 1) { - imageStores.sort(new Comparator() { // Sort data stores based on free capacity - @Override - public int compare(DataStore store1, DataStore store2) { - return Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store1), - _statsCollector.imageStoreCurrentFreeCapacity(store2)); - } - }); - for (DataStore imageStore : imageStores) { - // Return image store if used percentage is less then threshold value i.e. 90%. - if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) { - stores.add(imageStore); - } - } - } else if (imageStores.size() == 1) { - if (_statsCollector.imageStoreHasEnoughCapacity(imageStores.get(0))) { - stores.add(imageStores.get(0)); + imageStores.sort((store1, store2) -> Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store2), + _statsCollector.imageStoreCurrentFreeCapacity(store1))); + for (DataStore imageStore : imageStores) { + if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) { + stores.add(imageStore); } } return stores; diff --git a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImplTest.java b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImplTest.java index c0462034790..72acd65931a 100644 --- a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImplTest.java +++ b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImplTest.java @@ -16,6 +16,9 @@ // under the License. package org.apache.cloudstack.storage.image.manager; +import com.cloud.server.StatsCollector; +import com.cloud.utils.Pair; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.junit.Assert; @@ -26,14 +29,22 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + @RunWith(MockitoJUnitRunner.class) public class ImageStoreProviderManagerImplTest { @Mock ImageStoreDao imageStoreDao; + @Mock + StatsCollector statsCollectorMock; + @InjectMocks ImageStoreProviderManagerImpl imageStoreProviderManager = new ImageStoreProviderManagerImpl(); + @Test public void testGetImageStoreZoneId() { final long storeId = 1L; @@ -44,4 +55,56 @@ public class ImageStoreProviderManagerImplTest { long value = imageStoreProviderManager.getImageStoreZoneId(storeId); Assert.assertEquals(zoneId, value); } + + private Pair, List> prepareUnorderedAndOrderedImageStoresForCapacityTests(boolean hasStoragesWithEnoughCapacity) { + DataStore store1 = Mockito.mock(DataStore.class); + Mockito.doReturn(100L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store1); + Mockito.doReturn(false).when(statsCollectorMock).imageStoreHasEnoughCapacity(store1); + DataStore store2 = Mockito.mock(DataStore.class); + Mockito.doReturn(200L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store2); + Mockito.doReturn(hasStoragesWithEnoughCapacity).when(statsCollectorMock).imageStoreHasEnoughCapacity(store2); + DataStore store3 = Mockito.mock(DataStore.class); + Mockito.doReturn(300L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store3); + Mockito.doReturn(hasStoragesWithEnoughCapacity).when(statsCollectorMock).imageStoreHasEnoughCapacity(store3); + DataStore store4 = Mockito.mock(DataStore.class); + Mockito.doReturn(400L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store4); + Mockito.doReturn(false).when(statsCollectorMock).imageStoreHasEnoughCapacity(store4); + + List unordered = Arrays.asList(store1, store2, store3, store4); + List orderedAndEnoughCapacity = new ArrayList<>(); + if (hasStoragesWithEnoughCapacity) { + orderedAndEnoughCapacity.add(store3); + orderedAndEnoughCapacity.add(store2); + } + + return new Pair<>(unordered, orderedAndEnoughCapacity); + } + + @Test + public void getImageStoreWithFreeCapacityTestImageStoresWithEnoughCapacityExistReturnsImageStoreWithMostFreeCapacity() { + Pair, List> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(true); + + DataStore result = imageStoreProviderManager.getImageStoreWithFreeCapacity(unorderedAndOrdered.first()); + + Assert.assertEquals(unorderedAndOrdered.second().get(0), result); + } + + @Test + public void getImageStoreWithFreeCapacityTestImageStoresWithEnoughCapacityDoNotExistReturnsNull() { + Pair, List> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(false); + + DataStore result = imageStoreProviderManager.getImageStoreWithFreeCapacity(unorderedAndOrdered.first()); + + Assert.assertNull(result); + } + + @Test + public void orderImageStoresOnFreeCapacityTestReturnsImageStoresOrderedFromMostToLeast() { + Pair, List> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(true); + + List result = imageStoreProviderManager.orderImageStoresOnFreeCapacity(unorderedAndOrdered.first()); + + Assert.assertEquals(unorderedAndOrdered.second(), result); + } + }