-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update storage usage / size in backup usage response #12539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.20
Are you sure you want to change the base?
Update storage usage / size in backup usage response #12539
Conversation
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12539 +/- ##
==========================================
Coverage 16.26% 16.26%
- Complexity 13418 13428 +10
==========================================
Files 5658 5660 +2
Lines 499494 499921 +427
Branches 60625 60698 +73
==========================================
+ Hits 81232 81320 +88
- Misses 409214 409529 +315
- Partials 9048 9072 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the backup usage response to include storage size information that was previously stored but not exposed through the API.
Changes:
- Removed unused
rawUsagevariable from BackupUsageParser - Modernized Double instantiation to use primitive casting in BackupUsageParser and VMSnapshotOnPrimaryParser
- Added size and virtualSize fields to backup usage API response
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java | Removed unused variable and modernized Double casting for better code quality |
| usage/src/main/java/com/cloud/usage/parser/VMSnapshotOnPrimaryParser.java | Modernized Double casting from new Double(usage) to (double) usage |
| server/src/main/java/com/cloud/api/ApiResponseHelper.java | Added setSize() and setVirtualSize() calls to populate backup storage metrics in usage response |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16596 |
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
abh1sar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to fix the print statement in createUsageResponse
} else if (usageRecord.getUsageType() == UsageTypes.BACKUP) {
resourceType = ResourceObjectType.Backup;
final StringBuilder builder = new StringBuilder();
builder.append("Backup usage of size ").append(usageRecord.getUsageDisplay());
if (vmInstance != null) {
resourceId = vmInstance.getId();
usageRecResponse.setResourceName(vmInstance.getInstanceName());
usageRecResponse.setUsageId(vmInstance.getUuid());
builder.append(" for VM ").append(vmInstance.getHostName())
.append(" (").append(vmInstance.getUuid()).append(")");
final BackupOffering backupOffering = backupOfferingDao.findByIdIncludingRemoved(usageRecord.getOfferingId());
if (backupOffering != null) {
builder.append(" and backup offering ").append(backupOffering.getName())
.append(" (").append(backupOffering.getUuid()).append(", user ad-hoc/scheduled backup allowed: ")
.append(backupOffering.isUserDrivenBackupAllowed()).append(")");
}
}
Is should say something like
Backup usage for VM .. and backup offering ... with size (usageRecord.getSize()
Currently it shows
Snapshot usage for reference
|
@blueorangutan package |
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16625 |
|
@blueorangutan test |
|
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Tested the fix for backup usage API response. The size and virtualsize fields are now correctly included in the listUsageRecords API response for usage type 28 (Backup), along with an updated description format showing both size and virtual size with human-readable formatting.
Test Environment
- CloudStack Version: 4.20.3.0-SNAPSHOT
- Backup Provider: Dummy plugin
- Usage aggregation interval: 15 minutes
Test Setup
- Created VM with backup offering assigned
- Created backup for the VM
- Waited for usage aggregation to process backup usage events
API Response Verification
{
"account": "admin",
"accountid": "61127bf6-fd41-11f0-98c7-1e00370002e0",
"description": "Backup usage for VM backup-test-vm (8367f058-03de-4d85-9250-2d4ac1ca5f66) and backup offering test (bcc29bb8-b727-4fc1-abbe-e3a3d7899d69, user ad-hoc/scheduled backup allowed: true) with size (1000 bytes) 1000 and with virtual size (100 bytes) 100",
"domain": "ROOT",
"domainid": "1db1a70d-fd41-11f0-98c7-1e00370002e0",
"domainpath": "/",
"enddate": "2026-01-30T12:16:59+0000",
"name": "i-2-3-VM",
"rawusage": "0.25",
"size": 1000,
"startdate": "2026-01-30T12:02:00+0000",
"tags": [],
"usage": "0.25 Hrs",
"usageid": "8367f058-03de-4d85-9250-2d4ac1ca5f66",
"usagetype": 28,
"virtualmachineid": "8367f058-03de-4d85-9250-2d4ac1ca5f66",
"virtualsize": 100,
"zoneid": "bdb1d48b-6c33-4484-882c-a0b61d4887c9"
}Results
| Test | Result |
|---|---|
size field in API response |
PASS - 1000 |
virtualsize field in API response |
PASS - 100 |
| Description includes size | PASS - "with size (1000 bytes) 1000" |
| Description includes virtual size | PASS - "and with virtual size (100 bytes) 100" |
rawusage still represents hours |
PASS - 0.25 Hrs |
Database Verification
mysql> SELECT id, size, virtual_size FROM cloud_usage.cloud_usage WHERE usage_type=28 ORDER BY id DESC LIMIT 2;
+----+------+--------------+
| id | size | virtual_size |
+----+------+--------------+
| 50 | 1000 | 100 |
| 41 | 0 | 0 |
+----+------+--------------+Test Result: PASS
|
@sureshanaparti can you check @abh1sar 's comments and we can merge this |
done @borisstoyanov |
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
@sureshanaparti can you please test, what are the displayed values for size and virtual size. If they are correct? |
|
[SF] Trillian test result (tid-15339)
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16649 |
|
@abh1sar , tested with the last changes. The API Response Verification
|
Description
This PR updates storage usage / size in backup usage response.
Fixes #12536
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?