Skip to content

Fix to make recovered volumes be accounted for by Usage #6772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 3 commits into from
Oct 11, 2022
Merged

Fix to make recovered volumes be accounted for by Usage #6772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac3c49e
Pull recent changes and fix tests
JoaoJandre Sep 28, 2022
710cf97
Address reviews
JoaoJandre Sep 30, 2022
947768e
Fix tests
JoaoJandre Sep 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/src/main/java/com/cloud/storage/VolumeApiService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,6 @@ Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account acc
boolean validateVolumeSizeInBytes(long size);

Volume changeDiskOfferingForVolume(ChangeOfferingForVolumeCmd cmd) throws ResourceAllocationException;

void publishVolumeCreationUsageEvent(Volume volume);
}
21 changes: 21 additions & 0 deletions server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1709,9 +1709,30 @@ public Volume recoverVolume(long volumeId) {
_resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.volume, volume.isDisplay());
_resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.primary_storage, volume.isDisplay(), new Long(volume.getSize()));


publishVolumeCreationUsageEvent(volume);

return volume;
}

public void publishVolumeCreationUsageEvent(Volume volume) {
Long diskOfferingId = volume.getDiskOfferingId();
Long offeringId = null;
if (diskOfferingId != null) {
DiskOfferingVO offering = _diskOfferingDao.findById(diskOfferingId);
if (offering != null && !offering.isComputeOnly()) {
offeringId = offering.getId();
}
}
UsageEventUtils
.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), offeringId,
volume.getTemplateId(), volume.getSize(), Volume.class.getName(), volume.getUuid(), volume.isDisplay());

s_logger.debug(String.format("Volume [%s] has been successfully recovered, thus a new usage event %s has been published.", volume.getUuid(), EventTypes.EVENT_VOLUME_CREATE));
}



@Override
@ActionEvent(eventType = EventTypes.EVENT_VOLUME_CHANGE_DISK_OFFERING, eventDescription = "Changing disk offering of a volume")
public Volume changeDiskOfferingForVolume(ChangeOfferingForVolumeCmd cmd) throws ResourceAllocationException {
Expand Down
13 changes: 1 addition & 12 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2268,18 +2268,7 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) throws ResourceAllocationE
for (VolumeVO volume : volumes) {
if (volume.getVolumeType().equals(Volume.Type.ROOT)) {
// Create an event
Long templateId = volume.getTemplateId();
Long diskOfferingId = volume.getDiskOfferingId();
Long offeringId = null;
if (diskOfferingId != null) {
DiskOfferingVO offering = _diskOfferingDao.findById(diskOfferingId);
if (offering != null && !offering.isComputeOnly()) {
offeringId = offering.getId();
}
}
UsageEventUtils
.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), offeringId,
templateId, volume.getSize(), Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume());
_volumeService.publishVolumeCreationUsageEvent(volume);
}
}

Expand Down
80 changes: 77 additions & 3 deletions server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import java.util.UUID;
import java.util.concurrent.ExecutionException;

import com.cloud.event.EventTypes;
import com.cloud.event.UsageEventUtils;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import org.apache.cloudstack.acl.ControlledEntity;
Expand Down Expand Up @@ -69,7 +71,10 @@
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.runners.MockitoJUnitRunner;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.springframework.test.util.ReflectionTestUtils;

import com.cloud.api.query.dao.ServiceOfferingJoinDao;
Expand Down Expand Up @@ -109,12 +114,13 @@
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;

