> On July 24, 2013, 5:02 p.m., Alena Prokharchyk wrote:
> > 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. 
> >

But the accounts which are marked as cleanup=true are set as removed and admin 
can't list them by using API call. So, the only thing admin can see is the 
domain without any accounts. Please clarify if that makes sense.


- Sanjay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12853/#review23771
-----------------------------------------------------------


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
> 
>

Reply via email to