diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java index bf556622463..a773a9502ce 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java @@ -83,8 +83,9 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol protected static final String SELECT_HYPERTYPE_FROM_ZONE_VOLUME = "SELECT s.hypervisor from volumes v, storage_pool s where v.pool_id = s.id and v.id = ?"; protected static final String SELECT_POOLSCOPE = "SELECT s.scope from storage_pool s, volumes v where s.id = v.pool_id and v.id = ?"; - private static final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT = "SELECT pool.id, SUM(IF(vol.state='Ready' AND vol.account_id = ?, 1, 0)) FROM `cloud`.`storage_pool` pool LEFT JOIN `cloud`.`volumes` vol ON pool.id = vol.pool_id WHERE pool.data_center_id = ? " - + " AND pool.pod_id = ? AND pool.cluster_id = ? " + " GROUP BY pool.id ORDER BY 2 ASC "; + private static final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART1 = "SELECT pool.id, SUM(IF(vol.state='Ready' AND vol.account_id = ?, 1, 0)) FROM `cloud`.`storage_pool` pool LEFT JOIN `cloud`.`volumes` vol ON pool.id = vol.pool_id WHERE pool.data_center_id = ? "; + private static final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART2 = " GROUP BY pool.id ORDER BY 2 ASC "; + private static final String ORDER_ZONE_WIDE_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT = "SELECT pool.id, SUM(IF(vol.state='Ready' AND vol.account_id = ?, 1, 0)) FROM `cloud`.`storage_pool` pool LEFT JOIN `cloud`.`volumes` vol ON pool.id = vol.pool_id WHERE pool.data_center_id = ? " + " AND pool.scope = 'ZONE' AND pool.status='Up' " + " GROUP BY pool.id ORDER BY 2 ASC "; @@ -612,14 +613,27 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol public List listPoolIdsByVolumeCount(long dcId, Long podId, Long clusterId, long accountId) { TransactionLegacy txn = TransactionLegacy.currentTxn(); PreparedStatement pstmt = null; - List result = new ArrayList(); + List result = new ArrayList<>(); + StringBuilder sql = new StringBuilder(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART1); try { - String sql = ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT; - pstmt = txn.prepareAutoCloseStatement(sql); - pstmt.setLong(1, accountId); - pstmt.setLong(2, dcId); - pstmt.setLong(3, podId); - pstmt.setLong(4, clusterId); + List resourceIdList = new ArrayList<>(); + resourceIdList.add(accountId); + resourceIdList.add(dcId); + + if (podId != null) { + sql.append(" AND pool.pod_id = ?"); + resourceIdList.add(podId); + } + if (clusterId != null) { + sql.append(" AND pool.cluster_id = ?"); + resourceIdList.add(clusterId); + } + sql.append(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART2); + + pstmt = txn.prepareAutoCloseStatement(sql.toString()); + for (int i = 0; i < resourceIdList.size(); i++) { + pstmt.setLong(i + 1, resourceIdList.get(i)); + } ResultSet rs = pstmt.executeQuery(); while (rs.next()) { @@ -627,9 +641,11 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol } return result; } catch (SQLException e) { - throw new CloudRuntimeException("DB Exception on: " + ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT, e); + s_logger.debug("DB Exception on: " + sql.toString(), e); + throw new CloudRuntimeException(e); } catch (Throwable e) { - throw new CloudRuntimeException("Caught: " + ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT, e); + s_logger.debug("Caught: " + sql.toString(), e); + throw new CloudRuntimeException(e); } } diff --git a/engine/schema/src/test/java/com/cloud/storage/dao/VolumeDaoImplTest.java b/engine/schema/src/test/java/com/cloud/storage/dao/VolumeDaoImplTest.java new file mode 100644 index 00000000000..7968ee4a375 --- /dev/null +++ b/engine/schema/src/test/java/com/cloud/storage/dao/VolumeDaoImplTest.java @@ -0,0 +1,105 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.storage.dao; + +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.startsWith; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.utils.db.TransactionLegacy; + +@RunWith(MockitoJUnitRunner.class) +public class VolumeDaoImplTest { + @Mock + private PreparedStatement preparedStatementMock; + + @Mock + private TransactionLegacy transactionMock; + + private static MockedStatic mockedTransactionLegacy; + + private final VolumeDaoImpl volumeDao = new VolumeDaoImpl(); + + @BeforeClass + public static void init() { + mockedTransactionLegacy = Mockito.mockStatic(TransactionLegacy.class); + } + + @AfterClass + public static void close() { + mockedTransactionLegacy.close(); + } + + @Test + public void testListPoolIdsByVolumeCount_with_cluster_details() throws SQLException { + final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITH_CLUSTER = + "SELECT pool.id, SUM(IF(vol.state='Ready' AND vol.account_id = ?, 1, 0)) FROM `cloud`.`storage_pool` pool LEFT JOIN `cloud`.`volumes` vol ON pool.id = vol.pool_id WHERE pool.data_center_id = ? AND pool.pod_id = ? AND pool.cluster_id = ? GROUP BY pool.id ORDER BY 2 ASC "; + final long dcId = 1, accountId = 1; + final Long podId = 1L, clusterId = 1L; + + when(TransactionLegacy.currentTxn()).thenReturn(transactionMock); + when(transactionMock.prepareAutoCloseStatement(startsWith(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITH_CLUSTER))).thenReturn(preparedStatementMock); + ResultSet rs = Mockito.mock(ResultSet.class); + when(preparedStatementMock.executeQuery()).thenReturn(rs, rs); + + volumeDao.listPoolIdsByVolumeCount(dcId, podId, clusterId, accountId); + + verify(transactionMock, times(1)).prepareAutoCloseStatement(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITH_CLUSTER); + verify(preparedStatementMock, times(1)).setLong(1, accountId); + verify(preparedStatementMock, times(1)).setLong(2, dcId); + verify(preparedStatementMock, times(1)).setLong(3, podId); + verify(preparedStatementMock, times(1)).setLong(4, clusterId); + verify(preparedStatementMock, times(4)).setLong(anyInt(), anyLong()); + verify(preparedStatementMock, times(1)).executeQuery(); + } + + @Test + public void testListPoolIdsByVolumeCount_without_cluster_details() throws SQLException { + final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITHOUT_CLUSTER = + "SELECT pool.id, SUM(IF(vol.state='Ready' AND vol.account_id = ?, 1, 0)) FROM `cloud`.`storage_pool` pool LEFT JOIN `cloud`.`volumes` vol ON pool.id = vol.pool_id WHERE pool.data_center_id = ? GROUP BY pool.id ORDER BY 2 ASC "; + final long dcId = 1, accountId = 1; + + when(TransactionLegacy.currentTxn()).thenReturn(transactionMock); + when(transactionMock.prepareAutoCloseStatement(startsWith(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITHOUT_CLUSTER))).thenReturn(preparedStatementMock); + ResultSet rs = Mockito.mock(ResultSet.class); + when(preparedStatementMock.executeQuery()).thenReturn(rs, rs); + + volumeDao.listPoolIdsByVolumeCount(dcId, null, null, accountId); + + verify(transactionMock, times(1)).prepareAutoCloseStatement(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITHOUT_CLUSTER); + verify(preparedStatementMock, times(1)).setLong(1, accountId); + verify(preparedStatementMock, times(1)).setLong(2, dcId); + verify(preparedStatementMock, times(2)).setLong(anyInt(), anyLong()); + verify(preparedStatementMock, times(1)).executeQuery(); + } +} diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java index 58b6fc99a59..466ae7db9bc 100644 --- a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java +++ b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java @@ -17,13 +17,13 @@ package org.apache.cloudstack.storage.allocator; -import com.cloud.deploy.DeploymentPlan; -import com.cloud.deploy.DeploymentPlanner; -import com.cloud.storage.Storage; -import com.cloud.storage.StoragePool; -import com.cloud.user.Account; -import com.cloud.vm.DiskProfile; -import com.cloud.vm.VirtualMachineProfile; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.After; import org.junit.Assert; @@ -34,10 +34,14 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import com.cloud.deploy.DeploymentPlan; +import com.cloud.deploy.DeploymentPlanner; +import com.cloud.storage.Storage; +import com.cloud.storage.StoragePool; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.user.Account; +import com.cloud.vm.DiskProfile; +import com.cloud.vm.VirtualMachineProfile; @RunWith(MockitoJUnitRunner.class) public class AbstractStoragePoolAllocatorTest { @@ -51,6 +55,9 @@ public class AbstractStoragePoolAllocatorTest { Account account; private List pools; + @Mock + VolumeDao volumeDao; + @Before public void setUp() { pools = new ArrayList<>(); @@ -83,6 +90,29 @@ public class AbstractStoragePoolAllocatorTest { Mockito.verify(allocator, Mockito.times(0)).reorderRandomPools(pools); } + @Test + public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() { + allocator.allocationAlgorithm = "userdispersing"; + allocator.volumeDao = volumeDao; + + when(plan.getDataCenterId()).thenReturn(1l); + when(plan.getPodId()).thenReturn(1l); + when(plan.getClusterId()).thenReturn(1l); + when(account.getAccountId()).thenReturn(1l); + List poolIds = new ArrayList<>(); + poolIds.add(1l); + poolIds.add(9l); + when(volumeDao.listPoolIdsByVolumeCount(1l,1l,1l,1l)).thenReturn(poolIds); + + List reorderedPools = allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account); + Assert.assertEquals(poolIds.size(),reorderedPools.size()); + + Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByCapacity(plan, pools); + Mockito.verify(allocator, Mockito.times(1)).reorderPoolsByNumberOfVolumes(plan, pools, account); + Mockito.verify(allocator, Mockito.times(0)).reorderRandomPools(pools); + Mockito.verify(volumeDao, Mockito.times(1)).listPoolIdsByVolumeCount(1l,1l,1l,1l); + } + @Test public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() { allocator.allocationAlgorithm = "firstfitleastconsumed";