[GitHub] [cloudstack] nvazquez commented on a change in pull request #4251: Handle with VM snapshot events

[GitBox <gi...@apache.org>]

nvazquez commented on a change in pull request #4251:
URL: https://github.com/apache/cloudstack/pull/4251#discussion_r503940063



##########
File path: usage/src/main/java/com/cloud/usage/UsageManagerImpl.java
##########
@@ -1889,29 +1889,93 @@ private void createSecurityGroupEvent(UsageEventVO event) {
         }
     }
 
-    private void createVMSnapshotEvent(UsageEventVO event) {
-        Long vmId = event.getResourceId();
-        Long volumeId = event.getTemplateId();
+    /**
+     * Handles Vm Snapshot create and delete events:
+     * <ul>
+     *     <li>EventTypes#EVENT_VM_SNAPSHOT_CREATE</li>
+     *     <li>EventTypes#EVENT_VM_SNAPSHOT_DELETE</li>
+     * </ul>
+     * if the event received by this method is neither add nor remove, we ignore it.
+     */
+    protected void handleVMSnapshotEvent(UsageEventVO event) {
+        switch (event.getType()) {
+            case EventTypes.EVENT_VM_SNAPSHOT_CREATE:
+                createUsageVMSnapshot(event);
+                break;
+            case EventTypes.EVENT_VM_SNAPSHOT_DELETE:
+                deleteUsageVMSnapshot(event);
+                break;
+            default:
+                s_logger.debug(String.format("The event [type=%s, zoneId=%s, accountId=%s, resourceName=%s, diskOfferingId=%s, createDate=%s] is neither of type [%s] nor [%s]",
+                        event.getType(), event.getZoneId(), event.getAccountId(), event.getResourceName(), event.getOfferingId(), event.getCreateDate(), EventTypes.EVENT_VM_SNAPSHOT_CREATE, EventTypes.EVENT_VM_SNAPSHOT_DELETE));
+        }
+    }
+
+    /**
+     * Creates an entry for the Usage VM Snapshot.
+     */
+    protected void createUsageVMSnapshot(UsageEventVO event) {
+        long accountId = event.getAccountId();
+        Account acct = _accountDao.findByIdIncludingRemoved(event.getAccountId());
+        long domainId = acct.getDomainId();
         Long offeringId = event.getOfferingId();
-        Long zoneId = event.getZoneId();
-        Long accountId = event.getAccountId();
-        //Size could be null for VM snapshot delete events
-        long size = (event.getSize() == null) ? 0 : event.getSize();
+        long vmId = event.getResourceId();
+        long volumeId = event.getTemplateId();
+        long zoneId = event.getZoneId();
         Date created = event.getCreateDate();
-        Account acct = _accountDao.findByIdIncludingRemoved(event.getAccountId());
-        Long domainId = acct.getDomainId();
+        long size = (event.getSize() == null) ? 0 : event.getSize();
 
         UsageEventDetailsVO detailVO = _usageEventDetailsDao.findDetail(event.getId(), UsageEventVO.DynamicParameters.vmSnapshotId.name());
         Long vmSnapshotId = null;
         if (detailVO != null) {
             String snapId = detailVO.getValue();
-             vmSnapshotId = Long.valueOf(snapId);
+            vmSnapshotId = Long.valueOf(snapId);
         }
+        s_logger.debug(String.format("Creating usage VM Snapshot for VM id [%s] assigned to account [%s] domain [%s], zone [%s], and created at [%s]", vmId, accountId, domainId, zoneId,
+                event.getCreateDate()));
         UsageVMSnapshotVO vsVO = new UsageVMSnapshotVO(volumeId, zoneId, accountId, domainId, vmId, offeringId, size, created, null);
         vsVO.setVmSnapshotId(vmSnapshotId);
         _usageVMSnapshotDao.persist(vsVO);
     }
 
+    /**
+     * Find and delete, if exists, usage VM Snapshots entries
+     */
+    protected void deleteUsageVMSnapshot(UsageEventVO event) {
+        long accountId = event.getAccountId();
+        Account acct = _accountDao.findByIdIncludingRemoved(event.getAccountId());
+        Long domainId = acct.getDomainId();
+        Long diskOfferingId = event.getOfferingId();
+        long vmId = event.getResourceId();
+        long zoneId = event.getZoneId();
+        List<UsageVMSnapshotVO> usageVMSnapshots = findUsageVMSnapshots(accountId, zoneId, domainId, vmId, diskOfferingId);
+        if (CollectionUtils.isEmpty(usageVMSnapshots)){
+            s_logger.warn(String.format("No usage entry for VM snapshot for VM id [%s] assigned to account [%s] domain [%s] and zone [%s] was found.",
+                    vmId, accountId, domainId, zoneId));
+            if (usageVMSnapshots.size() > 1) {

Review comment:
       This `if` statement got nested inside the `isEmpty` check - needs to be outside




----------------------------------------------------------------
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.

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