From bd488c4bba019435515e48075d4437fef5e01aec Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 7 Jan 2025 17:17:12 +0530 Subject: [PATCH] server, plugin: enhance storage stats for IOPS (#10034) Adds framework layer change to allow retrieving and storing IOPS stats for storage pools. Custom `PrimaryStoreDriver` can implement method - `getStorageIopsStats` for returning IOPS stats. Existing method `getUsedIops` can also be overridden by such plugins when only used IOPS is returned. For testing purpose, implementation has been added for simulator hypervisor plugin to return capacity and used IOPS for a pool. For local storage pool, implementation has been added using iostat to return currently used IOPS. StoragePoolResponse class has been updated to return IOPS values which allows showing IOPS values in UI for different storage pool related views and APIs. Signed-off-by: Abhishek Kumar --- .../java/com/cloud/storage/StorageStats.java | 3 + .../apache/cloudstack/api/ApiConstants.java | 1 + .../api/response/StoragePoolResponse.java | 12 ++ .../agent/api/GetStorageStatsAnswer.java | 36 ++++- .../agent/api/GetStorageStatsAnswerTest.java | 81 ++++++++++ debian/control | 2 +- .../api/storage/PrimaryDataStoreDriver.java | 8 + .../upgrade/dao/Upgrade41700to41710.java | 68 ++++++-- .../storage/datastore/db/StoragePoolVO.java | 11 ++ .../META-INF/db/schema-42000to42010.sql | 3 + .../db/views/cloud.storage_pool_view.sql | 2 + .../upgrade/dao/Upgrade41700to41710Test.java | 123 +++++++++++++++ packaging/el8/cloud.spec | 1 + .../LibvirtGetStorageStatsCommandWrapper.java | 3 +- .../kvm/storage/KVMStoragePool.java | 8 + .../kvm/storage/LibvirtStorageAdaptor.java | 69 ++++++-- .../kvm/storage/LibvirtStoragePool.java | 28 +++- .../storage/LibvirtStorageAdaptorTest.java | 94 ++++++++++- .../agent/manager/MockStorageManagerImpl.java | 8 +- .../cloud/simulator/dao/MockVolumeDao.java | 2 + .../simulator/dao/MockVolumeDaoImpl.java | 12 ++ .../api/query/dao/StoragePoolJoinDaoImpl.java | 9 +- .../cloud/api/query/vo/StoragePoolJoinVO.java | 14 ++ .../java/com/cloud/server/StatsCollector.java | 28 +++- .../com/cloud/storage/StorageManagerImpl.java | 23 ++- .../com/cloud/server/StatsCollectorTest.java | 148 ++++++++++++++---- .../cloud/storage/StorageManagerImplTest.java | 96 +++++++++--- ui/public/locales/en.json | 1 + .../config/section/infra/primaryStorages.js | 2 +- 29 files changed, 788 insertions(+), 108 deletions(-) create mode 100644 core/src/test/java/com/cloud/agent/api/GetStorageStatsAnswerTest.java create mode 100644 engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade41700to41710Test.java diff --git a/api/src/main/java/com/cloud/storage/StorageStats.java b/api/src/main/java/com/cloud/storage/StorageStats.java index a474b23489c..502e2aaae40 100644 --- a/api/src/main/java/com/cloud/storage/StorageStats.java +++ b/api/src/main/java/com/cloud/storage/StorageStats.java @@ -26,4 +26,7 @@ public interface StorageStats { * @return bytes capacity of the storage server */ public long getCapacityBytes(); + + Long getCapacityIops(); + Long getUsedIops(); } diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index bf8b79b29d0..cf03f1d2699 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -509,6 +509,7 @@ public class ApiConstants { public static final String URL = "url"; public static final String USAGE_INTERFACE = "usageinterface"; public static final String USED_SUBNETS = "usedsubnets"; + public static final String USED_IOPS = "usediops"; public static final String USER_DATA = "userdata"; public static final String USER_DATA_NAME = "userdataname"; diff --git a/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java index 06d5103d731..676803ea86b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java @@ -97,6 +97,10 @@ public class StoragePoolResponse extends BaseResponseWithAnnotations { @Param(description = "total min IOPS currently in use by volumes") private Long allocatedIops; + @SerializedName(ApiConstants.USED_IOPS) + @Param(description = "total IOPS currently in use", since = "4.20.1") + private Long usedIops; + @SerializedName(ApiConstants.STORAGE_CUSTOM_STATS) @Param(description = "the storage pool custom stats", since = "4.18.1") private Map customStats; @@ -312,6 +316,14 @@ public class StoragePoolResponse extends BaseResponseWithAnnotations { this.allocatedIops = allocatedIops; } + public Long getUsedIops() { + return usedIops; + } + + public void setUsedIops(Long usedIops) { + this.usedIops = usedIops; + } + public Map getCustomStats() { return customStats; } diff --git a/core/src/main/java/com/cloud/agent/api/GetStorageStatsAnswer.java b/core/src/main/java/com/cloud/agent/api/GetStorageStatsAnswer.java index 26e7b749586..79753661066 100644 --- a/core/src/main/java/com/cloud/agent/api/GetStorageStatsAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetStorageStatsAnswer.java @@ -27,24 +27,46 @@ public class GetStorageStatsAnswer extends Answer implements StorageStats { protected GetStorageStatsAnswer() { } - protected long used; + protected long usedBytes; - protected long capacity; + protected long capacityBytes; + + protected Long capacityIops; + + protected Long usedIops; @Override public long getByteUsed() { - return used; + return usedBytes; } @Override public long getCapacityBytes() { - return capacity; + return capacityBytes; } - public GetStorageStatsAnswer(GetStorageStatsCommand cmd, long capacity, long used) { + @Override + public Long getCapacityIops() { + return capacityIops; + } + + @Override + public Long getUsedIops() { + return usedIops; + } + + public GetStorageStatsAnswer(GetStorageStatsCommand cmd, long capacityBytes, long usedBytes) { super(cmd, true, null); - this.capacity = capacity; - this.used = used; + this.capacityBytes = capacityBytes; + this.usedBytes = usedBytes; + } + + public GetStorageStatsAnswer(GetStorageStatsCommand cmd, long capacityBytes, long usedBytes, Long capacityIops, Long usedIops) { + super(cmd, true, null); + this.capacityBytes = capacityBytes; + this.usedBytes = usedBytes; + this.capacityIops = capacityIops; + this.usedIops = usedIops; } public GetStorageStatsAnswer(GetStorageStatsCommand cmd, String details) { diff --git a/core/src/test/java/com/cloud/agent/api/GetStorageStatsAnswerTest.java b/core/src/test/java/com/cloud/agent/api/GetStorageStatsAnswerTest.java new file mode 100644 index 00000000000..44af83ada2d --- /dev/null +++ b/core/src/test/java/com/cloud/agent/api/GetStorageStatsAnswerTest.java @@ -0,0 +1,81 @@ +// 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.agent.api; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class GetStorageStatsAnswerTest { + + @Test + public void testDefaultConstructor() { + GetStorageStatsAnswer answer = new GetStorageStatsAnswer(); + + Assert.assertEquals(0, answer.getByteUsed()); + Assert.assertEquals(0, answer.getCapacityBytes()); + Assert.assertNull(answer.getCapacityIops()); + Assert.assertNull(answer.getUsedIops()); + } + + @Test + public void testConstructorWithCapacityAndUsedBytes() { + GetStorageStatsCommand mockCmd = new GetStorageStatsCommand(); + long capacityBytes = 1024L; + long usedBytes = 512L; + + GetStorageStatsAnswer answer = new GetStorageStatsAnswer(mockCmd, capacityBytes, usedBytes); + + Assert.assertEquals(capacityBytes, answer.getCapacityBytes()); + Assert.assertEquals(usedBytes, answer.getByteUsed()); + Assert.assertNull(answer.getCapacityIops()); + Assert.assertNull(answer.getUsedIops()); + } + + @Test + public void testConstructorWithIops() { + GetStorageStatsCommand mockCmd = new GetStorageStatsCommand(); + long capacityBytes = 2048L; + long usedBytes = 1024L; + Long capacityIops = 1000L; + Long usedIops = 500L; + + GetStorageStatsAnswer answer = new GetStorageStatsAnswer(mockCmd, capacityBytes, usedBytes, capacityIops, usedIops); + + Assert.assertEquals(capacityBytes, answer.getCapacityBytes()); + Assert.assertEquals(usedBytes, answer.getByteUsed()); + Assert.assertEquals(capacityIops, answer.getCapacityIops()); + Assert.assertEquals(usedIops, answer.getUsedIops()); + } + + @Test + public void testErrorConstructor() { + GetStorageStatsCommand mockCmd = new GetStorageStatsCommand(); + String errorDetails = "An error occurred"; + + GetStorageStatsAnswer answer = new GetStorageStatsAnswer(mockCmd, errorDetails); + + Assert.assertFalse(answer.getResult()); + Assert.assertEquals(errorDetails, answer.getDetails()); + Assert.assertEquals(0, answer.getCapacityBytes()); + Assert.assertEquals(0, answer.getByteUsed()); + Assert.assertNull(answer.getCapacityIops()); + Assert.assertNull(answer.getUsedIops()); + } +} diff --git a/debian/control b/debian/control index c0cb95af035..a773844c27c 100644 --- a/debian/control +++ b/debian/control @@ -24,7 +24,7 @@ Description: CloudStack server library Package: cloudstack-agent Architecture: all -Depends: ${python:Depends}, ${python3:Depends}, openjdk-17-jre-headless | java17-runtime-headless | java17-runtime | zulu-17, cloudstack-common (= ${source:Version}), lsb-base (>= 9), openssh-client, qemu-kvm (>= 2.5) | qemu-system-x86 (>= 5.2), libvirt-bin (>= 1.3) | libvirt-daemon-system (>= 3.0), iproute2, ebtables, vlan, ipset, python3-libvirt, ethtool, iptables, cryptsetup, rng-tools, rsync, lsb-release, ufw, apparmor, cpu-checker, libvirt-daemon-driver-storage-rbd +Depends: ${python:Depends}, ${python3:Depends}, openjdk-17-jre-headless | java17-runtime-headless | java17-runtime | zulu-17, cloudstack-common (= ${source:Version}), lsb-base (>= 9), openssh-client, qemu-kvm (>= 2.5) | qemu-system-x86 (>= 5.2), libvirt-bin (>= 1.3) | libvirt-daemon-system (>= 3.0), iproute2, ebtables, vlan, ipset, python3-libvirt, ethtool, iptables, cryptsetup, rng-tools, rsync, lsb-release, ufw, apparmor, cpu-checker, libvirt-daemon-driver-storage-rbd, sysstat Recommends: init-system-helpers Conflicts: cloud-agent, cloud-agent-libs, cloud-agent-deps, cloud-agent-scripts Description: CloudStack agent diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java index 2011b1f08fb..c8d9015af90 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java @@ -111,6 +111,14 @@ public interface PrimaryDataStoreDriver extends DataStoreDriver { */ Pair getStorageStats(StoragePool storagePool); + /** + * Intended for managed storage + * returns the capacity and used IOPS or null if not supported + */ + default Pair getStorageIopsStats(StoragePool storagePool) { + return null; + } + /** * intended for managed storage * returns true if the storage can provide the volume stats (physical and virtual size) diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41700to41710.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41700to41710.java index e3eb2bf514d..266401e0c31 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41700to41710.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41700to41710.java @@ -23,12 +23,16 @@ import java.util.List; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDaoImpl; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.commons.collections.CollectionUtils; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDaoImpl; import com.cloud.upgrade.SystemVmTemplateRegistration; +import com.cloud.utils.db.GenericSearchBuilder; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.exception.CloudRuntimeException; public class Upgrade41700to41710 extends DbUpgradeAbstractImpl implements DbUpgradeSystemVmTemplate { @@ -95,24 +99,58 @@ public class Upgrade41700to41710 extends DbUpgradeAbstractImpl implements DbUpgr } } - private void updateStorPoolStorageType() { - storageDao = new PrimaryDataStoreDaoImpl(); - List storPoolPools = storageDao.findPoolsByProvider("StorPool"); - for (StoragePoolVO storagePoolVO : storPoolPools) { - if (StoragePoolType.SharedMountPoint == storagePoolVO.getPoolType()) { - storagePoolVO.setPoolType(StoragePoolType.StorPool); - storageDao.update(storagePoolVO.getId(), storagePoolVO); - } - updateStorageTypeForStorPoolVolumes(storagePoolVO.getId()); + protected PrimaryDataStoreDao getStorageDao() { + if (storageDao == null) { + storageDao = new PrimaryDataStoreDaoImpl(); } + return storageDao; } - private void updateStorageTypeForStorPoolVolumes(long storagePoolId) { - volumeDao = new VolumeDaoImpl(); - List volumes = volumeDao.findByPoolId(storagePoolId, null); - for (VolumeVO volumeVO : volumes) { - volumeVO.setPoolType(StoragePoolType.StorPool); - volumeDao.update(volumeVO.getId(), volumeVO); + protected VolumeDao getVolumeDao() { + if (volumeDao == null) { + volumeDao = new VolumeDaoImpl(); } + return volumeDao; + } + + /* + GenericDao.customSearch using GenericSearchBuilder and GenericDao.update using + GenericDao.createSearchBuilder used here to prevent any future issues when new fields + are added to StoragePoolVO or VolumeVO and this upgrade path starts to fail. + */ + protected void updateStorPoolStorageType() { + StoragePoolVO pool = getStorageDao().createForUpdate(); + pool.setPoolType(StoragePoolType.StorPool); + SearchBuilder sb = getStorageDao().createSearchBuilder(); + sb.and("provider", sb.entity().getStorageProviderName(), SearchCriteria.Op.EQ); + sb.and("type", sb.entity().getPoolType(), SearchCriteria.Op.EQ); + sb.done(); + SearchCriteria sc = sb.create(); + sc.setParameters("provider", StoragePoolType.StorPool.name()); + sc.setParameters("type", StoragePoolType.SharedMountPoint.name()); + getStorageDao().update(pool, sc); + + GenericSearchBuilder gSb = getStorageDao().createSearchBuilder(Long.class); + gSb.selectFields(gSb.entity().getId()); + gSb.and("provider", gSb.entity().getStorageProviderName(), SearchCriteria.Op.EQ); + gSb.done(); + SearchCriteria gSc = gSb.create(); + gSc.setParameters("provider", StoragePoolType.StorPool.name()); + List poolIds = getStorageDao().customSearch(gSc, null); + updateStorageTypeForStorPoolVolumes(poolIds); + } + + protected void updateStorageTypeForStorPoolVolumes(List storagePoolIds) { + if (CollectionUtils.isEmpty(storagePoolIds)) { + return; + } + VolumeVO volume = getVolumeDao().createForUpdate(); + volume.setPoolType(StoragePoolType.StorPool); + SearchBuilder sb = getVolumeDao().createSearchBuilder(); + sb.and("poolId", sb.entity().getPoolId(), SearchCriteria.Op.IN); + sb.done(); + SearchCriteria sc = sb.create(); + sc.setParameters("poolId", storagePoolIds.toArray()); + getVolumeDao().update(volume, sc); } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java index 92a444bd83f..c2f5d0a5d96 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java @@ -119,6 +119,9 @@ public class StoragePoolVO implements StoragePool { @Column(name = "capacity_iops", updatable = true, nullable = true) private Long capacityIops; + @Column(name = "used_iops", updatable = true, nullable = true) + private Long usedIops; + @Column(name = "hypervisor") @Convert(converter = HypervisorTypeConverter.class) private HypervisorType hypervisor; @@ -256,6 +259,14 @@ public class StoragePoolVO implements StoragePool { return capacityIops; } + public Long getUsedIops() { + return usedIops; + } + + public void setUsedIops(Long usedIops) { + this.usedIops = usedIops; + } + @Override public Long getClusterId() { return clusterId; diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42000to42010.sql b/engine/schema/src/main/resources/META-INF/db/schema-42000to42010.sql index aef99dd0c7f..8b70cce3404 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42000to42010.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42000to42010.sql @@ -32,3 +32,6 @@ CALL `cloud`.`IDEMPOTENT_ADD_FOREIGN_KEY`('cloud.mshost_peer', 'fk_mshost_peer__ -- Add last_id to the volumes table CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.volumes', 'last_id', 'bigint(20) unsigned DEFAULT NULL'); + +-- Add used_iops column to support IOPS data in storage stats +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.storage_pool', 'used_iops', 'bigint unsigned DEFAULT NULL COMMENT "IOPS currently in use for this storage pool" '); diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.storage_pool_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.storage_pool_view.sql index e6cc9458208..5d7585baa3b 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.storage_pool_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.storage_pool_view.sql @@ -31,7 +31,9 @@ SELECT `storage_pool`.`created` AS `created`, `storage_pool`.`removed` AS `removed`, `storage_pool`.`capacity_bytes` AS `capacity_bytes`, + `storage_pool`.`used_bytes` AS `used_bytes`, `storage_pool`.`capacity_iops` AS `capacity_iops`, + `storage_pool`.`used_iops` AS `used_iops`, `storage_pool`.`scope` AS `scope`, `storage_pool`.`hypervisor` AS `hypervisor`, `storage_pool`.`storage_provider_name` AS `storage_provider_name`, diff --git a/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade41700to41710Test.java b/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade41700to41710Test.java new file mode 100644 index 00000000000..ad7c0cede25 --- /dev/null +++ b/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade41700to41710Test.java @@ -0,0 +1,123 @@ +// 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.upgrade.dao; + +import java.util.Collections; +import java.util.List; + +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDaoImpl; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.storage.Storage; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.storage.dao.VolumeDaoImpl; +import com.cloud.utils.db.GenericSearchBuilder; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; + +@RunWith(MockitoJUnitRunner.class) +public class Upgrade41700to41710Test { + @Spy + Upgrade41700to41710 upgrade41700to41710; + + @Test + public void testGetStorageDao_FirstInvocationCreatesInstance() { + PrimaryDataStoreDao dao1 = upgrade41700to41710.getStorageDao(); + Assert.assertNotNull(dao1); + Assert.assertTrue(dao1 instanceof PrimaryDataStoreDaoImpl); + } + + @Test + public void testGetStorageDao_SubsequentInvocationReturnsSameInstance() { + PrimaryDataStoreDao dao1 = upgrade41700to41710.getStorageDao(); + PrimaryDataStoreDao dao2 = upgrade41700to41710.getStorageDao(); + Assert.assertSame(dao1, dao2); + } + + @Test + public void testGetVolumeDao_FirstInvocationCreatesInstance() { + VolumeDao dao1 = upgrade41700to41710.getVolumeDao(); + Assert.assertNotNull(dao1); + Assert.assertTrue(dao1 instanceof VolumeDaoImpl); + } + + @Test + public void testGetVolumeDao_SubsequentInvocationReturnsSameInstance() { + VolumeDao dao1 = upgrade41700to41710.getVolumeDao(); + VolumeDao dao2 = upgrade41700to41710.getVolumeDao(); + Assert.assertSame(dao1, dao2); + } + + @Test + public void testUpdateStorPoolStorageType_WithPoolIds() { + PrimaryDataStoreDao storageDao = Mockito.mock(PrimaryDataStoreDao.class); + Mockito.doReturn(storageDao).when(upgrade41700to41710).getStorageDao(); + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + SearchBuilder searchBuilder = Mockito.mock(SearchBuilder.class); + Mockito.when(storageDao.createSearchBuilder()).thenReturn(searchBuilder); + Mockito.when(searchBuilder.entity()).thenReturn(pool); + Mockito.when(searchBuilder.create()).thenReturn(Mockito.mock(SearchCriteria.class)); + GenericSearchBuilder gSb = Mockito.mock(GenericSearchBuilder.class); + Mockito.doReturn(gSb).when(storageDao).createSearchBuilder(Mockito.any()); + Mockito.when(gSb.create()).thenReturn(Mockito.mock(SearchCriteria.class)); + Mockito.when(gSb.entity()).thenReturn(pool); + Mockito.when(storageDao.createForUpdate()).thenReturn(pool); + Mockito.doNothing().when(upgrade41700to41710).updateStorageTypeForStorPoolVolumes(Mockito.any()); + + Mockito.when(storageDao.update(Mockito.any(StoragePoolVO.class), Mockito.any())).thenReturn(2); + Mockito.when(storageDao.customSearch(Mockito.any(), Mockito.any())).thenReturn(List.of(1L, 2L)); + upgrade41700to41710.updateStorPoolStorageType(); + Mockito.verify(storageDao, Mockito.times(1)).update(Mockito.any(StoragePoolVO.class), Mockito.any()); + Mockito.verify(upgrade41700to41710, Mockito.times(1)).updateStorageTypeForStorPoolVolumes(Mockito.any()); + } + + @Test + public void testUpdateStorageTypeForStorPoolVolumes_EmptyPoolIds() { + VolumeDao volumeDao = Mockito.mock(VolumeDao.class); + List storagePoolIds = Collections.emptyList(); + upgrade41700to41710.updateStorageTypeForStorPoolVolumes(storagePoolIds); + Mockito.verify(volumeDao, Mockito.never()).update(Mockito.any(VolumeVO.class), Mockito.any()); + } + + @Test + public void testUpdateStorageTypeForStorPoolVolumes_WithPoolIds() { + VolumeDao volumeDao = Mockito.mock(VolumeDao.class); + List storagePoolIds = List.of(1L, 2L, 3L); + VolumeVO volume = Mockito.mock(VolumeVO.class); + SearchBuilder searchBuilder = Mockito.mock(SearchBuilder.class); + SearchCriteria searchCriteria = Mockito.mock(SearchCriteria.class); + Mockito.when(volumeDao.createForUpdate()).thenReturn(volume); + Mockito.when(volumeDao.createSearchBuilder()).thenReturn(searchBuilder); + Mockito.when(searchBuilder.entity()).thenReturn(volume); + Mockito.when(searchBuilder.create()).thenReturn(searchCriteria); + Mockito.when(volumeDao.update(Mockito.any(VolumeVO.class), Mockito.any())).thenReturn(3); + Mockito.doReturn(volumeDao).when(upgrade41700to41710).getVolumeDao(); + upgrade41700to41710.updateStorageTypeForStorPoolVolumes(storagePoolIds); + Mockito.verify(volumeDao).createForUpdate(); + Mockito.verify(volume).setPoolType(Storage.StoragePoolType.StorPool); + Mockito.verify(volumeDao).update(Mockito.eq(volume), Mockito.eq(searchCriteria)); + Mockito.verify(searchCriteria).setParameters("poolId", storagePoolIds.toArray()); + } +} diff --git a/packaging/el8/cloud.spec b/packaging/el8/cloud.spec index eb03cfe0df4..e34778820cb 100644 --- a/packaging/el8/cloud.spec +++ b/packaging/el8/cloud.spec @@ -118,6 +118,7 @@ Requires: cryptsetup Requires: rng-tools Requires: (libgcrypt > 1.8.3 or libgcrypt20) Requires: (selinux-tools if qemu-tools) +Requires: sysstat Provides: cloud-agent Group: System Environment/Libraries %description agent diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java index d00f5b540e2..419b5449258 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java @@ -40,7 +40,8 @@ public final class LibvirtGetStorageStatsCommandWrapper extends CommandWrapper commands = new ArrayList<>(); + commands.add(new String[]{ + Script.getExecutableAbsolutePath("bash"), + "-c", + String.format( + "%s %s | %s 'NR==2 {print $1}'", + Script.getExecutableAbsolutePath("df"), + pool.getLocalPath(), + Script.getExecutableAbsolutePath("awk") + ) + }); + String result = Script.executePipedCommands(commands, 1000).second(); + if (StringUtils.isBlank(result)) { + return; + } + result = result.trim(); + commands.add(new String[]{ + Script.getExecutableAbsolutePath("bash"), + "-c", + String.format( + "%s -z %s 1 2 | %s 'NR==7 {print $2}'", + Script.getExecutableAbsolutePath("iostat"), + result, + Script.getExecutableAbsolutePath("awk") + ) + }); + result = Script.executePipedCommands(commands, 10000).second(); + logger.trace("Pool used IOPS result: {}", result); + if (StringUtils.isBlank(result)) { + return; + } + try { + double doubleValue = Double.parseDouble(result); + pool.setUsedIops((long) doubleValue); + logger.debug("Updated used IOPS: {} for pool: {}", pool.getUsedIops(), pool.getName()); + } catch (NumberFormatException e) { + logger.warn(String.format("Unable to parse retrieved used IOPS: %s for pool: %s", result, + pool.getName())); + } + } + @Override public KVMStoragePool getStoragePool(String uuid, boolean refreshInfo) { logger.info("Trying to fetch storage pool " + uuid + " from libvirt"); @@ -591,6 +639,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { } pool.setCapacity(storage.getInfo().capacity); pool.setUsed(storage.getInfo().allocation); + updateLocalPoolIops(pool); pool.setAvailable(storage.getInfo().available); logger.debug("Successfully refreshed pool " + uuid + diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java index 560020cad38..8e5af7c613d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java @@ -43,6 +43,8 @@ public class LibvirtStoragePool implements KVMStoragePool { protected String uuid; protected long capacity; protected long used; + protected Long capacityIops; + protected Long usedIops; protected long available; protected String name; protected String localPath; @@ -81,20 +83,38 @@ public class LibvirtStoragePool implements KVMStoragePool { this.used = used; } - public void setAvailable(long available) { - this.available = available; - } - @Override public long getUsed() { return this.used; } + @Override + public Long getCapacityIops() { + return capacityIops; + } + + public void setCapacityIops(Long capacityIops) { + this.capacityIops = capacityIops; + } + + @Override + public Long getUsedIops() { + return usedIops; + } + + public void setUsedIops(Long usedIops) { + this.usedIops = usedIops; + } + @Override public long getAvailable() { return this.available; } + public void setAvailable(long available) { + this.available = available; + } + public StoragePoolType getStoragePoolType() { return this.type; } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java index c2bbff7efb0..88346abd017 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java @@ -17,18 +17,22 @@ package com.cloud.hypervisor.kvm.storage; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; + import java.util.HashMap; import java.util.Map; import java.util.UUID; -import com.cloud.utils.exception.CloudRuntimeException; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.libvirt.Connect; import org.libvirt.StoragePool; -import org.libvirt.StoragePoolInfo; +import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; @@ -38,6 +42,9 @@ import org.mockito.junit.MockitoJUnitRunner; import com.cloud.hypervisor.kvm.resource.LibvirtConnection; import com.cloud.hypervisor.kvm.resource.LibvirtStoragePoolDef; import com.cloud.storage.Storage; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; @RunWith(MockitoJUnitRunner.class) public class LibvirtStorageAdaptorTest { @@ -46,6 +53,11 @@ public class LibvirtStorageAdaptorTest { private AutoCloseable closeable; + @Mock + LibvirtStoragePool mockPool; + + MockedStatic