bug 10438: always return success on disableAccount when it got disabled successfully in the DB. If his vms failed to stop on the backend, mark account for cleanup and let background thread to do the cleanup job

status 10438: resolved fixed
This commit is contained in:
alena 2011-07-11 17:45:50 -07:00
parent 67f76b5192
commit 50dce6d5a0
5 changed files with 57 additions and 64 deletions

View File

@ -50,7 +50,6 @@ import com.cloud.network.rules.FirewallRule;
import com.cloud.network.rules.RulesManager;
import com.cloud.network.vpn.RemoteAccessVpnElement;
import com.cloud.offering.NetworkOffering;
import com.cloud.offerings.NetworkOfferingVO;
import com.cloud.offerings.dao.NetworkOfferingDao;
import com.cloud.org.Cluster;
import com.cloud.user.AccountManager;
@ -61,7 +60,6 @@ import com.cloud.vm.NicProfile;
import com.cloud.vm.ReservationContext;
import com.cloud.vm.UserVmManager;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VirtualMachineProfile;
import com.cloud.vm.dao.DomainRouterDao;
import com.cloud.vm.dao.UserVmDao;

View File

@ -3681,50 +3681,6 @@ public class ManagementServerImpl implements ManagementServer {
return new String[] { "commands.properties" };
}
protected class AccountCleanupTask implements Runnable {
@Override
public void run() {
try {
GlobalLock lock = GlobalLock.getInternLock("AccountCleanup");
if (lock == null) {
s_logger.debug("Couldn't get the global lock");
return;
}
if (!lock.lock(30)) {
s_logger.debug("Couldn't lock the db");
return;
}
Transaction txn = null;
try {
txn = Transaction.open(Transaction.CLOUD_DB);
List<AccountVO> accounts = _accountDao.findCleanups();
s_logger.info("Found " + accounts.size() + " accounts to cleanup");
for (AccountVO account : accounts) {
s_logger.debug("Cleaning up " + account.getId());
try {
_accountMgr.cleanupAccount(account, _accountMgr.getSystemUser().getId(), _accountMgr.getSystemAccount());
} catch (Exception e) {
s_logger.error("Skipping due to error on account " + account.getId(), e);
}
}
} catch (Exception e) {
s_logger.error("Exception ", e);
} finally {
if (txn != null) {
txn.close();
}
lock.unlock();
}
} catch (Exception e) {
s_logger.error("Exception ", e);
}
}
}
protected class EventPurgeTask implements Runnable {
@Override
public void run() {

View File

@ -987,6 +987,7 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag
boolean success = false;
AccountVO acctForUpdate = _accountDao.createForUpdate();
acctForUpdate.setState(State.enabled);
acctForUpdate.setNeedsCleanup(false);
success = _accountDao.update(Long.valueOf(accountId), acctForUpdate);
return success;
}
@ -1169,14 +1170,19 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag
}
AccountVO account = _accountDao.findById(accountId);
if ((account == null) || account.getState().equals(State.disabled)) {
if ((account == null) || (account.getState().equals(State.disabled) && !account.getNeedsCleanup())) {
success = true;
} else {
AccountVO acctForUpdate = _accountDao.createForUpdate();
acctForUpdate.setState(State.disabled);
success = _accountDao.update(Long.valueOf(accountId), acctForUpdate);
success = (success && doDisableAccount(accountId));
if (success) {
if (!doDisableAccount(accountId)) {
s_logger.warn("Failed to disable account " + account + " resources as a part of disableAccount call, marking the account for cleanup");
_accountDao.markForCleanup(accountId);
}
}
}
return success;
}
@ -1832,9 +1838,10 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag
try {
txn = Transaction.open(Transaction.CLOUD_DB);
List<AccountVO> accounts = _accountDao.findCleanups();
s_logger.info("Found " + accounts.size() + " accounts to cleanup");
for (AccountVO account : accounts) {
//Cleanup removed accounts
List<AccountVO> removedAccounts = _accountDao.findCleanupsForRemovedAccounts();
s_logger.info("Found " + removedAccounts.size() + " removed accounts to cleanup");
for (AccountVO account : removedAccounts) {
s_logger.debug("Cleaning up " + account.getId());
try {
cleanupAccount(account, getSystemUser().getId(), getSystemAccount());
@ -1842,6 +1849,23 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag
s_logger.error("Skipping due to error on account " + account.getId(), e);
}
}
//cleanup disabled accounts
List<AccountVO> disabledAccounts = _accountDao.findCleanupsForDisabledAccounts();
s_logger.info("Found " + disabledAccounts.size() + " disabled accounts to cleanup");
for (AccountVO account : disabledAccounts) {
s_logger.debug("Cleaning up " + account.getId());
try {
if (disableAccount(account.getId())) {
account.setNeedsCleanup(false);
_accountDao.update(account.getId(), account);
}
} catch (Exception e) {
s_logger.error("Skipping due to error on account " + account.getId(), e);
}
}
} catch (Exception e) {
s_logger.error("Exception ", e);
} finally {

View File

@ -37,9 +37,10 @@ public interface AccountDao extends GenericDao<AccountVO, Long> {
List<AccountVO> findActiveAccounts(Long maxAccountId, Filter filter);
List<AccountVO> findRecentlyDeletedAccounts(Long maxAccountId, Date earliestRemovedDate, Filter filter);
List<AccountVO> findNewAccounts(Long minAccountId, Filter filter);
List<AccountVO> findCleanups();
List<AccountVO> findCleanupsForRemovedAccounts();
List<AccountVO> findAdminAccountsForDomain(Long domainId);
List<AccountVO> findActiveAccountsForDomain(Long domain);
void markForCleanup(long accountId);
List<AccountVO> listAccounts(String accountName, Long domainId, Filter filter);
List<AccountVO> listAccounts(String accountName, Long domainId, Filter filter);
List<AccountVO> findCleanupsForDisabledAccounts();
}

View File

@ -50,9 +50,9 @@ public class AccountDaoImpl extends GenericDaoBase<AccountVO, Long> implements A
protected final SearchBuilder<AccountVO> AccountNameSearch;
protected final SearchBuilder<AccountVO> AccountTypeSearch;
protected final SearchBuilder<AccountVO> DomainAccountsSearch;
protected final SearchBuilder<AccountVO> CleanupSearch;
protected final SearchBuilder<AccountVO> DomainAccountsSearch;
protected final SearchBuilder<AccountVO> CleanupForRemovedAccountsSearch;
protected final SearchBuilder<AccountVO> CleanupForDisabledAccountsSearch;
protected AccountDaoImpl() {
AccountNameSearch = createSearchBuilder();
@ -69,19 +69,33 @@ public class AccountDaoImpl extends GenericDaoBase<AccountVO, Long> implements A
DomainAccountsSearch.and("removed", DomainAccountsSearch.entity().getRemoved(), SearchCriteria.Op.NULL);
DomainAccountsSearch.done();
CleanupSearch = createSearchBuilder();
CleanupSearch.and("cleanup", CleanupSearch.entity().getNeedsCleanup(), SearchCriteria.Op.EQ);
CleanupSearch.and("removed", CleanupSearch.entity().getRemoved(), SearchCriteria.Op.NNULL);
CleanupSearch.done();
CleanupForRemovedAccountsSearch = createSearchBuilder();
CleanupForRemovedAccountsSearch.and("cleanup", CleanupForRemovedAccountsSearch.entity().getNeedsCleanup(), SearchCriteria.Op.EQ);
CleanupForRemovedAccountsSearch.and("removed", CleanupForRemovedAccountsSearch.entity().getRemoved(), SearchCriteria.Op.NNULL);
CleanupForRemovedAccountsSearch.done();
CleanupForDisabledAccountsSearch = createSearchBuilder();
CleanupForDisabledAccountsSearch.and("cleanup", CleanupForDisabledAccountsSearch.entity().getNeedsCleanup(), SearchCriteria.Op.EQ);
CleanupForDisabledAccountsSearch.and("removed", CleanupForDisabledAccountsSearch.entity().getRemoved(), SearchCriteria.Op.NULL);
CleanupForDisabledAccountsSearch.and("state", CleanupForDisabledAccountsSearch.entity().getState(), SearchCriteria.Op.EQ);
CleanupForDisabledAccountsSearch.done();
}
@Override
public List<AccountVO> findCleanups() {
SearchCriteria<AccountVO> sc = CleanupSearch.create();
public List<AccountVO> findCleanupsForRemovedAccounts() {
SearchCriteria<AccountVO> sc = CleanupForRemovedAccountsSearch.create();
sc.setParameters("cleanup", true);
return searchIncludingRemoved(sc, null, null, false);
}
@Override
public List<AccountVO> findCleanupsForDisabledAccounts() {
SearchCriteria<AccountVO> sc = CleanupForDisabledAccountsSearch.create();
sc.setParameters("cleanup", true);
sc.setParameters("state", State.disabled);
return listBy(sc);
}
@Override