You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/08/18 11:46:32 UTC

[GitHub] [cloudstack] weizhouapache opened a new pull request #5259: usage: create backup usage record for vmId-offeringId pair

weizhouapache opened a new pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259


   ### Description
   
   when creat usage record for backup usages in a period, we need to check if vm offering is changed. if vm offering is changed, create a new cloud usage record, otherwise update existing usage record.
   
   This PR fixes #4982 
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [x] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-897310613


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-891600800


   added unit test for BackupUsageParser.java


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901842190


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: debian. SL-JID 934


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-889685766


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-899184509


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Pearl1594 commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909901632


   @weizhouapache cc @nvazquez @rhtyd While the new approach fixes the problem; it solves the issue the same way #5005 attempted to and we did have reservations with the approach as it changes the behavior i.e., the columns `usage_display` and `raw_usage` which initially stored the size of the backup, now would show the duration over which the backup offering is assigned to the VM - Please note the comments at the right of the table below - Could this lead to some ambiguity ; I am for representing the usage records in the way the PR does, but do we need to have some upgrade path in place to change already recorded values which have `usage_display` and` raw_usage` in GB?
   
   That said, as of fixing the issue - the solution LGTM
   ```
   select id, raw_usage, usage_display, vm_instance_id, offering_id, usage_id, start_date, end_date, virtual_size, size from cloud_usage where vm_instance_id = 3 and usage_type = 28;
   
   +-------+----------------------+---------------+----------------+-------------+----------+---------------------+---------------------+--------------+------+  
   | 18350 |                 1000 | 0.0000 GiB    |              3 |           1 |        3 | 2021-09-01 04:32:00 | 2021-09-01 04:33:00 |          100 | 1000 | <-- When old usage job was running
   | 18365 |                 1000 | 0.0000 GiB    |              3 |           1 |        3 | 2021-09-01 04:33:00 | 2021-09-01 04:34:00 |          100 | 1000 |
   | 18380 | 0.017889168113470078 | 0.017889 Hrs  |              3 |           1 |        3 | 2021-09-01 04:34:00 | 2021-09-01 04:35:04 |          100 | 1000 | <-- After the new code changes kicked in
   | 18395 |  0.01666666753590107 | 0.016667 Hrs  |              3 |           1 |        3 | 2021-09-01 04:35:04 | 2021-09-01 04:36:04 |          100 | 1000 |
   | 18410 |  0.01666666753590107 | 0.016667 Hrs  |              3 |           1 |        3 | 2021-09-01 04:36:04 | 2021-09-01 04:37:04 |          100 | 1000 |
   ....
   | 18594 | 0.016668887808918953 | 0.016669 Hrs  |              3 |           1 |        3 | 2021-09-01 04:48:04 | 2021-09-01 04:49:04 |          100 | 1000 |
   | 18609 | 0.004332222044467926 | 0.004332 Hrs  |              3 |           1 |        3 | 2021-09-01 04:49:04 | 2021-09-01 04:50:04 |          100 | 1000 | <-- VM unassigned from bkp offering
   | 18737 | 0.014833054505288601 | 0.014833 Hrs  |              3 |           1 |        3 | 2021-09-01 04:58:04 | 2021-09-01 04:59:04 |          100 | 1000 | <-- VM re-assinged to the same bkp offering
   | 18752 | 0.016666390001773834 | 0.016666 Hrs  |              3 |           1 |        3 | 2021-09-01 04:59:04 | 2021-09-01 05:00:04 |          100 | 1000 |
   ....
   | 18951 |  0.013779444620013237 | 0.013779 Hrs  |              3 |           1 |        3 | 2021-09-01 05:12:04 | 2021-09-01 05:13:04 |          100 | 1000 |
   | 18952 | 0.0017772221472114325 | 0.001777 Hrs  |              3 |           2 |        3 | 2021-09-01 05:12:04 | 2021-09-01 05:13:04 |            0 |    0 | <-- VM assigned to new bkp offering (ID:2)
   | 18967 |  0.016665834933519363 | 0.016666 Hrs  |              3 |           2 |        3 | 2021-09-01 05:13:04 | 2021-09-01 05:14:04 |          100 | 1000 |
   +-------+-----------------------+---------------+----------------+-------------+----------+---------------------+---------------------+--------------+------+
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r700951087



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -68,65 +66,33 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
             return true;
         }
 
-        final Map<Long, BackupInfo> vmUsageMap = new HashMap<>();
         for (final UsageBackupVO usageBackup : usageBackups) {
             final Long vmId = usageBackup.getVmId();
             final Long zoneId = usageBackup.getZoneId();
             final Long offeringId = usageBackup.getBackupOfferingId();
-            if (vmUsageMap.get(vmId) == null) {
-                vmUsageMap.put(vmId, new BackupUsageParser.BackupInfo(new Backup.Metric(0L, 0L), zoneId, vmId, offeringId));
+            Date createdDate = usageBackup.getCreated();
+            Date removedDate = usageBackup.getRemoved();
+            if (createdDate.before(startDate)) {
+                createdDate = startDate;
             }
-            final Backup.Metric metric = vmUsageMap.get(vmId).getMetric();
-            metric.setBackupSize(metric.getBackupSize() + usageBackup.getSize());
-            metric.setDataSize(metric.getDataSize() + usageBackup.getProtectedSize());
-        }
+            if (removedDate == null || removedDate.after(endDate)) {
+                removedDate = endDate;
+            }
+            final long duration = (removedDate.getTime() - createdDate.getTime()) + 1;
+            final float usage = duration / 1000f / 60f / 60f;
+            DecimalFormat dFormat = new DecimalFormat("#.######");

Review comment:
       Could this cause issue where localisation is not English?

##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -68,65 +66,33 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
             return true;
         }
 
-        final Map<Long, BackupInfo> vmUsageMap = new HashMap<>();
         for (final UsageBackupVO usageBackup : usageBackups) {
             final Long vmId = usageBackup.getVmId();
             final Long zoneId = usageBackup.getZoneId();
             final Long offeringId = usageBackup.getBackupOfferingId();
-            if (vmUsageMap.get(vmId) == null) {
-                vmUsageMap.put(vmId, new BackupUsageParser.BackupInfo(new Backup.Metric(0L, 0L), zoneId, vmId, offeringId));
+            Date createdDate = usageBackup.getCreated();
+            Date removedDate = usageBackup.getRemoved();
+            if (createdDate.before(startDate)) {
+                createdDate = startDate;
             }
-            final Backup.Metric metric = vmUsageMap.get(vmId).getMetric();
-            metric.setBackupSize(metric.getBackupSize() + usageBackup.getSize());
-            metric.setDataSize(metric.getDataSize() + usageBackup.getProtectedSize());
-        }
+            if (removedDate == null || removedDate.after(endDate)) {
+                removedDate = endDate;
+            }
+            final long duration = (removedDate.getTime() - createdDate.getTime()) + 1;
+            final float usage = duration / 1000f / 60f / 60f;
+            DecimalFormat dFormat = new DecimalFormat("#.######");
+            String usageDisplay = dFormat.format(usage);
 
-        for (final BackupInfo backupInfo : vmUsageMap.values()) {
-            final Long vmId = backupInfo.getVmId();
-            final Long zoneId = backupInfo.getZoneId();
-            final Long offeringId = backupInfo.getOfferingId();
-            final Double rawUsage = (double) backupInfo.getMetric().getBackupSize();
-            final Double sizeGib = rawUsage / (1024.0 * 1024.0 * 1024.0);
-            final String description = String.format("Backup usage VM ID: %d", vmId);
-            final String usageDisplay = String.format("%.4f GiB", sizeGib);
+            final Double rawUsage = (double) usageBackup.getSize();
+            final String description = String.format("Backup usage VM ID: %d, backup offering: %d", vmId, offeringId);
 
             final UsageVO usageRecord =
-                    new UsageVO(zoneId, account.getAccountId(), account.getDomainId(), description, usageDisplay,
-                            UsageTypes.BACKUP, rawUsage, vmId, null, offeringId, null, vmId,
-                            backupInfo.getMetric().getBackupSize(), backupInfo.getMetric().getDataSize(), startDate, endDate);
+                    new UsageVO(zoneId, account.getAccountId(), account.getDomainId(), description, usageDisplay + " Hrs",

Review comment:
       This may not make sense, effectively a backup takes up space and not time. For example, two instances of backups can have different sizes - can you check how we do it for snapshots and volumes. Effectively backups of a VM are like snapshots for a volume.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-911777542


   Thanks @weizhouapache @Pearl1594 for your agreement, let's wait for @olivierlemasle to share his test results then we'll merge this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909689050






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900779537


   <b>Trillian test result (tid-1682)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 31546 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1682-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Smoke tests completed. 77 look OK, 10 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 0.14 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 0.16 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 1.30 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.12 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r692881237



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -68,18 +68,7 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
             return true;
         }
 
-        final Map<Long, BackupInfo> vmUsageMap = new HashMap<>();
-        for (final UsageBackupVO usageBackup : usageBackups) {
-            final Long vmId = usageBackup.getVmId();
-            final Long zoneId = usageBackup.getZoneId();
-            final Long offeringId = usageBackup.getBackupOfferingId();
-            if (vmUsageMap.get(vmId) == null) {
-                vmUsageMap.put(vmId, new BackupUsageParser.BackupInfo(new Backup.Metric(0L, 0L), zoneId, vmId, offeringId));
-            }
-            final Backup.Metric metric = vmUsageMap.get(vmId).getMetric();
-            metric.setBackupSize(metric.getBackupSize() + usageBackup.getSize());
-            metric.setDataSize(metric.getDataSize() + usageBackup.getProtectedSize());
-        }
+        Map<String, BackupInfo> vmUsageMap = getVmUsageMap(usageBackups);

Review comment:
       @weizhouapache should we handle it by ensure everytime the offering is changed, a usage event is handled? How does other resources say VM handle this when their offerings are changed, maybe we should follow the same logic? (I've not checked, so maybe your approach is in-line with that)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900877565


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 903


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900513421


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909689050


   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-899184225


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909746584


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909118097


   Ping @olivierlemasle - can you review/test this? Thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r691848711



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -100,6 +89,32 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
         return true;
     }
 
+    protected static final Map<String, BackupInfo> getVmUsageMap(final List<UsageBackupVO> usageBackups) {
+        final Map<Long, Long> vmOffering = new HashMap<Long, Long>();
+        final Map<Long, Integer> vmOfferingIndex = new HashMap<Long, Integer>();
+        final Map<String, BackupInfo> vmUsageMap = new HashMap<>();
+        for (final UsageBackupVO usageBackup : usageBackups) {
+            final Long vmId = usageBackup.getVmId();
+            final Long zoneId = usageBackup.getZoneId();
+            final Long offeringId = usageBackup.getBackupOfferingId();
+            Integer index = vmOfferingIndex.get(vmId) == null ? -1 : vmOfferingIndex.get(vmId);
+            boolean isVmOfferingEqualsToPrevious = vmOffering.get(vmId) != null && vmOffering.get(vmId) == offeringId;

Review comment:
       maybe also change the name to `isVmOfferingEqualToPrevious`, without the 3rd person singular 's'. Also the name `vmOfferingEqualsToPrevious` is better.

##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -100,6 +89,32 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
         return true;
     }
 
+    protected static final Map<String, BackupInfo> getVmUsageMap(final List<UsageBackupVO> usageBackups) {
+        final Map<Long, Long> vmOffering = new HashMap<Long, Long>();
+        final Map<Long, Integer> vmOfferingIndex = new HashMap<Long, Integer>();
+        final Map<String, BackupInfo> vmUsageMap = new HashMap<>();
+        for (final UsageBackupVO usageBackup : usageBackups) {
+            final Long vmId = usageBackup.getVmId();
+            final Long zoneId = usageBackup.getZoneId();
+            final Long offeringId = usageBackup.getBackupOfferingId();
+            Integer index = vmOfferingIndex.get(vmId) == null ? -1 : vmOfferingIndex.get(vmId);
+            boolean isVmOfferingEqualsToPrevious = vmOffering.get(vmId) != null && vmOffering.get(vmId) == offeringId;

Review comment:
       ```suggestion
               boolean isVmOfferingEqualsToPrevious = offeringId.equals(vmOffering.get(vmId));
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900513872


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r700965716



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -68,65 +66,33 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
             return true;
         }
 
