Skip to content

Commit cec8bc4

Browse files
committed
Usage: do not add a second backup usage row on offering re-assign
Removing a backup offering while keeping the backups leaves its usage row active. Re-assigning the same offering added another active row, so the same backups were billed twice. Skip the new row when one is active.
1 parent 5589a08 commit cec8bc4

2 files changed

Lines changed: 54 additions & 1 deletion

File tree

‎usage/src/main/java/com/cloud/usage/UsageManagerImpl.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2038,7 +2038,7 @@ private void createVmSnapshotOnPrimaryEvent(UsageEventVO event) {
20382038
}
20392039
}
20402040

2041-
private void createBackupEvent(final UsageEventVO event) {
2041+
protected void createBackupEvent(final UsageEventVO event) {
20422042
Long vmId = event.getResourceId();
20432043
Long zoneId = event.getZoneId();
20442044
Long accountId = event.getAccountId();
@@ -2048,6 +2048,12 @@ private void createBackupEvent(final UsageEventVO event) {
20482048
Date created = event.getCreateDate();
20492049

20502050
if (EventTypes.EVENT_VM_BACKUP_OFFERING_ASSIGN.equals(event.getType())) {
2051+
// Removing the offering while keeping the backups leaves the usage active, so re-assigning the
2052+
// same offering must not add a second row that bills the same backups again.
2053+
if (!usageBackupDao.listActiveUsage(vmId, backupOfferingId).isEmpty()) {
2054+
logger.debug("VM [{}] already has active usage for backup offering [{}], not creating another usage entry.", vmId, backupOfferingId);
2055+
return;
2056+
}
20512057
final UsageBackupVO backupVO = new UsageBackupVO(zoneId, accountId, domainId, vmId, backupOfferingId, created);
20522058
usageBackupDao.persist(backupVO);
20532059
} else if (EventTypes.EVENT_VM_BACKUP_OFFERING_REMOVED_AND_BACKUPS_DELETED.equals(event.getType())) {

‎usage/src/test/java/com/cloud/usage/UsageManagerImplTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
package com.cloud.usage;
1818

1919
import java.util.ArrayList;
20+
import java.util.Date;
2021
import java.util.List;
2122

2223
import com.cloud.event.dao.UsageEventDetailsDao;
24+
import com.cloud.usage.dao.UsageBackupDao;
2325
import com.cloud.usage.dao.UsageVMSnapshotDao;
2426
import org.junit.Before;
2527
import org.junit.Test;
@@ -58,6 +60,9 @@ public class UsageManagerImplTest {
5860
@Mock
5961
private AccountDao accountDaoMock;
6062

63+
@Mock
64+
private UsageBackupDao usageBackupDaoMock;
65+
6166
@Mock
6267
private UsageVPNUserVO vpnUserMock;
6368

@@ -243,4 +248,46 @@ public void handleVMSnapshotEventTestEventIsNeitherAddNorRemove() {
243248
Mockito.verify(usageManagerImpl, Mockito.never()).createUsageVpnUser(usageEventVOMock,accountMock);
244249
Mockito.verify(usageManagerImpl, Mockito.never()).deleteUsageVpnUser(usageEventVOMock, accountMock);
245250
}
251+
252+
private void mockBackupEvent(String type) {
253+
Mockito.when(usageEventVOMock.getType()).thenReturn(type);
254+
Mockito.when(usageEventVOMock.getResourceId()).thenReturn(10L);
255+
Mockito.when(usageEventVOMock.getZoneId()).thenReturn(1L);
256+
Mockito.when(usageEventVOMock.getAccountId()).thenReturn(accountMockId);
257+
Mockito.when(usageEventVOMock.getOfferingId()).thenReturn(20L);
258+
}
259+
260+
@Test
261+
public void createBackupEventTestAssignCreatesUsage() {
262+
mockBackupEvent(EventTypes.EVENT_VM_BACKUP_OFFERING_ASSIGN);
263+
Mockito.when(usageEventVOMock.getCreateDate()).thenReturn(new Date());
264+
Mockito.when(usageBackupDaoMock.listActiveUsage(10L, 20L)).thenReturn(new ArrayList<>());
265+
266+
usageManagerImpl.createBackupEvent(usageEventVOMock);
267+
268+
Mockito.verify(usageBackupDaoMock).persist(Mockito.any(UsageBackupVO.class));
269+
}
270+
271+
@Test
272+
public void createBackupEventTestAssignSkipsWhenUsageIsActive() {
273+
mockBackupEvent(EventTypes.EVENT_VM_BACKUP_OFFERING_ASSIGN);
274+
Mockito.when(usageBackupDaoMock.listActiveUsage(10L, 20L)).thenReturn(List.of(Mockito.mock(UsageBackupVO.class)));
275+
276+
usageManagerImpl.createBackupEvent(usageEventVOMock);
277+
278+
Mockito.verify(usageBackupDaoMock, Mockito.never()).persist(Mockito.any(UsageBackupVO.class));
279+
}
280+
281+
@Test
282+
public void createBackupEventTestMetricPassesEventDate() {
283+
Date eventDate = new Date();
284+
mockBackupEvent(EventTypes.EVENT_VM_BACKUP_USAGE_METRIC);
285+
Mockito.when(usageEventVOMock.getCreateDate()).thenReturn(eventDate);
286+
Mockito.when(usageEventVOMock.getSize()).thenReturn(100L);
287+
Mockito.when(usageEventVOMock.getVirtualSize()).thenReturn(1000L);
288+
289+
usageManagerImpl.createBackupEvent(usageEventVOMock);
290+
291+
Mockito.verify(usageBackupDaoMock).updateMetrics(10L, 20L, 100L, 1000L, eventDate);
292+
}
246293
}

0 commit comments

Comments
 (0)