diff --git a/engine/components-api/src/com/cloud/event/UsageEventUtils.java b/engine/components-api/src/com/cloud/event/UsageEventUtils.java index 69892d60b72e..76344158502b 100644 --- a/engine/components-api/src/com/cloud/event/UsageEventUtils.java +++ b/engine/components-api/src/com/cloud/event/UsageEventUtils.java @@ -177,7 +177,7 @@ private static void publishUsageEvent(String usageEventType, Long accountId, Lon return; // no provider is configured to provide events bus, so just return } - Account account = s_accountDao.findById(accountId); + Account account = s_accountDao.findByIdIncludingRemoved(accountId); DataCenterVO dc = s_dcDao.findById(zoneId); // if account has been deleted, this might be called during cleanup of resources and results in null pointer diff --git a/server/src/com/cloud/storage/listener/VolumeStateListener.java b/server/src/com/cloud/storage/listener/VolumeStateListener.java index 9a739ba2bc11..5c02894ecb85 100644 --- a/server/src/com/cloud/storage/listener/VolumeStateListener.java +++ b/server/src/com/cloud/storage/listener/VolumeStateListener.java @@ -115,6 +115,7 @@ private void pubishOnEventBus(String event, String status, Volume vo, State oldS eventDescription.put("id", vo.getUuid()); eventDescription.put("old-state", oldState.name()); eventDescription.put("new-state", newState.name()); + eventDescription.put("status", status); String eventDate = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss Z").format(new Date()); eventDescription.put("eventDateTime", eventDate); diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index ead7735d82d8..98168301600f 100644 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -84,6 +84,7 @@ import com.cloud.event.ActionEventUtils; import com.cloud.event.ActionEvents; import com.cloud.event.EventTypes; +import com.cloud.event.UsageEventUtils; import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.CloudAuthenticationException; import com.cloud.exception.ConcurrentOperationException; @@ -160,6 +161,7 @@ import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.InstanceGroupDao; import com.cloud.vm.dao.UserVmDao; @@ -679,6 +681,20 @@ public boolean deleteAccount(AccountVO account, long callerUserId, Account calle return cleanupAccount(account, callerUserId, caller); } + /** + * Given a VM, emit volume delete events for all of its ROOT volumes. Called from the cleanupAccount method + * to make sure expunged VMs emit volume delete events. This method does not do anything with the VM or volumes; + * it only emits the volume delete events. + * @param vm - The VM whose ROOT volumes to emit events for. + */ + protected void emitDeleteEventsForExpungingVolumes(UserVmVO vm) { + List volumes = _volumeDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT); + for (VolumeVO volume : volumes) { + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_DELETE, volume.getAccountId(), volume.getDataCenterId(), + volume.getId(), volume.getName(), Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume()); + } + } + protected boolean cleanupAccount(AccountVO account, long callerUserId, Account caller) { long accountId = account.getId(); boolean accountCleanupNeeded = false; @@ -763,6 +779,12 @@ protected boolean cleanupAccount(AccountVO account, long callerUserId, Account c s_logger.error("Unable to expunge vm: " + vm.getId()); accountCleanupNeeded = true; } + else if (!vm.getState().equals(VirtualMachine.State.Destroyed)) { + // We have to emit the event here because the state listener is ignoring root volume deletions. + // It assumes that the UserVMManager is responsible for emitting the usage event for them when + // the vm delete command is processed. + emitDeleteEventsForExpungingVolumes(vm); + } } // Mark the account's volumes as destroyed diff --git a/server/test/com/cloud/user/AccountManagerImplTest.java b/server/test/com/cloud/user/AccountManagerImplTest.java index a895ec27d778..0d72ba3f1c60 100644 --- a/server/test/com/cloud/user/AccountManagerImplTest.java +++ b/server/test/com/cloud/user/AccountManagerImplTest.java @@ -17,23 +17,23 @@ package com.cloud.user; import java.lang.reflect.Field; + import java.net.InetAddress; import java.net.UnknownHostException; -import java.util.Arrays; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; import javax.inject.Inject; import com.cloud.server.auth.UserAuthenticator; import com.cloud.utils.Pair; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.runners.MockitoJUnitRunner; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; @@ -44,10 +44,16 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.region.gslb.GlobalLoadBalancerRuleDao; - -import com.cloud.vm.snapshot.VMSnapshotManager; -import com.cloud.vm.snapshot.VMSnapshotVO; -import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import com.cloud.configuration.ConfigurationManager; import com.cloud.configuration.dao.ResourceCountDao; @@ -58,6 +64,9 @@ import com.cloud.domain.Domain; import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; +import com.cloud.event.UsageEventUtils; +import com.cloud.event.UsageEventVO; +import com.cloud.event.dao.UsageEventDao; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.as.AutoScaleManager; @@ -74,7 +83,9 @@ import com.cloud.projects.ProjectManager; import com.cloud.projects.dao.ProjectAccountDao; import com.cloud.projects.dao.ProjectDao; +import com.cloud.storage.Volume; import com.cloud.storage.VolumeApiService; +import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; @@ -87,12 +98,16 @@ import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.InstanceGroupDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import org.springframework.test.util.ReflectionTestUtils; +import com.cloud.vm.snapshot.VMSnapshotManager; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; @RunWith(MockitoJUnitRunner.class) public class AccountManagerImplTest { @@ -192,6 +207,10 @@ public class AccountManagerImplTest { @Mock VMSnapshotDao _vmSnapshotDao; + @Mock + UsageEventDao _usageEventDao; + final List usageEvents = new ArrayList<>(); //list of persisted usage event VOs + @Mock User callingUser; @Mock @@ -205,6 +224,13 @@ public class AccountManagerImplTest { @Mock private UserAuthenticator userAuthenticator; + /** + * Maintain a list of old fields in the usage utils class... This + * is because of weirdness of how it uses static fields and an init + * method. + */ + private Map oldFields = new HashMap<>(); + @Before public void setup() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { @@ -353,6 +379,177 @@ public void testAuthenticateUser() throws UnknownHostException { Mockito.verify(userAuthenticator, Mockito.times(1)).authenticate("test", "fail", 1L, null); Mockito.verify(userAuthenticator, Mockito.never()).authenticate("test", null, 1L, null); Mockito.verify(userAuthenticator, Mockito.never()).authenticate("test", "", 1L, null); + } + + public UsageEventUtils setupUsageUtils() { + //_usageEventDao = Mockito.mock(UsageEventDao.class); + Mockito.when(_usageEventDao.persist(Mockito.any(UsageEventVO.class))).then(new Answer() { + @Override public Void answer(InvocationOnMock invocation) throws Throwable { + UsageEventVO vo = (UsageEventVO)invocation.getArguments()[0]; + usageEvents.add(vo); + return null; + } + }); + + Mockito.when(_usageEventDao.listAll()).thenReturn(usageEvents); + + UsageEventUtils utils = new UsageEventUtils(); + + Map usageUtilsFields = new HashMap(); + usageUtilsFields.put("usageEventDao", "_usageEventDao"); + usageUtilsFields.put("accountDao", "_accountDao"); + usageUtilsFields.put("dcDao", "_dcDao"); + usageUtilsFields.put("configDao", "_configDao"); + + for (String fieldName : usageUtilsFields.keySet()) { + try { + Field f = UsageEventUtils.class.getDeclaredField(fieldName); + f.setAccessible(true); + //Remember the old fields for cleanup later (see cleanupUsageUtils) + Field staticField = UsageEventUtils.class.getDeclaredField("s_" + fieldName); + staticField.setAccessible(true); + oldFields.put(f.getName(), staticField.get(null)); + f.set(utils, + this.getClass() + .getDeclaredField( + usageUtilsFields.get(fieldName)) + .get(this)); + } catch (IllegalArgumentException | IllegalAccessException + | NoSuchFieldException | SecurityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } + + } + try { + Method method = UsageEventUtils.class.getDeclaredMethod("init"); + method.setAccessible(true); + method.invoke(utils); + } catch (SecurityException | NoSuchMethodException + | IllegalAccessException | IllegalArgumentException + | InvocationTargetException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } + + return utils; + } + public void cleanupUsageUtils() { + UsageEventUtils utils = new UsageEventUtils(); + + for (String fieldName : oldFields.keySet()) { + try { + Field f = UsageEventUtils.class.getDeclaredField(fieldName); + f.setAccessible(true); + f.set(utils, oldFields.get(fieldName)); + } catch (IllegalArgumentException | IllegalAccessException + | NoSuchFieldException | SecurityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } + + } + try { + Method method = UsageEventUtils.class.getDeclaredMethod("init"); + method.setAccessible(true); + method.invoke(utils); + } catch (SecurityException | NoSuchMethodException + | IllegalAccessException | IllegalArgumentException + | InvocationTargetException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } + } + + public List deleteUserAccountRootVolumeUsageEvents(boolean vmDestroyedPrior) { + AccountVO account = new AccountVO(); + account.setId(42l); + DomainVO domain = new DomainVO(); + UserVmVO vm = Mockito.mock(UserVmVO.class); + VolumeVO vol = Mockito.mock(VolumeVO.class); + Mockito.when(_accountDao.findById(42l)).thenReturn(account); + Mockito.when( + securityChecker.checkAccess(Mockito.any(Account.class), + Mockito.any(ControlledEntity.class), Mockito.any(AccessType.class), + Mockito.anyString())) + .thenReturn(true); + Mockito.when(_accountDao.remove(42l)).thenReturn(true); + Mockito.when(_userVmDao.listByAccountId(42l)).thenReturn( + Arrays.asList(vm)); + Mockito.when(_userVmDao.findByUuid(Mockito.any(String.class))).thenReturn(vm); + Mockito.when( + _vmMgr.expunge(Mockito.any(UserVmVO.class), Mockito.anyLong(), + Mockito.any(Account.class))).thenReturn(true); + Mockito.when(vm.getState()).thenReturn( + vmDestroyedPrior + ? VirtualMachine.State.Destroyed + : VirtualMachine.State.Running); + Mockito.when( + _volumeDao.findByInstanceAndType(Mockito.any(Long.class), + Mockito.eq(Volume.Type.ROOT))).thenReturn( + Arrays.asList(vol)); + Mockito.when(vol.getAccountId()).thenReturn((long) 1); + Mockito.when(vol.getDataCenterId()).thenReturn((long) 1); + Mockito.when(vol.getId()).thenReturn((long) 1); + Mockito.when(vol.getName()).thenReturn("root volume"); + Mockito.when(vol.getUuid()).thenReturn("vol-111111"); + Mockito.when(vol.isDisplayVolume()).thenReturn(true); + + Mockito.when(_domainMgr.getDomain(Mockito.anyLong())) + .thenReturn(domain); + + accountManager.deleteUserAccount(42); + return _usageEventDao.listAll(); + } + + @Test + public void emitExpungingRootVolumeDeleteEvents() { + UsageEventUtils utils = setupUsageUtils(); + + UserVmVO vm = Mockito.mock(UserVmVO.class); + VolumeVO vol = Mockito.mock(VolumeVO.class); + + Mockito.when(vol.getAccountId()).thenReturn(1l); + Mockito.when(vol.getDataCenterId()).thenReturn(1l); + Mockito.when(vol.getId()).thenReturn(1l); + Mockito.when(vol.getName()).thenReturn("test-vol"); + Mockito.when(vol.getUuid()).thenReturn(UUID.randomUUID().toString()); + Mockito.when(vol.isDisplayVolume()).thenReturn(true); + + Mockito.when(vm.getId()).thenReturn(1l); + + List mockVolumeList = Collections.singletonList(vol); + + Mockito.when(_volumeDao.findByInstanceAndType(Mockito.eq(1l), Mockito.eq(Volume.Type.ROOT))) + .thenReturn(mockVolumeList); + + + accountManager.emitDeleteEventsForExpungingVolumes(vm); + + List emittedEvents = _usageEventDao.listAll(); + Assert.assertNotNull(emittedEvents); + Assert.assertEquals(1, emittedEvents.size()); + Assert.assertNotNull(emittedEvents.get(0)); + Assert.assertEquals(vol.getId(), emittedEvents.get(0).getResourceId()); + Assert.assertEquals(vol.getName(), emittedEvents.get(0).getResourceName()); + + cleanupUsageUtils(); + } + + @Test + public void destroyedVMRootVolumeUsageEvent() { + UsageEventUtils utils = setupUsageUtils(); + List emittedEvents = deleteUserAccountRootVolumeUsageEvents(true); + Assert.assertEquals(0, emittedEvents.size()); + cleanupUsageUtils(); + } + + @Test + public void runningVMRootVolumeUsageEvent() { + UsageEventUtils utils = setupUsageUtils(); + List emittedEvents = deleteUserAccountRootVolumeUsageEvents(false); + Assert.assertEquals(1, emittedEvents.size()); + cleanupUsageUtils(); } }