Skip to content

Fixes regarding VOLUME_DELETE events resulting from account deletion #1624

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
asfgit merged 1 commit into from
Oct 22, 2016
+787 −203
Merged

Fixes regarding VOLUME_DELETE events resulting from account deletion #1624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 11 additions & 1 deletion server/src/com/cloud/user/AccountManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,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;
Expand Down Expand Up @@ -755,8 +756,17 @@ protected boolean cleanupAccount(AccountVO account, long callerUserId, Account c
s_logger.debug("Expunging # of vms (accountId=" + accountId + "): " + vms.size());
}

// no need to catch exception at this place as expunging vm should pass in order to perform further cleanup
for (UserVmVO vm : vms) {
if (vm.getState() != VirtualMachine.State.Destroyed && vm.getState() != VirtualMachine.State.Expunging) {
try {
_vmMgr.destroyVm(vm.getId());
} catch (Exception e) {
e.printStackTrace();
s_logger.warn("Failed destroying instance " + vm.getUuid() + " as part of account deletion.");
}
}
// no need to catch exception at this place as expunging vm
// should pass in order to perform further cleanup
if (!_vmMgr.expunge(vm, callerUserId, caller)) {
s_logger.error("Unable to expunge vm: " + vm.getId());
accountCleanupNeeded = true;
Expand Down
220 changes: 18 additions & 202 deletions server/test/com/cloud/user/AccountManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,220 +16,35 @@
// under the License.
package com.cloud.user;

import java.lang.reflect.Field;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.ArrayList;

import javax.inject.Inject;

import java.util.Arrays;
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;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
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 com.cloud.configuration.ConfigurationManager;
import com.cloud.configuration.dao.ResourceCountDao;
import com.cloud.configuration.dao.ResourceLimitDao;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.dc.dao.DataCenterVnetDao;
import com.cloud.dc.dao.DedicatedResourceDao;
import com.cloud.domain.Domain;
import com.cloud.domain.DomainVO;
import com.cloud.domain.dao.DomainDao;
import com.cloud.exception.ConcurrentOperationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.as.AutoScaleManager;
import com.cloud.network.dao.AccountGuestVlanMapDao;
import com.cloud.network.dao.IPAddressDao;
import com.cloud.network.dao.NetworkDao;
import com.cloud.network.dao.RemoteAccessVpnDao;
import com.cloud.network.dao.VpnUserDao;
import com.cloud.network.security.SecurityGroupManager;
import com.cloud.network.security.dao.SecurityGroupDao;
import com.cloud.network.vpc.VpcManager;
import com.cloud.network.vpn.RemoteAccessVpnService;
import com.cloud.network.vpn.Site2SiteVpnManager;
import com.cloud.projects.ProjectManager;
import com.cloud.projects.dao.ProjectAccountDao;
import com.cloud.projects.dao.ProjectDao;
import com.cloud.storage.VolumeApiService;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.storage.snapshot.SnapshotManager;
import com.cloud.template.TemplateManager;
import com.cloud.user.Account.State;
import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.UserAccountDao;
import com.cloud.user.dao.UserDao;
import com.cloud.vm.UserVmManager;
import com.cloud.vm.UserVmManagerImpl;
import com.cloud.vm.UserVmVO;
import com.cloud.vm.VMInstanceVO;
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;

@RunWith(MockitoJUnitRunner.class)
public class AccountManagerImplTest {
@Mock
AccountDao _accountDao;
@Mock
ConfigurationDao _configDao;
@Mock
ResourceCountDao _resourceCountDao;
@Mock
UserDao _userDao;
@Mock
InstanceGroupDao _vmGroupDao;
@Mock
UserAccountDao _userAccountDao;
@Mock
VolumeDao _volumeDao;
@Mock
UserVmDao _userVmDao;
@Mock
VMTemplateDao _templateDao;
@Mock
NetworkDao _networkDao;
@Mock
SecurityGroupDao _securityGroupDao;
@Mock
VMInstanceDao _vmDao;
@Mock
protected SnapshotDao _snapshotDao;
@Mock
protected VMTemplateDao _vmTemplateDao;
@Mock
SecurityGroupManager _networkGroupMgr;
@Mock
NetworkOrchestrationService _networkMgr;
@Mock
SnapshotManager _snapMgr;
@Mock
UserVmManager _vmMgr;
@Mock
TemplateManager _tmpltMgr;
@Mock
ConfigurationManager _configMgr;
@Mock
VirtualMachineManager _itMgr;
@Mock
RemoteAccessVpnDao _remoteAccessVpnDao;
@Mock
RemoteAccessVpnService _remoteAccessVpnMgr;
@Mock
VpnUserDao _vpnUser;
@Mock
DataCenterDao _dcDao;
@Mock
DomainManager _domainMgr;
@Mock
ProjectManager _projectMgr;
@Mock
ProjectDao _projectDao;
@Mock
AccountDetailsDao _accountDetailsDao;
@Mock
DomainDao _domainDao;
@Mock
ProjectAccountDao _projectAccountDao;
@Mock
IPAddressDao _ipAddressDao;
@Mock
VpcManager _vpcMgr;
@Mock
DomainRouterDao _routerDao;
@Mock
Site2SiteVpnManager _vpnMgr;
@Mock
AutoScaleManager _autoscaleMgr;
@Mock
VolumeApiService volumeService;
@Mock
AffinityGroupDao _affinityGroupDao;
@Mock
AccountGuestVlanMapDao _accountGuestVlanMapDao;
@Mock
DataCenterVnetDao _dataCenterVnetDao;
@Mock
ResourceLimitService _resourceLimitMgr;
@Mock
ResourceLimitDao _resourceLimitDao;
@Mock
DedicatedResourceDao _dedicatedDao;
@Mock
GlobalLoadBalancerRuleDao _gslbRuleDao;
@Mock
MessageBus _messageBus;

@Mock
VMSnapshotManager _vmSnapshotMgr;
@Mock
VMSnapshotDao _vmSnapshotDao;
public class AccountManagerImplTest extends AccountManagetImplTestBase {

@Mock
User callingUser;
@Mock
Account callingAccount;

AccountManagerImpl accountManager;

@Mock
SecurityChecker securityChecker;

@Mock
private UserAuthenticator userAuthenticator;

@Before
public void setup() throws NoSuchFieldException, SecurityException,
IllegalArgumentException, IllegalAccessException {
accountManager = new AccountManagerImpl();
for (Field field : AccountManagerImpl.class.getDeclaredFields()) {
if (field.getAnnotation(Inject.class) != null) {
field.setAccessible(true);
try {
Field mockField = this.getClass().getDeclaredField(
field.getName());
field.set(accountManager, mockField.get(this));
} catch (Exception e) {
// ignore missing fields
}
}
}
ReflectionTestUtils.setField(accountManager, "_userAuthenticators", Arrays.asList(userAuthenticator));
accountManager.setSecurityCheckers(Arrays.asList(securityChecker));
CallContext.register(callingUser, callingAccount);
}

@After
public void cleanup() {
CallContext.unregister();
}
UserVmManagerImpl _vmMgr;

@Test
public void disableAccountNotexisting()
Expand All @@ -240,7 +55,7 @@ public void disableAccountNotexisting()

@Test
public void disableAccountDisabled() throws ConcurrentOperationException,
ResourceUnavailableException {
ResourceUnavailableException {
AccountVO disabledAccount = new AccountVO();
disabledAccount.setState(State.disabled);
Mockito.when(_accountDao.findById(42l)).thenReturn(disabledAccount);
Expand All @@ -249,7 +64,7 @@ public void disableAccountDisabled() throws ConcurrentOperationException,

@Test
public void disableAccount() throws ConcurrentOperationException,
ResourceUnavailableException {
ResourceUnavailableException {
AccountVO account = new AccountVO();
account.setState(State.enabled);
Mockito.when(_accountDao.findById(42l)).thenReturn(account);
Expand All @@ -272,17 +87,17 @@ public void deleteUserAccount() {
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.any(ControlledEntity.class), Mockito.any(AccessType.class),
Mockito.anyString()))
.thenReturn(true);
Mockito.when(_accountDao.remove(42l)).thenReturn(true);
Mockito.when(_configMgr.releaseAccountSpecificVirtualRanges(42l))
.thenReturn(true);
.thenReturn(true);
Mockito.when(_domainMgr.getDomain(Mockito.anyLong())).thenReturn(domain);
Mockito.when(
securityChecker.checkAccess(Mockito.any(Account.class),
Mockito.any(Domain.class)))
.thenReturn(true);
.thenReturn(true);
Mockito.when(_vmSnapshotDao.listByAccountId(Mockito.anyLong())).thenReturn(new ArrayList<VMSnapshotVO>());

Assert.assertTrue(accountManager.deleteUserAccount(42));
Expand All @@ -301,10 +116,10 @@ public void deleteUserAccountCleanup() {
securityChecker.checkAccess(Mockito.any(Account.class),
Mockito.any(ControlledEntity.class), Mockito.any(AccessType.class),
Mockito.anyString()))
.thenReturn(true);
.thenReturn(true);
Mockito.when(_accountDao.remove(42l)).thenReturn(true);
Mockito.when(_configMgr.releaseAccountSpecificVirtualRanges(42l))
.thenReturn(true);
.thenReturn(true);
Mockito.when(_userVmDao.listByAccountId(42l)).thenReturn(
Arrays.asList(Mockito.mock(UserVmVO.class)));
Mockito.when(
Expand All @@ -314,7 +129,7 @@ public void deleteUserAccountCleanup() {
Mockito.when(
securityChecker.checkAccess(Mockito.any(Account.class),
Mockito.any(Domain.class)))
.thenReturn(true);
.thenReturn(true);

Assert.assertTrue(accountManager.deleteUserAccount(42));
// assert that this was NOT a clean delete
Expand All @@ -327,7 +142,7 @@ public void deleteUserAccountCleanup() {
public void testAuthenticateUser() throws UnknownHostException {
Pair<Boolean, UserAuthenticator.ActionOnFailedAuthentication> successAuthenticationPair = new Pair<>(true, null);
Pair<Boolean, UserAuthenticator.ActionOnFailedAuthentication> failureAuthenticationPair = new Pair<>(false,
UserAuthenticator.ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
UserAuthenticator.ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);

UserAccountVO userAccountVO = new UserAccountVO();
userAccountVO.setSource(User.Source.UNKNOWN);
Expand All @@ -353,6 +168,7 @@ 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);

}


}
Loading