Skip to content

Commit f045990

Browse files
DaanHooglandDaanHoogland
authored
Merge branch 'main' into furtherUnmanagedFixes
2 parents 0c057af + 64f4480 commit f045990

11 files changed

Lines changed: 234 additions & 73 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,18 @@ public interface ClusterDrsAlgorithm extends Adapter {
6565
* the service offering for the virtual machine
6666
* @param destHost
6767
* the destination host for the virtual machine
68-
* @param hostCpuUsedMap
69-
* a map of host IDs to the amount of CPU used on each host
70-
* @param hostMemoryUsedMap
71-
* a map of host IDs to the amount of memory used on each host
68+
* @param hostCpuFreeMap
69+
* a map of host IDs to the amount of CPU free on each host
70+
* @param hostMemoryFreeMap
71+
* a map of host IDs to the amount of memory free on each host
7272
* @param requiresStorageMotion
7373
* whether storage motion is required for the virtual machine
7474
*
7575
* @return a ternary containing improvement, cost, benefit
7676
*/
7777
Ternary<Double, Double, Double> getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering,
78-
Host destHost, Map<Long, Long> hostCpuUsedMap,
79-
Map<Long, Long> hostMemoryUsedMap, Boolean requiresStorageMotion);
78+
Host destHost, Map<Long, Long> hostCpuFreeMap,
79+
Map<Long, Long> hostMemoryFreeMap, Boolean requiresStorageMotion);
8080

