server, plugin: enhance storage stats for IOPS (apache#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 <[email protected]>
ablecloud-team · Jan 10, 2025 · 9245e38 · 9245e38
1 parent 9cb5eb0
commit 9245e38
Show file tree

Hide file tree

Showing 29 changed files with 790 additions and 108 deletions.
diff --git 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
@@ -516,6 +516,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
@@ -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<String, String> customStats;
@@ -312,6 +316,14 @@ public void setAllocatedIops(Long allocatedIops) {
        this.allocatedIops = allocatedIops;
     }
 
+    public Long getUsedIops() {
+        return usedIops;
+    }
+
+    public void setUsedIops(Long usedIops) {
+        this.usedIops = usedIops;
+    }
+
     public Map<String, String> 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
@@ -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;
+    }
+
+    @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.capacityBytes = capacityBytes;
+        this.usedBytes = usedBytes;
     }
 
-    public GetStorageStatsAnswer(GetStorageStatsCommand cmd, long capacity, long used) {
+    public GetStorageStatsAnswer(GetStorageStatsCommand cmd, long capacityBytes, long usedBytes, Long capacityIops, Long usedIops) {
         super(cmd, true, null);
-        this.capacity = capacity;
-        this.used = used;
+        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
@@ -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
@@ -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/.../main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java b/.../main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java
@@ -111,6 +111,14 @@ default Map<String, String> getCustomStorageStats(StoragePool pool) {
      */
     Pair<Long, Long> getStorageStats(StoragePool storagePool);
 
+    /**
+     * Intended for managed storage
+     * returns the capacity and used IOPS or null if not supported
+     */
+    default Pair<Long, Long> 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
@@ -23,12 +23,16 @@
 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 void updateSystemVmTemplates(Connection conn) {
         }
     }
 
-    private void updateStorPoolStorageType() {
-        storageDao = new PrimaryDataStoreDaoImpl();
-        List<StoragePoolVO> 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<VolumeVO> 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<StoragePoolVO> sb = getStorageDao().createSearchBuilder();
+        sb.and("provider", sb.entity().getStorageProviderName(), SearchCriteria.Op.EQ);
+        sb.and("type", sb.entity().getPoolType(), SearchCriteria.Op.EQ);
+        sb.done();
+        SearchCriteria<StoragePoolVO> sc = sb.create();
+        sc.setParameters("provider", StoragePoolType.StorPool.name());
+        sc.setParameters("type", StoragePoolType.SharedMountPoint.name());
+        getStorageDao().update(pool, sc);
+
+        GenericSearchBuilder<StoragePoolVO, Long> gSb = getStorageDao().createSearchBuilder(Long.class);
+        gSb.selectFields(gSb.entity().getId());
+        gSb.and("provider", gSb.entity().getStorageProviderName(), SearchCriteria.Op.EQ);
+        gSb.done();
+        SearchCriteria<Long> gSc = gSb.create();
+        gSc.setParameters("provider", StoragePoolType.StorPool.name());
+        List<Long> poolIds = getStorageDao().customSearch(gSc, null);
+        updateStorageTypeForStorPoolVolumes(poolIds);
+    }
+
+    protected void updateStorageTypeForStorPoolVolumes(List<Long> storagePoolIds) {
+        if (CollectionUtils.isEmpty(storagePoolIds)) {
+            return;
+        }
+        VolumeVO volume = getVolumeDao().createForUpdate();
+        volume.setPoolType(StoragePoolType.StorPool);
+        SearchBuilder<VolumeVO> sb = getVolumeDao().createSearchBuilder();
+        sb.and("poolId", sb.entity().getPoolId(), SearchCriteria.Op.IN);
+        sb.done();
+        SearchCriteria<VolumeVO> 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
@@ -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;
@@ -277,6 +280,14 @@ public Long getCapacityIops() {
         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
@@ -33,3 +33,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
@@ -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`,