----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12853/#review23771 -----------------------------------------------------------
There is no bug in current Java code, and the java part of this review ticket shouldn't be checked in. In the method cleanupDomain(), we try to remove every account before checking if accounts marked for cleanup, exist. // delete users which will also delete accounts and release resources for those accounts SearchCriteria<AccountVO> sc = _accountDao.createSearchCriteria(); sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId); List<AccountVO> accounts = _accountDao.search(sc, null); for (AccountVO account : accounts) { if (account.getType() != Account.ACCOUNT_TYPE_PROJECT) { s_logger.debug("Deleting account " + account + " as a part of domain id=" + domainId + " cleanup"); boolean deleteAccount = _accountMgr.deleteAccount(account, UserContext.current().getCallerUserId(), UserContext.current().getCaller()); if (!deleteAccount) { s_logger.warn("Failed to cleanup account id=" + account.getId() + " as a part of domain cleanup"); } Only after that we verify if there are accounts left. And only if the code above failed, the code below returns non-empty list: // don't remove the domain if there are accounts required cleanup boolean deleteDomainSuccess = true; List<AccountVO> accountsForCleanup = _accountDao.findCleanupsForRemovedAccounts(domainId); So it works as expected. - Alena Prokharchyk On July 23, 2013, 12:34 p.m., Sanjay Tripathi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12853/ > ----------------------------------------------------------- > > (Updated July 23, 2013, 12:34 p.m.) > > > Review request for cloudstack, Alena Prokharchyk, Devdeep Singh, and Prasanna > Santhanam. > > > Bugs: CLOUDSTACK-3688 > > > Repository: cloudstack-git > > > Description > ------- > > > CLOUDSTACK-3688: Test cases > test_accounts.TestDomainForceRemove.test_forceDeleteDomain, not found the > domain to delete and failed. > > This issue is in CS product and not in the test_script. > The problem is coming because in case of deleteDomain with cleanup = true, > CS is not allowing deletion of domain if there are any account under that > domain needs clean up; though these accounts are removed and admin can't > see them in the listaccounts. > So CS should not restrict the deleteDomain in case of accounts needs > cleanup. > > > Diffs > ----- > > server/src/com/cloud/user/DomainManagerImpl.java 1117ff0 > test/integration/component/test_accounts.py 3c284bd > > Diff: https://reviews.apache.org/r/12853/diff/ > > > Testing > ------- > > Verified marvin test on my local cloudstack setup. > > > Thanks, > > Sanjay Tripathi > >