8181
/**
8282
* Calculates the imbalance of the cluster after a virtual machine migration.
@@ -87,30 +87,30 @@ Ternary<Double, Double, Double> getMetrics(long clusterId, VirtualMachine vm, Se
8787
* the virtual machine being migrated
8888
* @param destHost
8989
* the destination host for the virtual machine
90-
* @param hostCpuUsedMap
91-
* a map of host IDs to the amount of CPU used on each host
92-
* @param hostMemoryUsedMap
93-
* a map of host IDs to the amount of memory used on each host
90+
* @param hostCpuFreeMap
91+
* a map of host IDs to the amount of CPU free on each host
92+
* @param hostMemoryFreeMap
93+
* a map of host IDs to the amount of memory free on each host
9494
*
9595
* @return a pair containing the CPU and memory imbalance of the cluster after the migration
9696
*/
9797
default Pair<Double, Double> getImbalancePostMigration(ServiceOffering serviceOffering, VirtualMachine vm,
98-
Host destHost, Map<Long, Long> hostCpuUsedMap,
99-
Map<Long, Long> hostMemoryUsedMap) {
98+
Host destHost, Map<Long, Long> hostCpuFreeMap,
99+
Map<Long, Long> hostMemoryFreeMap) {
100100
List<Long> postCpuList = new ArrayList<>();
101101
List<Long> postMemoryList = new ArrayList<>();
102102
final int vmCpu = serviceOffering.getCpu() * serviceOffering.getSpeed();
103103
final long vmRam = serviceOffering.getRamSize() * 1024L * 1024L;
104104

105-
for (Long hostId : hostCpuUsedMap.keySet()) {
106-
long cpu = hostCpuUsedMap.get(hostId);
107-
long memory = hostMemoryUsedMap.get(hostId);
105+
for (Long hostId : hostCpuFreeMap.keySet()) {
106+
long cpu = hostCpuFreeMap.get(hostId);
107+
long memory = hostMemoryFreeMap.get(hostId);
108108
if (hostId == destHost.getId()) {
109-
postCpuList.add(cpu + vmCpu);
110-
postMemoryList.add(memory + vmRam);
111-
} else if (hostId.equals(vm.getHostId())) {
112109
postCpuList.add(cpu - vmCpu);
113110
postMemoryList.add(memory - vmRam);
111+
} else if (hostId.equals(vm.getHostId())) {
112+
postCpuList.add(cpu + vmCpu);
113+
postMemoryList.add(memory + vmRam);
114114
} else {
115115
postCpuList.add(cpu);
116116
postMemoryList.add(memory);

‎engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,9 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
8383
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 = ?";
8484
protected static final String SELECT_POOLSCOPE = "SELECT s.scope from storage_pool s, volumes v where s.id = v.pool_id and v.id = ?";
8585

86-
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 = ? "
87-
+ " AND pool.pod_id = ? AND pool.cluster_id = ? " + " GROUP BY pool.id ORDER BY 2 ASC ";
86+
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 = ? ";
87+
private static final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART2 = " GROUP BY pool.id ORDER BY 2 ASC ";
88+
8889
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 = ? "
8990
+ " AND pool.scope = 'ZONE' AND pool.status='Up' " + " GROUP BY pool.id ORDER BY 2 ASC ";
9091

@@ -612,24 +613,39 @@ public boolean updateState(com.cloud.storage.Volume.State currentState, Event ev
612613
public List<Long> listPoolIdsByVolumeCount(long dcId, Long podId, Long clusterId, long accountId) {
613614
TransactionLegacy txn = TransactionLegacy.currentTxn();
614615
PreparedStatement pstmt = null;
615-
List<Long> result = new ArrayList<Long>();
616+
List<Long> result = new ArrayList<>();
617+
StringBuilder sql = new StringBuilder(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART1);
616618
try {
617-
String sql = ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT;
618-
pstmt = txn.prepareAutoCloseStatement(sql);
619-
pstmt.setLong(1, accountId);
620-
pstmt.setLong(2, dcId);
621-
pstmt.setLong(3, podId);
622-
pstmt.setLong(4, clusterId);
619+
List<Long> resourceIdList = new ArrayList<>();
620+
resourceIdList.add(accountId);
621+
resourceIdList.add(dcId);
622+
623+
if (podId != null) {
624+
sql.append(" AND pool.pod_id = ?");
625+
resourceIdList.add(podId);
626+
}
627+
if (clusterId != null) {
628+
sql.append(" AND pool.cluster_id = ?");
629+
resourceIdList.add(clusterId);
630+
}
631+
sql.append(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART2);
632+
633+
pstmt = txn.prepareAutoCloseStatement(sql.toString());
634+
for (int i = 0; i < resourceIdList.size(); i++) {
635+
pstmt.setLong(i + 1, resourceIdList.get(i));
636+
}
623637

624638
ResultSet rs = pstmt.executeQuery();
625639
while (rs.next()) {
626640
result.add(rs.getLong(1));
627641
}
628642
return result;
629643
} catch (SQLException e) {
630-
throw new CloudRuntimeException("DB Exception on: " + ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT, e);
644+
s_logger.debug("DB Exception on: " + sql.toString(), e);
645+
throw new CloudRuntimeException(e);
631646
} catch (Throwable e) {
632-
throw new CloudRuntimeException("Caught: " + ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT, e);
647+
s_logger.debug("Caught: " + sql.toString(), e);
648+
throw new CloudRuntimeException(e);
633649
}
634650
}
635651

‎engine/schema/src/test/java/com/cloud/storage/dao/VolumeDaoImplTest.java‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.storage.dao;
18+
19+
import static org.mockito.ArgumentMatchers.anyInt;
20+
import static org.mockito.ArgumentMatchers.anyLong;
21+
import static org.mockito.ArgumentMatchers.startsWith;
22+
import static org.mockito.Mockito.times;
23+
import static org.mockito.Mockito.verify;
24+
import static org.mockito.Mockito.when;
25+
26+
import java.sql.PreparedStatement;
27+
import java.sql.ResultSet;
28+
import java.sql.SQLException;
29+
30+
import org.junit.AfterClass;
31+
import org.junit.BeforeClass;
32+
import org.junit.Test;
33+
import org.junit.runner.RunWith;
34+
import org.mockito.Mock;
35+
import org.mockito.MockedStatic;
36+
import org.mockito.Mockito;
37+
import org.mockito.junit.MockitoJUnitRunner;
38+
39+
import com.cloud.utils.db.TransactionLegacy;
40+
41+
@RunWith(MockitoJUnitRunner.class)
42+
public class VolumeDaoImplTest {
43+
@Mock
44+
private PreparedStatement preparedStatementMock;
45+
46+
@Mock
47+
private TransactionLegacy transactionMock;
48+
49+
private static MockedStatic<TransactionLegacy> mockedTransactionLegacy;
50+
51+
private final VolumeDaoImpl volumeDao = new VolumeDaoImpl();
52+
53+
@BeforeClass
54+
public static void init() {
55+
mockedTransactionLegacy = Mockito.mockStatic(TransactionLegacy.class);
56+
}
57+
58+
@AfterClass
59+
public static void close() {
60+
mockedTransactionLegacy.close();
61+
}
62+
63+
@Test
64+
public void testListPoolIdsByVolumeCount_with_cluster_details() throws SQLException {
65+
final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITH_CLUSTER =
66+
"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 ";
67+
final long dcId = 1, accountId = 1;
68+
final Long podId = 1L, clusterId = 1L;
69+
70+
when(TransactionLegacy.currentTxn()).thenReturn(transactionMock);
71+
when(transactionMock.prepareAutoCloseStatement(startsWith(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITH_CLUSTER))).thenReturn(preparedStatementMock);
72+
ResultSet rs = Mockito.mock(ResultSet.class);
73+
when(preparedStatementMock.executeQuery()).thenReturn(rs, rs);
74+
75+
volumeDao.listPoolIdsByVolumeCount(dcId, podId, clusterId, accountId);
76+
77+
verify(transactionMock, times(1)).prepareAutoCloseStatement(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITH_CLUSTER);
78+
verify(preparedStatementMock, times(1)).setLong(1, accountId);
79+
verify(preparedStatementMock, times(1)).setLong(2, dcId);
80+
verify(preparedStatementMock, times(1)).setLong(3, podId);
81+
verify(preparedStatementMock, times(1)).setLong(4, clusterId);
82+
verify(preparedStatementMock, times(4)).setLong(anyInt(), anyLong());
83+
verify(preparedStatementMock, times(1)).executeQuery();
84+
}
85+
86+
@Test
87+
public void testListPoolIdsByVolumeCount_without_cluster_details() throws SQLException {
88+
final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITHOUT_CLUSTER =
89+
"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 ";
90+
final long dcId = 1, accountId = 1;
91+
92+
when(TransactionLegacy.currentTxn()).thenReturn(transactionMock);
93+
when(transactionMock.prepareAutoCloseStatement(startsWith(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITHOUT_CLUSTER))).thenReturn(preparedStatementMock);
94+
ResultSet rs = Mockito.mock(ResultSet.class);
95+
when(preparedStatementMock.executeQuery()).thenReturn(rs, rs);
96+
97+
volumeDao.listPoolIdsByVolumeCount(dcId, null, null, accountId);
98+
99+
verify(transactionMock, times(1)).prepareAutoCloseStatement(ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_QUERY_WITHOUT_CLUSTER);
100+
verify(preparedStatementMock, times(1)).setLong(1, accountId);
101+
verify(preparedStatementMock, times(1)).setLong(2, dcId);
102+
verify(preparedStatementMock, times(2)).setLong(anyInt(), anyLong());
103+
verify(preparedStatementMock, times(1)).executeQuery();
104+
}
105+
}

‎engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java‎

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
package org.apache.cloudstack.storage.allocator;
1818

1919

20-
import com.cloud.deploy.DeploymentPlan;
21-
import com.cloud.deploy.DeploymentPlanner;
22-
import com.cloud.storage.Storage;
23-
import com.cloud.storage.StoragePool;
24-
import com.cloud.user.Account;
25-
import com.cloud.vm.DiskProfile;
26-
import com.cloud.vm.VirtualMachineProfile;
20+
import static org.mockito.Mockito.when;
21+
22+
import java.util.ArrayList;
23+
import java.util.HashSet;
24+
import java.util.List;
25+
import java.util.Set;
26+
2727
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
2828
import org.junit.After;
2929
import org.junit.Assert;
@@ -34,10 +34,14 @@
3434
import org.mockito.Mockito;
3535
import org.mockito.junit.MockitoJUnitRunner;
3636

37-
import java.util.ArrayList;
38-
import java.util.HashSet;
39-
import java.util.List;
40-
import java.util.Set;
37+
import com.cloud.deploy.DeploymentPlan;
38+
import com.cloud.deploy.DeploymentPlanner;
39+
import com.cloud.storage.Storage;
40+
import com.cloud.storage.StoragePool;
41+
import com.cloud.storage.dao.VolumeDao;
42+
import com.cloud.user.Account;
43+
import com.cloud.vm.DiskProfile;
44+
import com.cloud.vm.VirtualMachineProfile;
4145

4246
@RunWith(MockitoJUnitRunner.class)
4347
public class AbstractStoragePoolAllocatorTest {
@@ -51,6 +55,9 @@ public class AbstractStoragePoolAllocatorTest {
5155
Account account;
5256
private List<StoragePool> pools;
5357

58+
@Mock
59+
VolumeDao volumeDao;
60+
5461
@Before
5562
public void setUp() {
5663
pools = new ArrayList<>();
@@ -83,6 +90,29 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() {
8390
Mockito.verify(allocator, Mockito.times(0)).reorderRandomPools(pools);
8491
}
8592

93+
@Test
94+
public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
95+
allocator.allocationAlgorithm = "userdispersing";
96+
allocator.volumeDao = volumeDao;
97+
98+
when(plan.getDataCenterId()).thenReturn(1l);
99+
when(plan.getPodId()).thenReturn(1l);
100+
when(plan.getClusterId()).thenReturn(1l);
101+
when(account.getAccountId()).thenReturn(1l);
102+
List<Long> poolIds = new ArrayList<>();
103+
poolIds.add(1l);
104+
poolIds.add(9l);
105+
when(volumeDao.listPoolIdsByVolumeCount(1l,1l,1l,1l)).thenReturn(poolIds);
106+
107+
List<StoragePool> reorderedPools = allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
108+
Assert.assertEquals(poolIds.size(),reorderedPools.size());
109+
110+
Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByCapacity(plan, pools);
111+
Mockito.verify(allocator, Mockito.times(1)).reorderPoolsByNumberOfVolumes(plan, pools, account);
112+
Mockito.verify(allocator, Mockito.times(0)).reorderRandomPools(pools);
113+
Mockito.verify(volumeDao, Mockito.times(1)).listPoolIdsByVolumeCount(1l,1l,1l,1l);
114+
}
115+
86116
@Test
87117
public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() {
88118
allocator.allocationAlgorithm = "firstfitleastconsumed";

‎plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public class BalancedTest {
6868

6969
List<Long> cpuList, memoryList;
7070

71-
Map<Long, Long> hostCpuUsedMap, hostMemoryUsedMap;
71+
Map<Long, Long> hostCpuFreeMap, hostMemoryFreeMap;
7272

7373

7474
@Mock
@@ -105,13 +105,13 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException {
105105
cpuList = Arrays.asList(1L, 2L);
106106
memoryList = Arrays.asList(512L, 2048L);
107107

108-
hostCpuUsedMap = new HashMap<>();
109-
hostCpuUsedMap.put(1L, 1000L);
110-
hostCpuUsedMap.put(2L, 2000L);
108+
hostCpuFreeMap = new HashMap<>();
109+
hostCpuFreeMap.put(1L, 2000L);
110+
hostCpuFreeMap.put(2L, 1000L);
111111

112-
hostMemoryUsedMap = new HashMap<>();
113-
hostMemoryUsedMap.put(1L, 512L * 1024L * 1024L);
114-
hostMemoryUsedMap.put(2L, 2048L * 1024L * 1024L);
112+
hostMemoryFreeMap = new HashMap<>();
113+
hostMemoryFreeMap.put(1L, 2048L * 1024L * 1024L);
114+
hostMemoryFreeMap.put(2L, 512L * 1024L * 1024L);
115115
}
116116

117117
private void overrideDefaultConfigValue(final ConfigKey configKey, final String name,
@@ -191,7 +191,7 @@ public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldExce
191191
public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException {
192192
overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu");
193193
Ternary<Double, Double, Double> result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost,
194-
hostCpuUsedMap, hostMemoryUsedMap, false);
194+
hostCpuFreeMap, hostMemoryFreeMap, false);
195195
assertEquals(0.0, result.first(), 0.01);
196196
assertEquals(0.0, result.second(), 0.0);
197197
assertEquals(1.0, result.third(), 0.0);
@@ -205,7 +205,7 @@ public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessExcept
205205
public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException {
206206
overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory");
207207
Ternary<Double, Double, Double> result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost,
208-
hostCpuUsedMap, hostMemoryUsedMap, false);
208+
hostCpuFreeMap, hostMemoryFreeMap, false);
209209
assertEquals(0.4, result.first(), 0.01);
210210
assertEquals(0, result.second(), 0.0);
211211
assertEquals(1, result.third(), 0.0);
@@ -219,7 +219,7 @@ public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessExc
219219
public void getMetricsWithDefault() throws NoSuchFieldException, IllegalAccessException {
220220
overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "both");
221221
Ternary<Double, Double, Double> result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost,
222-
hostCpuUsedMap, hostMemoryUsedMap, false);
222+
hostCpuFreeMap, hostMemoryFreeMap, false);
223223
assertEquals(0.4, result.first(), 0.01);
224224
assertEquals(0, result.second(), 0.0);
225225
assertEquals(1, result.third(), 0.0);

0 commit comments

Comments
 (0)