Removed the remove method. Not sure why we added to VirtualMachineManager
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/9bc5870f Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/9bc5870f Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/9bc5870f Branch: refs/heads/master Commit: 9bc5870f01b81d7b6bfbde08d7f2a43f64425ab1 Parents: 3ae0acc Author: Alex Huang <alex.hu...@citrix.com> Authored: Tue Jul 23 15:13:56 2013 -0700 Committer: Alex Huang <alex.hu...@citrix.com> Committed: Tue Jul 23 18:24:58 2013 -0700 ---------------------------------------------------------------------- .../lb/ElasticLoadBalancerManagerImpl.java | 3 +- .../lb/InternalLoadBalancerVMManagerImpl.java | 4 +- .../InternalLBVMManagerTest.java | 19 +++------ .../consoleproxy/ConsoleProxyManagerImpl.java | 17 ++++---- .../VirtualNetworkApplianceManagerImpl.java | 9 ++-- .../secondary/SecondaryStorageManagerImpl.java | 17 ++++---- server/src/com/cloud/vm/UserVmManagerImpl.java | 12 ++++-- .../src/com/cloud/vm/VirtualMachineManager.java | 6 +-- .../com/cloud/vm/VirtualMachineManagerImpl.java | 44 +++++++------------- 9 files changed, 54 insertions(+), 77 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java ---------------------------------------------------------------------- diff --git a/plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java b/plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java index 7c0f87b..8df0caf 100644 --- a/plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java +++ b/plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java @@ -733,7 +733,8 @@ public class ElasticLoadBalancerManagerImpl extends ManagerBase implements Elast if (gceed) { try { s_logger.info("Attempting to destroy ELB VM: " + elbVm); - _itMgr.expunge(elbVm, user, _systemAcct); + _itMgr.expunge(elbVm.getUuid()); + _routerDao.remove(elbVm.getId()); } catch (ResourceUnavailableException e) { s_logger.warn("Unable to destroy unused ELB vm " + elbVm + " due to ", e); gceed = false; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java ---------------------------------------------------------------------- diff --git a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java index 1799e60..e683eb1 100644 --- a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java +++ b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java @@ -513,7 +513,9 @@ public class InternalLoadBalancerVMManagerImpl extends ManagerBase implements _accountMgr.checkAccess(caller, null, true, internalLbVm); - return _itMgr.expunge(internalLbVm, _accountMgr.getActiveUser(callerUserId), caller); + _itMgr.expunge(internalLbVm.getUuid()); + _internalLbVmDao.remove(internalLbVm.getId()); + return true; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/plugins/network-elements/internal-loadbalancer/test/org/apache/cloudstack/internallbvmmgr/InternalLBVMManagerTest.java ---------------------------------------------------------------------- diff --git a/plugins/network-elements/internal-loadbalancer/test/org/apache/cloudstack/internallbvmmgr/InternalLBVMManagerTest.java b/plugins/network-elements/internal-loadbalancer/test/org/apache/cloudstack/internallbvmmgr/InternalLBVMManagerTest.java index 85cf87e..82f90fb 100644 --- a/plugins/network-elements/internal-loadbalancer/test/org/apache/cloudstack/internallbvmmgr/InternalLBVMManagerTest.java +++ b/plugins/network-elements/internal-loadbalancer/test/org/apache/cloudstack/internallbvmmgr/InternalLBVMManagerTest.java @@ -24,13 +24,8 @@ import java.util.List; import javax.inject.Inject; -import com.cloud.offering.NetworkOffering; -import com.cloud.offerings.NetworkOfferingVO; -import com.cloud.offerings.dao.NetworkOfferingDao; import junit.framework.TestCase; -import org.apache.cloudstack.lb.ApplicationLoadBalancerRuleVO; -import org.apache.cloudstack.network.lb.InternalLoadBalancerVMManager; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -38,6 +33,9 @@ import org.mockito.Mockito; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.apache.cloudstack.lb.ApplicationLoadBalancerRuleVO; +import org.apache.cloudstack.network.lb.InternalLoadBalancerVMManager; + import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.manager.Commands; @@ -56,12 +54,12 @@ import com.cloud.network.router.VirtualRouter; import com.cloud.network.router.VirtualRouter.Role; import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.LoadBalancerContainer.Scheme; +import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; -import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; -import com.cloud.user.User; import com.cloud.utils.component.ComponentContext; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Ip; @@ -103,6 +101,7 @@ public class InternalLBVMManagerTest extends TestCase { long validVmId = 1L; long invalidVmId = 2L; + @Override @Before public void setUp() { //mock system offering creation as it's used by configure() method called by initComponentsLifeCycle @@ -160,12 +159,6 @@ public class InternalLBVMManagerTest extends TestCase { networkOfferingVO.setConcurrentConnections(500); Mockito.when(_offeringDao.findById(Mockito.anyLong())).thenReturn(networkOfferingVO); - try { - Mockito.when(_itMgr.expunge(Mockito.any(DomainRouterVO.class), Mockito.any(User.class), Mockito.any(Account.class))).thenReturn(true); - } catch (ResourceUnavailableException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - } Mockito.when(_domainRouterDao.findById(validVmId)).thenReturn(vm); Mockito.when(_domainRouterDao.findById(invalidVmId)).thenReturn(null); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java b/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java index e172092..a1ceac5 100755 --- a/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java +++ b/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java @@ -1152,17 +1152,16 @@ public class ConsoleProxyManagerImpl extends ManagerBase implements ConsoleProxy ConsoleProxyVO proxy = _consoleProxyDao.findById(vmId); try { //expunge the vm - boolean result = _itMgr.expunge(proxy, _accountMgr.getSystemUser(), _accountMgr.getSystemAccount()); - if (result) { - HostVO host = _hostDao.findByTypeNameAndZoneId(proxy.getDataCenterId(), proxy.getHostName(), - Host.Type.ConsoleProxy); - if (host != null) { - s_logger.debug("Removing host entry for proxy id=" + vmId); - result = result && _hostDao.remove(host.getId()); - } + _itMgr.expunge(proxy.getUuid()); + _consoleProxyDao.remove(vmId); + HostVO host = _hostDao.findByTypeNameAndZoneId(proxy.getDataCenterId(), proxy.getHostName(), + Host.Type.ConsoleProxy); + if (host != null) { + s_logger.debug("Removing host entry for proxy id=" + vmId); + return _hostDao.remove(host.getId()); } - return result; + return true; } catch (ResourceUnavailableException e) { s_logger.warn("Unable to expunge " + proxy, e); return false; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 4859f0d..5ab5934 100755 --- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -421,12 +421,9 @@ public class VirtualNetworkApplianceManagerImpl extends ManagerBase implements V _accountMgr.checkAccess(caller, null, true, router); - boolean result = _itMgr.expunge(router, _accountMgr.getActiveUser(callerUserId), _accountMgr.getAccount(router.getAccountId())); - - if (result) { - return router; - } - return null; + _itMgr.expunge(router.getUuid()); + _routerDao.remove(router.getId()); + return router; } @Override http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java b/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java index 56b8dba..05246c9 100755 --- a/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java +++ b/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java @@ -968,17 +968,16 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar SecondaryStorageVmVO ssvm = _secStorageVmDao.findById(vmId); try { - boolean result = _itMgr.expunge(ssvm, _accountMgr.getSystemUser(), _accountMgr.getSystemAccount()); - if (result) { - HostVO host = _hostDao.findByTypeNameAndZoneId(ssvm.getDataCenterId(), ssvm.getHostName(), - Host.Type.SecondaryStorageVM); - if (host != null) { - s_logger.debug("Removing host entry for ssvm id=" + vmId); - result = result && _hostDao.remove(host.getId()); - } + _itMgr.expunge(ssvm.getUuid()); + _secStorageVmDao.remove(ssvm.getId()); + HostVO host = _hostDao.findByTypeNameAndZoneId(ssvm.getDataCenterId(), ssvm.getHostName(), + Host.Type.SecondaryStorageVM); + if (host != null) { + s_logger.debug("Removing host entry for ssvm id=" + vmId); + _hostDao.remove(host.getId()); } - return result; + return true; } catch (ResourceUnavailableException e) { s_logger.warn("Unable to expunge " + ssvm, e); return false; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/server/src/com/cloud/vm/UserVmManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 694ef5b..6db1e8a 100755 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -1503,10 +1503,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir @Override public boolean expunge(UserVmVO vm, long callerUserId, Account caller) { try { + List<VolumeVO> rootVol = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT); // expunge the vm - if (!_itMgr.advanceExpunge(vm, _accountMgr.getSystemUser(), caller)) { - s_logger.info("Did not expunge " + vm); - return false; + _itMgr.advanceExpunge(vm.getUuid()); + // Update Resource count + if (vm.getAccountId() != Account.ACCOUNT_ID_SYSTEM && !rootVol.isEmpty()) { + _resourceLimitMgr.decrementResourceCount(vm.getAccountId(), ResourceType.volume); + _resourceLimitMgr.decrementResourceCount(vm.getAccountId(), ResourceType.primary_storage, + new Long(rootVol.get(0).getSize())); } // Only if vm is not expunged already, cleanup it's resources @@ -1524,7 +1528,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return false; } - _itMgr.remove(vm, _accountMgr.getSystemUser(), caller); + _vmDao.remove(vm.getId()); } return true; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/server/src/com/cloud/vm/VirtualMachineManager.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/vm/VirtualMachineManager.java b/server/src/com/cloud/vm/VirtualMachineManager.java index cf0ede2..5dbd80a 100644 --- a/server/src/com/cloud/vm/VirtualMachineManager.java +++ b/server/src/com/cloud/vm/VirtualMachineManager.java @@ -78,7 +78,7 @@ public interface VirtualMachineManager extends Manager { void stop(String vmUuid) throws ResourceUnavailableException; - <T extends VMInstanceVO> boolean expunge(T vm, User caller, Account account) throws ResourceUnavailableException; + void expunge(String vmUuid) throws ResourceUnavailableException; void registerGuru(VirtualMachine.Type type, VirtualMachineGuru guru); @@ -92,9 +92,7 @@ public interface VirtualMachineManager extends Manager { void advanceStop(String vmUuid, boolean cleanupEvenIfUnableToStop) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException; - <T extends VMInstanceVO> boolean advanceExpunge(T vm, User caller, Account account) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException; - - <T extends VMInstanceVO> boolean remove(T vm, User caller, Account account); + void advanceExpunge(String vmUuid) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException; void destroy(String vmUuid) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9bc5870f/server/src/com/cloud/vm/VirtualMachineManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java index d9a7631..8fbdb19 100755 --- a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -83,7 +83,6 @@ import com.cloud.alert.AlertManager; import com.cloud.capacity.CapacityManager; import com.cloud.configuration.Config; import com.cloud.configuration.ConfigurationManager; -import com.cloud.configuration.Resource.ResourceType; import com.cloud.configuration.dao.ConfigurationDao; import com.cloud.dc.ClusterDetailsDao; import com.cloud.dc.ClusterDetailsVO; @@ -395,16 +394,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } @Override - public <T extends VMInstanceVO> boolean expunge(T vm, User caller, Account account) throws ResourceUnavailableException { + public void expunge(String vmUuid) throws ResourceUnavailableException { try { - if (advanceExpunge(vm, caller, account)) { - // Mark vms as removed - remove(vm, caller, account); - return true; - } else { - s_logger.info("Did not expunge " + vm); - return false; - } + advanceExpunge(vmUuid); } catch (OperationTimedoutException e) { throw new CloudRuntimeException("Operation timed out", e); } catch (ConcurrentOperationException e) { @@ -413,12 +405,17 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } @Override - public <T extends VMInstanceVO> boolean advanceExpunge(T vm, User caller, Account account) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException { + public void advanceExpunge(String vmUuid) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException { + VMInstanceVO vm = _vmDao.findByUuid(vmUuid); + advanceExpunge(vm); + } + + protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException { if (vm == null || vm.getRemoved() != null) { if (s_logger.isDebugEnabled()) { s_logger.debug("Unable to find vm or vm is destroyed: " + vm); } - return true; + return; } advanceStop(vm, false); @@ -426,11 +423,12 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { if (!stateTransitTo(vm, VirtualMachine.Event.ExpungeOperation, vm.getHostId())) { s_logger.debug("Unable to destroy the vm because it is not in the correct state: " + vm); - return false; + throw new CloudRuntimeException("Unable to destroy " + vm); + } } catch (NoTransitionException e) { s_logger.debug("Unable to destroy the vm because it is not in the correct state: " + vm); - return false; + throw new CloudRuntimeException("Unable to destroy " + vm, e); } if (s_logger.isDebugEnabled()) { @@ -450,7 +448,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac _networkMgr.cleanupNics(profile); // Clean up volumes based on the vm's instance id - List<VolumeVO> rootVol = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT); volumeMgr.cleanupVolumes(vm.getId()); VirtualMachineGuru guru = getVmGuru(vm); @@ -475,12 +472,11 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac _agentMgr.send(hostId, cmds); if(!cmds.isSuccessful()){ for (Answer answer : cmds.getAnswers()){ - if(answer != null && !answer.getResult()){ + if (!answer.getResult()) { s_logger.warn("Failed to expunge vm due to: " + answer.getDetails()); - break; + throw new CloudRuntimeException("Unable to expunge " + vm + " due to " + answer.getDetails()); } } - return false; } } } @@ -489,13 +485,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac s_logger.debug("Expunged " + vm); } - // Update Resource count - if (vm.getAccountId() != Account.ACCOUNT_ID_SYSTEM && !rootVol.isEmpty()) { - _resourceLimitMgr.decrementResourceCount(vm.getAccountId(), ResourceType.volume); - _resourceLimitMgr.decrementResourceCount(vm.getAccountId(), ResourceType.primary_storage, - new Long(rootVol.get(0).getSize())); - } - return true; } @DB @@ -1336,11 +1325,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } @Override - public <T extends VMInstanceVO> boolean remove(T vm, User user, Account caller) { - return _vmDao.remove(vm.getId()); - } - - @Override public void destroy(String vmUuid) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException { VMInstanceVO vm = _vmDao.findByUuid(vmUuid); if (vm == null || vm.getState() == State.Destroyed || vm.getState() == State.Expunging || vm.getRemoved() != null) {