@RunWith(MockitoJUnitRunner.class)
@RunWith(PowerMockRunner.class)
@PowerMockIgnore("javax.management.*")
public class VolumeApiServiceImplTest {

@Spy
@InjectMocks
private VolumeApiServiceImpl volumeApiServiceImpl;
private VolumeApiServiceImpl volumeApiServiceImpl = new VolumeApiServiceImpl();
@Mock
private SnapshotManager snapshotManagerMock;
@Mock
Expand Down Expand Up @@ -189,6 +195,10 @@ public class VolumeApiServiceImplTest {
private long vmInstanceMockId = 1123l;
private long volumeSizeMock = 456789921939l;

private long diskOfferingMockId = 100203L;

private long offeringMockId = 31902L;

@Before
public void setup() throws InterruptedException, ExecutionException {
Mockito.lenient().doReturn(volumeMockId).when(volumeDataStoreVoMock).getVolumeId();
Expand Down Expand Up @@ -1245,4 +1255,68 @@ private void prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(long
boolean result = volumeApiServiceImpl.isNewDiskOfferingTheSameAndCustomServiceOffering(existingDiskOffering, newDiskOffering);
Assert.assertEquals(expectedResult, result);
}

@Test
@PrepareForTest(UsageEventUtils.class)
public void publishVolumeCreationUsageEventTestNullDiskOfferingId() {
Mockito.doReturn(null).when(volumeVoMock).getDiskOfferingId();
PowerMockito.mockStatic(UsageEventUtils.class);

volumeApiServiceImpl.publishVolumeCreationUsageEvent(volumeVoMock);

PowerMockito.verifyStatic(UsageEventUtils.class);
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volumeVoMock.getAccountId(), volumeVoMock.getDataCenterId(), volumeVoMock.getId(), volumeVoMock.getName(),
null, volumeVoMock.getTemplateId(), volumeVoMock.getSize(), Volume.class.getName(), volumeVoMock.getUuid(), volumeVoMock.isDisplay());

}

@Test
@PrepareForTest(UsageEventUtils.class)
public void publishVolumeCreationUsageEventTestNullDiskOfferingVo() {
Mockito.doReturn(diskOfferingMockId).when(volumeVoMock).getDiskOfferingId();
Mockito.doReturn(null).when(_diskOfferingDao).findById(diskOfferingMockId);
PowerMockito.mockStatic(UsageEventUtils.class);

volumeApiServiceImpl.publishVolumeCreationUsageEvent(volumeVoMock);

PowerMockito.verifyStatic(UsageEventUtils.class);
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volumeVoMock.getAccountId(), volumeVoMock.getDataCenterId(), volumeVoMock.getId(), volumeVoMock.getName(),
null, volumeVoMock.getTemplateId(), volumeVoMock.getSize(), Volume.class.getName(), volumeVoMock.getUuid(), volumeVoMock.isDisplay());

}

@Test
@PrepareForTest(UsageEventUtils.class)
public void publishVolumeCreationUsageEventTestDiskOfferingVoTypeNotDisk() {
Mockito.doReturn(diskOfferingMockId).when(volumeVoMock).getDiskOfferingId();
Mockito.doReturn(newDiskOfferingMock).when(_diskOfferingDao).findById(diskOfferingMockId);
Mockito.doReturn(true).when(newDiskOfferingMock).isComputeOnly();

PowerMockito.mockStatic(UsageEventUtils.class);

volumeApiServiceImpl.publishVolumeCreationUsageEvent(volumeVoMock);

PowerMockito.verifyStatic(UsageEventUtils.class);
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volumeVoMock.getAccountId(), volumeVoMock.getDataCenterId(), volumeVoMock.getId(), volumeVoMock.getName(),
null, volumeVoMock.getTemplateId(), volumeVoMock.getSize(), Volume.class.getName(), volumeVoMock.getUuid(), volumeVoMock.isDisplay());

}

@Test
@PrepareForTest(UsageEventUtils.class)
public void publishVolumeCreationUsageEventTestOfferingIdNotNull() {
Mockito.doReturn(diskOfferingMockId).when(volumeVoMock).getDiskOfferingId();
Mockito.doReturn(newDiskOfferingMock).when(_diskOfferingDao).findById(diskOfferingMockId);
Mockito.doReturn(false).when(newDiskOfferingMock).isComputeOnly();
Mockito.doReturn(offeringMockId).when(newDiskOfferingMock).getId();

PowerMockito.mockStatic(UsageEventUtils.class);

volumeApiServiceImpl.publishVolumeCreationUsageEvent(volumeVoMock);

PowerMockito.verifyStatic(UsageEventUtils.class);
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volumeVoMock.getAccountId(), volumeVoMock.getDataCenterId(), volumeVoMock.getId(), volumeVoMock.getName(),
offeringMockId, volumeVoMock.getTemplateId(), volumeVoMock.getSize(), Volume.class.getName(), volumeVoMock.getUuid(), volumeVoMock.isDisplay());

}
}