-        final Map<Long, BackupInfo> vmUsageMap = new HashMap<>();
         for (final UsageBackupVO usageBackup : usageBackups) {
             final Long vmId = usageBackup.getVmId();
             final Long zoneId = usageBackup.getZoneId();
             final Long offeringId = usageBackup.getBackupOfferingId();
-            if (vmUsageMap.get(vmId) == null) {
-                vmUsageMap.put(vmId, new BackupUsageParser.BackupInfo(new Backup.Metric(0L, 0L), zoneId, vmId, offeringId));
+            Date createdDate = usageBackup.getCreated();
+            Date removedDate = usageBackup.getRemoved();
+            if (createdDate.before(startDate)) {
+                createdDate = startDate;
             }
-            final Backup.Metric metric = vmUsageMap.get(vmId).getMetric();
-            metric.setBackupSize(metric.getBackupSize() + usageBackup.getSize());
-            metric.setDataSize(metric.getDataSize() + usageBackup.getProtectedSize());
-        }
+            if (removedDate == null || removedDate.after(endDate)) {
+                removedDate = endDate;
+            }
+            final long duration = (removedDate.getTime() - createdDate.getTime()) + 1;
+            final float usage = duration / 1000f / 60f / 60f;
+            DecimalFormat dFormat = new DecimalFormat("#.######");

Review comment:
       @rhtyd 
   good question ......
   the usage records are not displayed on UI so it is not an issue I think. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-911596222


   @Pearl1594 @borisstoyanov 
   does it LGTY ?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900861847


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901318746


   <b>Trillian test result (tid-1689)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36216 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1689-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 87 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd merged pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-911742604


   That would be great, thanks @olivierlemasle 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-891891495


   @nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901828106


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-892329871


   <b>Trillian test result (tid-1463)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 42356 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1463-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 84 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 964.84 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 1074.36 | test_privategw_acl.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 306.44 | test_routers_network_ops.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 479.73 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 430.10 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 477.75 | test_vpc_redundant.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900513421






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909780602


   <b>Trillian Build Failed (tid-1885)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909118097


   Ping @olivierlemasle - can you review/test this? Thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900884684


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-897691578


   <b>Trillian test result (tid-1614)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 37235 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1614-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 84 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 3613.80 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 78.41 | test_kubernetes_clusters.py
   test_01_secure_vm_migration | `Error` | 1.11 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 0.19 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 2.08 | test_vm_life_cycle.py
   test_04_nonsecured_to_secured_vm_migration | `Error` | 1.09 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 6.36 | test_vm_life_cycle.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 301.65 | test_hostha_kvm.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd closed pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900513872






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-910953234


   <b>Trillian test result (tid-1894)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 53847 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1894-kvm-centos7.zip
   Smoke tests completed. 77 look OK, 14 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_deploy_vm_start_failure | `Error` | 60.39 | test_deploy_vm.py
   test_deploy_vm_volume_creation_failure | `Error` | 59.48 | test_deploy_vm.py
   test_vm_ha | `Error` | 77.75 | test_vm_ha.py
   test_vm_sync | `Error` | 132.18 | test_vm_sync.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestDomainsServiceOfferings>:setup | `Error` | 3.39 | test_domain_service_offerings.py
   test_nic_secondaryip_add_remove | `Error` | 0.04 | test_multipleips_per_nic.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 0.14 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 0.14 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 1.29 | test_routers_network_ops.py
   test_06_download_detached_volume | `Error` | 186.40 | test_volumes.py
   ContextSuite context=TestVolumes>:teardown | `Error` | 262.24 | test_volumes.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.12 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestInternalLb>:setup | `Error` | 0.00 | test_internal_lb.py
   test_01_1_create_iso_with_checksum_sha1_negative | `Error` | 66.46 | test_iso.py
   test_01_create_iso_with_checksum_sha1 | `Error` | 65.33 | test_iso.py
   test_01_create_iso_with_checksum_sha1 | `Error` | 66.41 | test_iso.py
   test_02_1_create_iso_with_checksum_sha256_negative | `Error` | 66.43 | test_iso.py
   test_02_create_iso_with_checksum_sha256 | `Error` | 65.34 | test_iso.py
   test_02_create_iso_with_checksum_sha256 | `Error` | 66.41 | test_iso.py
   test_03_1_create_iso_with_checksum_md5_negative | `Error` | 66.41 | test_iso.py
   test_03_create_iso_with_checksum_md5 | `Error` | 65.40 | test_iso.py
   test_03_create_iso_with_checksum_md5 | `Error` | 66.48 | test_iso.py
   test_04_create_iso_with_no_checksum | `Error` | 65.39 | test_iso.py
   test_04_create_iso_with_no_checksum | `Error` | 66.48 | test_iso.py
   test_01_create_iso | `Failure` | 1511.50 | test_iso.py
   ContextSuite context=TestISO>:setup | `Error` | 3025.04 | test_iso.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache edited a comment on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache edited a comment on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909312514


   @Pearl1594 updated this pr.
   the records in cloud_usage looks like
   ```
   mysql> select raw_usage, usage_display, size, virtual_size, start_date, end_date, usage_id, description from cloud_usage where usage_type=28;
   +------------------------+---------------+------+--------------+---------------------+---------------------+----------+------------------------------------------------------------------+
   | raw_usage              | usage_display | size | virtual_size | start_date          | end_date            | usage_id | description                                                      |
   +------------------------+---------------+------+--------------+---------------------+---------------------+----------+------------------------------------------------------------------+
   
   ...
   |     0.0833333358168602 | 0.083333 Hrs  | 1000 |          100 | 2021-08-31 14:31:34 | 2021-08-31 14:36:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |    0.06708250194787979 | 0.067083 Hrs  | 1000 |          100 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |  0.0033336111810058355 | 0.003334 Hrs  |    0 |            0 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 2                       |
   |   0.009029166772961617 | 0.009029 Hrs  |    0 |            0 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |    0.08333361148834229 | 0.083334 Hrs  | 1000 |          100 | 2021-08-31 14:41:34 | 2021-08-31 14:46:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   ```
   
   main changes
   (1) display hours instead of size in raw_usage (see Option 1 in #4982)
   (2) create a record for each backup offering in a period (consider there are multiple backup offering if we remove/assign vm from/to backup offering).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-896466589


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-889736304


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r700962149



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -68,65 +66,33 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
             return true;
         }
 
-        final Map<Long, BackupInfo> vmUsageMap = new HashMap<>();
         for (final UsageBackupVO usageBackup : usageBackups) {
             final Long vmId = usageBackup.getVmId();
             final Long zoneId = usageBackup.getZoneId();
             final Long offeringId = usageBackup.getBackupOfferingId();
-            if (vmUsageMap.get(vmId) == null) {
-                vmUsageMap.put(vmId, new BackupUsageParser.BackupInfo(new Backup.Metric(0L, 0L), zoneId, vmId, offeringId));
+            Date createdDate = usageBackup.getCreated();
+            Date removedDate = usageBackup.getRemoved();
+            if (createdDate.before(startDate)) {
+                createdDate = startDate;
             }
-            final Backup.Metric metric = vmUsageMap.get(vmId).getMetric();
-            metric.setBackupSize(metric.getBackupSize() + usageBackup.getSize());
-            metric.setDataSize(metric.getDataSize() + usageBackup.getProtectedSize());
-        }
+            if (removedDate == null || removedDate.after(endDate)) {
+                removedDate = endDate;
+            }
+            final long duration = (removedDate.getTime() - createdDate.getTime()) + 1;
+            final float usage = duration / 1000f / 60f / 60f;
+            DecimalFormat dFormat = new DecimalFormat("#.######");
+            String usageDisplay = dFormat.format(usage);
 
-        for (final BackupInfo backupInfo : vmUsageMap.values()) {
-            final Long vmId = backupInfo.getVmId();
-            final Long zoneId = backupInfo.getZoneId();
-            final Long offeringId = backupInfo.getOfferingId();
-            final Double rawUsage = (double) backupInfo.getMetric().getBackupSize();
-            final Double sizeGib = rawUsage / (1024.0 * 1024.0 * 1024.0);
-            final String description = String.format("Backup usage VM ID: %d", vmId);
-            final String usageDisplay = String.format("%.4f GiB", sizeGib);
+            final Double rawUsage = (double) usageBackup.getSize();
+            final String description = String.format("Backup usage VM ID: %d, backup offering: %d", vmId, offeringId);
 
             final UsageVO usageRecord =
-                    new UsageVO(zoneId, account.getAccountId(), account.getDomainId(), description, usageDisplay,
-                            UsageTypes.BACKUP, rawUsage, vmId, null, offeringId, null, vmId,
-                            backupInfo.getMetric().getBackupSize(), backupInfo.getMetric().getDataSize(), startDate, endDate);
+                    new UsageVO(zoneId, account.getAccountId(), account.getDomainId(), description, usageDisplay + " Hrs",

Review comment:
       @rhtyd 
   I have checked the usage for snapshots and volumes - cloudstack displays running time for both. see
   
   https://github.com/apache/cloudstack/blob/main/usage/src/main/java/com/cloud/usage/parser/VolumeUsageParser.java#L150-L153
   
   https://github.com/apache/cloudstack/blob/main/usage/src/main/java/com/cloud/usage/parser/VMSnapshotUsageParser.java#L131-L134
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-897086725


   <b>Trillian test result (tid-1586)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 55943 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1586-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 68 look OK, 19 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_port_fwd_on_src_nat | `Failure` | 814.76 | test_network.py
   test_02_port_fwd_on_non_src_nat | `Failure` | 816.94 | test_network.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.17 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   ContextSuite context=TestDeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   test_01_secure_vm_migration | `Error` | 1.08 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 1.09 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 1.09 | test_vm_life_cycle.py
   test_04_nonsecured_to_secured_vm_migration | `Error` | 1.09 | test_vm_life_cycle.py
   ContextSuite context=TestVMLifeCycle>:setup | `Error` | 1.75 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.63 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 6.73 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 8.78 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 7.78 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 7.79 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 7.75 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Error` | 4.64 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Error` | 5.68 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 7.99 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 6.05 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 5.67 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 8.28 | test_vpc_vpn.py
   test_01_cancel_host_maintenace_with_no_migration_jobs | `Error` | 0.06 | test_host_maintenance.py
   test_02_cancel_host_maintenace_with_migration_jobs | `Error` | 0.04 | test_host_maintenance.py
   test_03_cancel_host_maintenace_with_migration_jobs_failure | `Error` | 0.04 | test_host_maintenance.py
   test_01_cancel_host_maintenance_ssh_enabled_agent_connected | `Error` | 0.01 | test_host_maintenance.py
   test_03_cancel_host_maintenance_ssh_disabled_agent_connected | `Error` | 0.01 | test_host_maintenance.py
   test_04_cancel_host_maintenance_ssh_disabled_agent_disconnected | `Error` | 0.01 | test_host_maintenance.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 300.75 | test_hostha_kvm.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909688172


   @blueorangutan package 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901839496


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900861374


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r700849081



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -68,18 +68,7 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
             return true;
         }
 
-        final Map<Long, BackupInfo> vmUsageMap = new HashMap<>();
-        for (final UsageBackupVO usageBackup : usageBackups) {
-            final Long vmId = usageBackup.getVmId();
-            final Long zoneId = usageBackup.getZoneId();
-            final Long offeringId = usageBackup.getBackupOfferingId();
-            if (vmUsageMap.get(vmId) == null) {
-                vmUsageMap.put(vmId, new BackupUsageParser.BackupInfo(new Backup.Metric(0L, 0L), zoneId, vmId, offeringId));
-            }
-            final Backup.Metric metric = vmUsageMap.get(vmId).getMetric();
-            metric.setBackupSize(metric.getBackupSize() + usageBackup.getSize());
-            metric.setDataSize(metric.getDataSize() + usageBackup.getProtectedSize());
-        }
+        Map<String, BackupInfo> vmUsageMap = getVmUsageMap(usageBackups);

Review comment:
       @rhtyd 
   updated this pr to display the duration of backup offering in the period (see Pearl's testing result).
   since you are the author of backup and restore framework, could you please review it ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-897324862


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 852


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] olivierlemasle commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-911600024


   Hi, I'll look at it today.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-902336175


   <b>Trillian test result (tid-1725)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33148 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1725-kvm-centos7.zip
   Smoke tests completed. 87 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Pearl1594 commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909901632


   @weizhouapache cc @nvazquez @rhtyd While the new approach fixes the problem; it solves the issue the same way #5005 attempted to and we did have reservations with the approach as it changes the behavior i.e., the columns `usage_display` and `raw_usage` which initially stored the size of the backup, now would show the duration over which the backup offering is assigned to the VM - Please note the comments at the right of the table below - Could this lead to some ambiguity ; I am for representing the usage records in the way the PR does, but do we need to have some upgrade path in place to change already recorded values which have `usage_display` and` raw_usage` in GB?
   
   That said, as of fixing the issue - the solution LGTM
   ```
   select id, raw_usage, usage_display, vm_instance_id, offering_id, usage_id, start_date, end_date, virtual_size, size from cloud_usage where vm_instance_id = 3 and usage_type = 28;
   
   +-------+----------------------+---------------+----------------+-------------+----------+---------------------+---------------------+--------------+------+  
   | 18350 |                 1000 | 0.0000 GiB    |              3 |           1 |        3 | 2021-09-01 04:32:00 | 2021-09-01 04:33:00 |          100 | 1000 | <-- When old usage job was running
   | 18365 |                 1000 | 0.0000 GiB    |              3 |           1 |        3 | 2021-09-01 04:33:00 | 2021-09-01 04:34:00 |          100 | 1000 |
   | 18380 | 0.017889168113470078 | 0.017889 Hrs  |              3 |           1 |        3 | 2021-09-01 04:34:00 | 2021-09-01 04:35:04 |          100 | 1000 | <-- After the new code changes kicked in
   | 18395 |  0.01666666753590107 | 0.016667 Hrs  |              3 |           1 |        3 | 2021-09-01 04:35:04 | 2021-09-01 04:36:04 |          100 | 1000 |
   | 18410 |  0.01666666753590107 | 0.016667 Hrs  |              3 |           1 |        3 | 2021-09-01 04:36:04 | 2021-09-01 04:37:04 |          100 | 1000 |
   ....
   | 18594 | 0.016668887808918953 | 0.016669 Hrs  |              3 |           1 |        3 | 2021-09-01 04:48:04 | 2021-09-01 04:49:04 |          100 | 1000 |
   | 18609 | 0.004332222044467926 | 0.004332 Hrs  |              3 |           1 |        3 | 2021-09-01 04:49:04 | 2021-09-01 04:50:04 |          100 | 1000 | <-- VM unassigned from bkp offering
   | 18737 | 0.014833054505288601 | 0.014833 Hrs  |              3 |           1 |        3 | 2021-09-01 04:58:04 | 2021-09-01 04:59:04 |          100 | 1000 | <-- VM re-assinged to the same bkp offering
   | 18752 | 0.016666390001773834 | 0.016666 Hrs  |              3 |           1 |        3 | 2021-09-01 04:59:04 | 2021-09-01 05:00:04 |          100 | 1000 |
   ....
   | 18951 |  0.013779444620013237 | 0.013779 Hrs  |              3 |           1 |        3 | 2021-09-01 05:12:04 | 2021-09-01 05:13:04 |          100 | 1000 |
   | 18952 | 0.0017772221472114325 | 0.001777 Hrs  |              3 |           2 |        3 | 2021-09-01 05:12:04 | 2021-09-01 05:13:04 |            0 |    0 | <-- VM assigned to new bkp offering (ID:2)
   | 18967 |  0.016665834933519363 | 0.016666 Hrs  |              3 |           2 |        3 | 2021-09-01 05:13:04 | 2021-09-01 05:14:04 |          100 | 1000 |
   +-------+-----------------------+---------------+----------------+-------------+----------+---------------------+---------------------+--------------+------+
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r692015774



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -100,6 +89,32 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
         return true;
     }
 
+    protected static final Map<String, BackupInfo> getVmUsageMap(final List<UsageBackupVO> usageBackups) {
+        final Map<Long, Long> vmOffering = new HashMap<Long, Long>();
+        final Map<Long, Integer> vmOfferingIndex = new HashMap<Long, Integer>();
+        final Map<String, BackupInfo> vmUsageMap = new HashMap<>();
+        for (final UsageBackupVO usageBackup : usageBackups) {
+            final Long vmId = usageBackup.getVmId();
+            final Long zoneId = usageBackup.getZoneId();
+            final Long offeringId = usageBackup.getBackupOfferingId();
+            Integer index = vmOfferingIndex.get(vmId) == null ? -1 : vmOfferingIndex.get(vmId);
+            boolean isVmOfferingEqualsToPrevious = vmOffering.get(vmId) != null && vmOffering.get(vmId) == offeringId;

Review comment:
       @DaanHoogland thanks. done!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909762573


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-897310883


   @nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-891890725


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909759310


   <b>Trillian Build Failed (tid-1881)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-910983833


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901839993


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901828166


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909709418


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 1082


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-902634338


   @Pearl1594 I think you're aware of the original issue, can you review this? cc @nvazquez @weizhouapache 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r690946101



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -100,6 +89,33 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
         return true;
     }
 
+    protected static final Map<String, BackupInfo> getVmUsageMap(final List<UsageBackupVO> usageBackups) {
+        final Map<Long, Long> vmOffering = new HashMap<Long, Long>();
+        final Map<Long, Integer> vmOfferingIndex = new HashMap<Long, Integer>();
+        final Map<String, BackupInfo> vmUsageMap = new HashMap<>();
+        for (final UsageBackupVO usageBackup : usageBackups) {
+            final Long vmId = usageBackup.getVmId();
+            final Long zoneId = usageBackup.getZoneId();
+            final Long offeringId = usageBackup.getBackupOfferingId();
+            Integer index = vmOfferingIndex.get(vmId) == null ? 0 : vmOfferingIndex.get(vmId);
+            if (vmOffering.get(vmId) == null || vmOffering.get(vmId) != offeringId)  {
+                if (vmOffering.get(vmId) != null) {

Review comment:
       @GutoVeronezi  done.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-900885251


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-899642179


   <b>Trillian test result (tid-1647)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 45331 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1647-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Smoke tests completed. 85 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_05_rvpc_multi_tiers | `Failure` | 535.39 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 569.51 | test_vpc_redundant.py
   test_02_VPC_default_routes | `Failure` | 939.29 | test_vpc_router_nics.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909746584


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-891923366


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909312514






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-912340252


   Thanks @olivierlemasle merging this based on your feedback, as well comments from Pearl and Wei. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909762776


   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-897325530


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909118097






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901959865


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-889686332


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-896466798


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909312514


   @Pearl1594 updated this pr.
   the records in cloud_usage looks like
   ```
   mysql> select raw_usage, usage_display, size, virtual_size, start_date, end_date, usage_id, description from cloud_usage where usage_type=28;
   +------------------------+---------------+------+--------------+---------------------+---------------------+----------+------------------------------------------------------------------+
   | raw_usage              | usage_display | size | virtual_size | start_date          | end_date            | usage_id | description                                                      |
   +------------------------+---------------+------+--------------+---------------------+---------------------+----------+------------------------------------------------------------------+
   
   ...
   |     0.0833333358168602 | 0.083333 Hrs  | 1000 |          100 | 2021-08-31 14:31:34 | 2021-08-31 14:36:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |    0.06708250194787979 | 0.067083 Hrs  | 1000 |          100 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |  0.0033336111810058355 | 0.003334 Hrs  |    0 |            0 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 2                       |
   |   0.009029166772961617 | 0.009029 Hrs  |    0 |            0 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |    0.08333361148834229 | 0.083334 Hrs  | 1000 |          100 | 2021-08-31 14:41:34 | 2021-08-31 14:46:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   ```
   
   main changes
   (1) display hours instead of size in raw_usage
   (2) create a record for each backup offering in a period (consider there are multiple backup offering if we remove/assign vm from/to backup offering).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-911528908


   <b>Trillian test result (tid-1922)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33050 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1922-kvm-centos7.zip
   Smoke tests completed. 87 look OK, 0 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909758095


   <b>Trillian Build Failed (tid-1880)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r690946101



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -100,6 +89,33 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
         return true;
     }
 
+    protected static final Map<String, BackupInfo> getVmUsageMap(final List<UsageBackupVO> usageBackups) {
+        final Map<Long, Long> vmOffering = new HashMap<Long, Long>();
+        final Map<Long, Integer> vmOfferingIndex = new HashMap<Long, Integer>();
+        final Map<String, BackupInfo> vmUsageMap = new HashMap<>();
+        for (final UsageBackupVO usageBackup : usageBackups) {
+            final Long vmId = usageBackup.getVmId();
+            final Long zoneId = usageBackup.getZoneId();
+            final Long offeringId = usageBackup.getBackupOfferingId();
+            Integer index = vmOfferingIndex.get(vmId) == null ? 0 : vmOfferingIndex.get(vmId);
+            if (vmOffering.get(vmId) == null || vmOffering.get(vmId) != offeringId)  {
+                if (vmOffering.get(vmId) != null) {

Review comment:
       @GutoVeronezi  done.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909748001


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-910983290


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-897325459


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache edited a comment on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache edited a comment on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-909312514


   @Pearl1594 updated this pr.
   the records in cloud_usage looks like
   ```
   mysql> select raw_usage, usage_display, size, virtual_size, start_date, end_date, usage_id, description from cloud_usage where usage_type=28;
   +------------------------+---------------+------+--------------+---------------------+---------------------+----------+------------------------------------------------------------------+
   | raw_usage              | usage_display | size | virtual_size | start_date          | end_date            | usage_id | description                                                      |
   +------------------------+---------------+------+--------------+---------------------+---------------------+----------+------------------------------------------------------------------+
   
   ...
   |     0.0833333358168602 | 0.083333 Hrs  | 1000 |          100 | 2021-08-31 14:31:34 | 2021-08-31 14:36:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |    0.06708250194787979 | 0.067083 Hrs  | 1000 |          100 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |  0.0033336111810058355 | 0.003334 Hrs  |    0 |            0 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 2                       |
   |   0.009029166772961617 | 0.009029 Hrs  |    0 |            0 | 2021-08-31 14:36:34 | 2021-08-31 14:41:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   |    0.08333361148834229 | 0.083334 Hrs  | 1000 |          100 | 2021-08-31 14:41:34 | 2021-08-31 14:46:34 |       68 | Backup usage VM ID: 68, backup offering: 1                       |
   ```
   
   main changes
   (1) display hours instead of size in raw_usage (see Option 1 in #4982)
   (2) create a record for each backup offering in a period (consider there are multiple backup offering if we remove/assign vm from/to backup offering).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r700965716



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -68,65 +66,33 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
             return true;
         }
 
-        final Map<Long, BackupInfo> vmUsageMap = new HashMap<>();
         for (final UsageBackupVO usageBackup : usageBackups) {
             final Long vmId = usageBackup.getVmId();
             final Long zoneId = usageBackup.getZoneId();
             final Long offeringId = usageBackup.getBackupOfferingId();
-            if (vmUsageMap.get(vmId) == null) {
-                vmUsageMap.put(vmId, new BackupUsageParser.BackupInfo(new Backup.Metric(0L, 0L), zoneId, vmId, offeringId));
+            Date createdDate = usageBackup.getCreated();
+            Date removedDate = usageBackup.getRemoved();
+            if (createdDate.before(startDate)) {
+                createdDate = startDate;
             }
-            final Backup.Metric metric = vmUsageMap.get(vmId).getMetric();
-            metric.setBackupSize(metric.getBackupSize() + usageBackup.getSize());
-            metric.setDataSize(metric.getDataSize() + usageBackup.getProtectedSize());
-        }
+            if (removedDate == null || removedDate.after(endDate)) {
+                removedDate = endDate;
+            }
+            final long duration = (removedDate.getTime() - createdDate.getTime()) + 1;
+            final float usage = duration / 1000f / 60f / 60f;
+            DecimalFormat dFormat = new DecimalFormat("#.######");

Review comment:
       @rhtyd 
   good question ......
   if it is a bug, we need to change all other usage parsers which use the same code. we have not received bug report about it until now.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-889703504


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 707


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901959620


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-889736828


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-891913955


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 738


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-901864785


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 935


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#discussion_r689593436



##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -100,6 +89,33 @@ public static boolean parse(AccountVO account, Date startDate, Date endDate) {
         return true;
     }
 
+    protected static final Map<String, BackupInfo> getVmUsageMap(final List<UsageBackupVO> usageBackups) {
+        final Map<Long, Long> vmOffering = new HashMap<Long, Long>();
+        final Map<Long, Integer> vmOfferingIndex = new HashMap<Long, Integer>();
+        final Map<String, BackupInfo> vmUsageMap = new HashMap<>();
+        for (final UsageBackupVO usageBackup : usageBackups) {
+            final Long vmId = usageBackup.getVmId();
+            final Long zoneId = usageBackup.getZoneId();
+            final Long offeringId = usageBackup.getBackupOfferingId();
+            Integer index = vmOfferingIndex.get(vmId) == null ? 0 : vmOfferingIndex.get(vmId);
+            if (vmOffering.get(vmId) == null || vmOffering.get(vmId) != offeringId)  {
+                if (vmOffering.get(vmId) != null) {

Review comment:
       We could extract these methods results to a variable and use it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-890218661


   <b>Trillian test result (tid-1428)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 46955 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5259-t1428-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 86 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 510.42 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Error` | 3879.13 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 76.95 | test_kubernetes_clusters.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5259: usage: create backup usage record for vmId-offeringId pair

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5259:
URL: https://github.com/apache/cloudstack/pull/5259#issuecomment-891923112


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org