cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nvazquez <...@git.apache.org>
Subject [GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to...
Date Fri, 07 Apr 2017 15:47:43 GMT
Github user nvazquez commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1935#discussion_r110421376
  
    --- Diff: server/src/com/cloud/user/DomainManagerImpl.java ---
    @@ -273,82 +284,145 @@ public boolean deleteDomain(long domainId, Boolean cleanup) {
     
         @Override
         public boolean deleteDomain(DomainVO domain, Boolean cleanup) {
    -        // mark domain as inactive
    -        s_logger.debug("Marking domain id=" + domain.getId() + " as " + Domain.State.Inactive
+ " before actually deleting it");
    -        domain.setState(Domain.State.Inactive);
    -        _domainDao.update(domain.getId(), domain);
    -        boolean rollBackState = false;
    -        boolean hasDedicatedResources = false;
    +        GlobalLock lock = getGlobalLock("AccountCleanup");
    +        if (lock == null) {
    +            s_logger.debug("Couldn't get the global lock");
    +            return false;
    +        }
    +
    +        if (!lock.lock(30)) {
    +            s_logger.debug("Couldn't lock the db");
    +            return false;
    +        }
     
             try {
    -            long ownerId = domain.getAccountId();
    -            if ((cleanup != null) && cleanup.booleanValue()) {
    -                if (!cleanupDomain(domain.getId(), ownerId)) {
    -                    rollBackState = true;
    -                    CloudRuntimeException e =
    -                        new CloudRuntimeException("Failed to clean up domain resources
and sub domains, delete failed on domain " + domain.getName() + " (id: " +
    -                            domain.getId() + ").");
    -                    e.addProxyObject(domain.getUuid(), "domainId");
    -                    throw e;
    -                }
    -            } else {
    -                //don't delete the domain if there are accounts set for cleanup, or non-removed
networks exist, or domain has dedicated resources
    -                List<Long> networkIds = _networkDomainDao.listNetworkIdsByDomain(domain.getId());
    -                List<AccountVO> accountsForCleanup = _accountDao.findCleanupsForRemovedAccounts(domain.getId());
    -                List<DedicatedResourceVO> dedicatedResources = _dedicatedDao.listByDomainId(domain.getId());
    -                if (dedicatedResources != null && !dedicatedResources.isEmpty())
{
    -                    s_logger.error("There are dedicated resources for the domain " +
domain.getId());
    -                    hasDedicatedResources = true;
    -                }
    -                if (accountsForCleanup.isEmpty() && networkIds.isEmpty() &&
!hasDedicatedResources) {
    -                    _messageBus.publish(_name, MESSAGE_PRE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL,
domain);
    -                    if (!_domainDao.remove(domain.getId())) {
    -                        rollBackState = true;
    -                        CloudRuntimeException e =
    -                            new CloudRuntimeException("Delete failed on domain " + domain.getName()
+ " (id: " + domain.getId() +
    -                                "); Please make sure all users and sub domains have been
removed from the domain before deleting");
    -                        e.addProxyObject(domain.getUuid(), "domainId");
    -                        throw e;
    -                    }
    -                    _messageBus.publish(_name, MESSAGE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL,
domain);
    +            // mark domain as inactive
    +            s_logger.debug("Marking domain id=" + domain.getId() + " as " + Domain.State.Inactive
+ " before actually deleting it");
    +            domain.setState(Domain.State.Inactive);
    +            _domainDao.update(domain.getId(), domain);
    +
    +            boolean rollBackState = false;
    +
    +            try {
    +                long ownerId = domain.getAccountId();
    +                if (BooleanUtils.toBoolean(cleanup)) {
    +                    tryCleanupDomain(domain, ownerId);
                     } else {
    -                    rollBackState = true;
    -                    String msg = null;
    -                    if (!accountsForCleanup.isEmpty()) {
    -                        msg = accountsForCleanup.size() + " accounts to cleanup";
    -                    } else if (!networkIds.isEmpty()) {
    -                        msg = networkIds.size() + " non-removed networks";
    -                    } else if (hasDedicatedResources) {
    -                        msg = "dedicated resources.";
    -                    }
    +                    removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain);
    +                }
     
    -                    CloudRuntimeException e = new CloudRuntimeException("Can't delete
the domain yet because it has " + msg);
    -                    e.addProxyObject(domain.getUuid(), "domainId");
    -                    throw e;
    +                cleanupDomainOfferings(domain.getId());
    +                CallContext.current().putContextParameter(Domain.class, domain.getUuid());
    +                return true;
    +            } catch (Exception ex) {
    +                s_logger.error("Exception deleting domain with id " + domain.getId(),
ex);
    +                if (ex instanceof CloudRuntimeException) {
    +                    rollBackState = true;
    +                    throw (CloudRuntimeException)ex;
    +                }
    +                else
    +                    return false;
    +            } finally {
    +                //when success is false
    +                if (rollBackState) {
    +                    s_logger.debug("Changing domain id=" + domain.getId() + " state back
to " + Domain.State.Active +
    --- End diff --
    
    Nice, I like how method is after this last refactor. It is much clearer without `rollBackState`
variable, thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message