From c4f00783765ce5dee27d2bacfd34d2b5831af267 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 14 Mar 2025 19:43:03 +0530 Subject: [PATCH 1/4] cks: cleanup clusters on owner account cleanup When the owner account of k8s clusters is deleted, while its node VMs get expunged, the cluster entry in DB remain present. This PR fixex the issue by cleaning up all clusters for the account deleted. Signed-off-by: Abhishek Kumar --- .../cluster/KubernetesServiceHelper.java | 2 ++ .../cluster/KubernetesClusterManagerImpl.java | 36 ++++++++++++++----- .../cluster/KubernetesClusterService.java | 3 ++ .../cluster/KubernetesServiceHelperImpl.java | 8 +++++ .../cluster/dao/KubernetesClusterDao.java | 2 +- .../cluster/dao/KubernetesClusterDaoImpl.java | 14 ++++---- .../com/cloud/user/AccountManagerImpl.java | 5 +++ 7 files changed, 54 insertions(+), 16 deletions(-) diff --git a/api/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelper.java b/api/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelper.java index a39c26680fdb..924dbf37f67a 100644 --- a/api/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelper.java +++ b/api/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelper.java @@ -18,6 +18,7 @@ import org.apache.cloudstack.acl.ControlledEntity; +import com.cloud.user.Account; import com.cloud.uservm.UserVm; import com.cloud.utils.component.Adapter; @@ -25,4 +26,5 @@ public interface KubernetesServiceHelper extends Adapter { ControlledEntity findByUuid(String uuid); void checkVmCanBeDestroyed(UserVm userVm); + void cleanupForAccount(Account account); } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java index 9c402f83b031..7f5e331c89af 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java @@ -1445,9 +1445,7 @@ public boolean deleteKubernetesCluster(DeleteKubernetesClusterCmd cmd) throws Cl } accountManager.checkAccess(CallContext.current().getCallingAccount(), SecurityChecker.AccessType.OperateEntry, false, cluster); if (cluster.getClusterType() == KubernetesCluster.ClusterType.CloudManaged) { - KubernetesClusterDestroyWorker destroyWorker = new KubernetesClusterDestroyWorker(cluster, this); - destroyWorker = ComponentContext.inject(destroyWorker); - return destroyWorker.destroy(); + return destroyKubernetesCluster(cluster); } else { boolean cleanup = cmd.getCleanup(); boolean expunge = cmd.getExpunge(); @@ -1730,6 +1728,30 @@ public List removeVmsFromClu return responseList; } + protected boolean destroyKubernetesCluster(KubernetesCluster kubernetesCluster) { + KubernetesClusterDestroyWorker destroyWorker = new KubernetesClusterDestroyWorker(kubernetesCluster, + KubernetesClusterManagerImpl.this); + destroyWorker = ComponentContext.inject(destroyWorker); + return destroyWorker.destroy(); + } + + @Override + public void cleanupForAccount(Account account) { + List clusters = kubernetesClusterDao.listForCleanupByAccount(account.getId()); + if (CollectionUtils.isEmpty(clusters)) { + return; + } + LOGGER.debug(String.format("Cleaning up %d Kubernetes cluster for %s", clusters.size(), account)); + for (KubernetesClusterVO cluster : clusters) { + try { + destroyKubernetesCluster(cluster); + } catch (CloudRuntimeException e) { + LOGGER.warn(String.format("Failed to destroy Kubernetes cluster: %s during cleanup for %s", + cluster.getName(), account), e); + } + } + } + @Override public List> getCommands() { List> cmdList = new ArrayList>(); @@ -1781,9 +1803,7 @@ public void reallyRun() { LOGGER.info(String.format("Running Kubernetes cluster garbage collector on Kubernetes cluster : %s", kubernetesCluster.getName())); } try { - KubernetesClusterDestroyWorker destroyWorker = new KubernetesClusterDestroyWorker(kubernetesCluster, KubernetesClusterManagerImpl.this); - destroyWorker = ComponentContext.inject(destroyWorker); - if (destroyWorker.destroy()) { + if (destroyKubernetesCluster(kubernetesCluster)) { if (LOGGER.isInfoEnabled()) { LOGGER.info(String.format("Garbage collection complete for Kubernetes cluster : %s", kubernetesCluster.getName())); } @@ -1909,9 +1929,7 @@ public void reallyRun() { LOGGER.info(String.format("Running Kubernetes cluster state scanner on Kubernetes cluster : %s for state: %s", kubernetesCluster.getName(), KubernetesCluster.State.Destroying.toString())); } try { - KubernetesClusterDestroyWorker destroyWorker = new KubernetesClusterDestroyWorker(kubernetesCluster, KubernetesClusterManagerImpl.this); - destroyWorker = ComponentContext.inject(destroyWorker); - destroyWorker.destroy(); + destroyKubernetesCluster(kubernetesCluster); } catch (Exception e) { LOGGER.warn(String.format("Failed to run Kubernetes cluster Destroying state scanner on Kubernetes cluster : %s status scanner", kubernetesCluster.getName()), e); } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java index 6acc876493ef..0c8b7d9fa67c 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java @@ -33,6 +33,7 @@ import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; +import com.cloud.user.Account; import com.cloud.utils.component.PluggableService; import com.cloud.utils.exception.CloudRuntimeException; @@ -122,4 +123,6 @@ public interface KubernetesClusterService extends PluggableService, Configurable boolean addVmsToCluster(AddVirtualMachinesToKubernetesClusterCmd cmd); List removeVmsFromCluster(RemoveVirtualMachinesFromKubernetesClusterCmd cmd); + + void cleanupForAccount(Account account); } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java index 9551142d24e4..57c0ede36540 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java @@ -34,6 +34,7 @@ import com.cloud.kubernetes.cluster.dao.KubernetesClusterVmMapDao; import com.cloud.kubernetes.version.KubernetesSupportedVersion; import com.cloud.kubernetes.version.KubernetesVersionEventTypes; +import com.cloud.user.Account; import com.cloud.uservm.UserVm; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.exception.CloudRuntimeException; @@ -47,6 +48,8 @@ public class KubernetesServiceHelperImpl extends AdapterBase implements Kubernet private KubernetesClusterDao kubernetesClusterDao; @Inject private KubernetesClusterVmMapDao kubernetesClusterVmMapDao; + @Inject + KubernetesClusterService kubernetesClusterService; protected void setEventTypeEntityDetails(Class eventTypeDefinedClass, Class entityClass) { Field[] declaredFields = eventTypeDefinedClass.getDeclaredFields(); @@ -94,6 +97,11 @@ public void checkVmCanBeDestroyed(UserVm userVm) { throw new CloudRuntimeException(msg); } + @Override + public void cleanupForAccount(Account account) { + kubernetesClusterService.cleanupForAccount(account); + } + @Override public String getConfigComponentName() { return KubernetesServiceHelper.class.getSimpleName(); diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/dao/KubernetesClusterDao.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/dao/KubernetesClusterDao.java index 9341912012f1..219e26a8052c 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/dao/KubernetesClusterDao.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/dao/KubernetesClusterDao.java @@ -26,7 +26,7 @@ public interface KubernetesClusterDao extends GenericDao, StateDao { - List listByAccount(long accountId); + List listForCleanupByAccount(long accountId); List findKubernetesClustersToGarbageCollect(); List findManagedKubernetesClustersInState(KubernetesCluster.State state); List listByNetworkId(long networkId); diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/dao/KubernetesClusterDaoImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/dao/KubernetesClusterDaoImpl.java index 63cca3563f72..fb2fc5571a13 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/dao/KubernetesClusterDaoImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/dao/KubernetesClusterDaoImpl.java @@ -30,16 +30,17 @@ @Component public class KubernetesClusterDaoImpl extends GenericDaoBase implements KubernetesClusterDao { - private final SearchBuilder AccountIdSearch; + private final SearchBuilder CleanupAccountIdSearch; private final SearchBuilder GarbageCollectedSearch; private final SearchBuilder ManagedStateSearch; private final SearchBuilder SameNetworkSearch; private final SearchBuilder KubernetesVersionSearch; public KubernetesClusterDaoImpl() { - AccountIdSearch = createSearchBuilder(); - AccountIdSearch.and("account", AccountIdSearch.entity().getAccountId(), SearchCriteria.Op.EQ); - AccountIdSearch.done(); + CleanupAccountIdSearch = createSearchBuilder(); + CleanupAccountIdSearch.and("account", CleanupAccountIdSearch.entity().getAccountId(), SearchCriteria.Op.EQ); + CleanupAccountIdSearch.and("cluster_type", CleanupAccountIdSearch.entity().getClusterType(), SearchCriteria.Op.EQ); + CleanupAccountIdSearch.done(); GarbageCollectedSearch = createSearchBuilder(); GarbageCollectedSearch.and("gc", GarbageCollectedSearch.entity().isCheckForGc(), SearchCriteria.Op.EQ); @@ -62,8 +63,9 @@ public KubernetesClusterDaoImpl() { } @Override - public List listByAccount(long accountId) { - SearchCriteria sc = AccountIdSearch.create(); + public List listForCleanupByAccount(long accountId) { + SearchCriteria sc = CleanupAccountIdSearch.create(); + sc.setParameters("cluster_type", KubernetesCluster.ClusterType.CloudManaged); sc.setParameters("account", accountId); return listBy(sc, null); } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 8e06c5768818..464b866fb342 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -117,6 +117,7 @@ import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceUnavailableException; +import com.cloud.kubernetes.cluster.KubernetesServiceHelper; import com.cloud.network.IpAddress; import com.cloud.network.IpAddressManager; import com.cloud.network.Network; @@ -301,6 +302,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M private UserDataDao userDataDao; @Inject private NetworkPermissionDao networkPermissionDao; + @Inject + KubernetesServiceHelper kubernetesServiceHelper; private List _querySelectors; @@ -929,6 +932,8 @@ protected boolean cleanupAccount(AccountVO account, long callerUserId, Account c } } + kubernetesServiceHelper.cleanupForAccount(account); + // Destroy the account's VMs List vms = _userVmDao.listByAccountId(accountId); if (s_logger.isDebugEnabled()) { From ad123ee807c65cca680d72b9b2ef5f9bbfbb61bd Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 14 Mar 2025 21:53:31 +0530 Subject: [PATCH 2/4] fix tests Signed-off-by: Abhishek Kumar --- .../test/java/com/cloud/user/AccountManagetImplTestBase.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java b/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java index 2fa18221a940..771f9e480547 100644 --- a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java +++ b/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java @@ -24,6 +24,7 @@ import com.cloud.dc.dao.DedicatedResourceDao; import com.cloud.domain.dao.DomainDao; import com.cloud.event.dao.UsageEventDao; +import com.cloud.kubernetes.cluster.KubernetesServiceHelper; import com.cloud.network.as.AutoScaleManager; import com.cloud.network.dao.AccountGuestVlanMapDao; import com.cloud.network.dao.IPAddressDao; @@ -198,6 +199,8 @@ public class AccountManagetImplTestBase { UserDataDao userDataDao; @Mock NetworkPermissionDao networkPermissionDaoMock; + @Mock + KubernetesServiceHelper kubernetesServiceHelper; @Spy @InjectMocks From f84e7d987433c50055c6f6a90e8ab55dca702716 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 14 Mar 2025 22:02:16 +0530 Subject: [PATCH 3/4] add tests Signed-off-by: Abhishek Kumar --- .../KubernetesClusterManagerImplTest.java | 191 +++++++++++------- 1 file changed, 123 insertions(+), 68 deletions(-) diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java index a6d46ffc9aa1..7a1745f8ea31 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java @@ -19,6 +19,35 @@ package com.cloud.kubernetes.cluster; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.command.user.kubernetes.cluster.AddVirtualMachinesToKubernetesClusterCmd; +import org.apache.cloudstack.api.command.user.kubernetes.cluster.RemoveVirtualMachinesFromKubernetesClusterCmd; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + import com.cloud.api.query.dao.TemplateJoinDao; import com.cloud.api.query.vo.TemplateJoinVO; import com.cloud.dc.DataCenter; @@ -37,29 +66,9 @@ import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.User; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; -import org.apache.cloudstack.api.BaseCmd; -import org.apache.cloudstack.api.command.user.kubernetes.cluster.AddVirtualMachinesToKubernetesClusterCmd; -import org.apache.cloudstack.api.command.user.kubernetes.cluster.RemoveVirtualMachinesFromKubernetesClusterCmd; -import org.apache.cloudstack.context.CallContext; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.Spy; -import org.mockito.junit.MockitoJUnitRunner; - -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; @RunWith(MockitoJUnitRunner.class) public class KubernetesClusterManagerImplTest { @@ -89,26 +98,30 @@ public class KubernetesClusterManagerImplTest { @InjectMocks KubernetesClusterManagerImpl kubernetesClusterManager; + + @Mock + private Account account; + @Test public void testValidateVpcTierAllocated() { Network network = Mockito.mock(Network.class); - Mockito.when(network.getState()).thenReturn(Network.State.Allocated); + when(network.getState()).thenReturn(Network.State.Allocated); kubernetesClusterManager.validateVpcTier(network); } @Test(expected = InvalidParameterValueException.class) public void testValidateVpcTierDefaultDenyRule() { Network network = Mockito.mock(Network.class); - Mockito.when(network.getState()).thenReturn(Network.State.Implemented); - Mockito.when(network.getNetworkACLId()).thenReturn(NetworkACL.DEFAULT_DENY); + when(network.getState()).thenReturn(Network.State.Implemented); + when(network.getNetworkACLId()).thenReturn(NetworkACL.DEFAULT_DENY); kubernetesClusterManager.validateVpcTier(network); } @Test public void testValidateVpcTierValid() { Network network = Mockito.mock(Network.class); - Mockito.when(network.getState()).thenReturn(Network.State.Implemented); - Mockito.when(network.getNetworkACLId()).thenReturn(NetworkACL.DEFAULT_ALLOW); + when(network.getState()).thenReturn(Network.State.Implemented); + when(network.getNetworkACLId()).thenReturn(NetworkACL.DEFAULT_ALLOW); kubernetesClusterManager.validateVpcTier(network); } @@ -117,7 +130,7 @@ public void validateIsolatedNetworkIpRulesNoRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(new ArrayList<>()); + when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(new ArrayList<>()); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -131,7 +144,7 @@ public void validateIsolatedNetworkIpRulesNoConflictingRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(80, 80), createRule(443, 443))); + when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(80, 80), createRule(443, 443))); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -140,7 +153,7 @@ public void validateIsolatedNetworkIpRulesApiConflictingRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(6440, 6445), createRule(443, 443))); + when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(6440, 6445), createRule(443, 443))); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -149,7 +162,7 @@ public void validateIsolatedNetworkIpRulesSshConflictingRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(2200, KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT), createRule(443, 443))); + when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(2200, KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT), createRule(443, 443))); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -158,7 +171,7 @@ public void validateIsolatedNetworkIpRulesNearConflictingRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(2220, 2221), createRule(2225, 2227), createRule(6440, 6442), createRule(6444, 6446))); + when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(2220, 2221), createRule(2225, 2227), createRule(6440, 6442), createRule(6444, 6446))); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -171,7 +184,7 @@ public void testValidateKubernetesClusterScaleSizeNullNewSizeNoError() { public void testValidateKubernetesClusterScaleSizeSameNewSizeNoError() { Long size = 2L; KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getNodeCount()).thenReturn(size); + when(clusterVO.getNodeCount()).thenReturn(size); kubernetesClusterManager.validateKubernetesClusterScaleSize(clusterVO, size, 100, Mockito.mock(DataCenter.class)); } @@ -179,8 +192,8 @@ public void testValidateKubernetesClusterScaleSizeSameNewSizeNoError() { public void testValidateKubernetesClusterScaleSizeStoppedCluster() { Long size = 2L; KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getNodeCount()).thenReturn(size); - Mockito.when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Stopped); + when(clusterVO.getNodeCount()).thenReturn(size); + when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Stopped); kubernetesClusterManager.validateKubernetesClusterScaleSize(clusterVO, 3L, 100, Mockito.mock(DataCenter.class)); } @@ -188,57 +201,57 @@ public void testValidateKubernetesClusterScaleSizeStoppedCluster() { public void testValidateKubernetesClusterScaleSizeZeroNewSize() { Long size = 2L; KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); - Mockito.when(clusterVO.getNodeCount()).thenReturn(size); + when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); + when(clusterVO.getNodeCount()).thenReturn(size); kubernetesClusterManager.validateKubernetesClusterScaleSize(clusterVO, 0L, 100, Mockito.mock(DataCenter.class)); } @Test(expected = InvalidParameterValueException.class) public void testValidateKubernetesClusterScaleSizeOverMaxSize() { KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); - Mockito.when(clusterVO.getControlNodeCount()).thenReturn(1L); + when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); + when(clusterVO.getControlNodeCount()).thenReturn(1L); kubernetesClusterManager.validateKubernetesClusterScaleSize(clusterVO, 4L, 4, Mockito.mock(DataCenter.class)); } @Test public void testValidateKubernetesClusterScaleSizeDownscaleNoError() { KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); - Mockito.when(clusterVO.getControlNodeCount()).thenReturn(1L); - Mockito.when(clusterVO.getNodeCount()).thenReturn(4L); + when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); + when(clusterVO.getControlNodeCount()).thenReturn(1L); + when(clusterVO.getNodeCount()).thenReturn(4L); kubernetesClusterManager.validateKubernetesClusterScaleSize(clusterVO, 2L, 10, Mockito.mock(DataCenter.class)); } @Test(expected = InvalidParameterValueException.class) public void testValidateKubernetesClusterScaleSizeUpscaleDeletedTemplate() { KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); - Mockito.when(clusterVO.getControlNodeCount()).thenReturn(1L); - Mockito.when(clusterVO.getNodeCount()).thenReturn(2L); - Mockito.when(templateDao.findById(Mockito.anyLong())).thenReturn(null); + when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); + when(clusterVO.getControlNodeCount()).thenReturn(1L); + when(clusterVO.getNodeCount()).thenReturn(2L); + when(templateDao.findById(Mockito.anyLong())).thenReturn(null); kubernetesClusterManager.validateKubernetesClusterScaleSize(clusterVO, 4L, 10, Mockito.mock(DataCenter.class)); } @Test(expected = InvalidParameterValueException.class) public void testValidateKubernetesClusterScaleSizeUpscaleNotInZoneTemplate() { KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); - Mockito.when(clusterVO.getControlNodeCount()).thenReturn(1L); - Mockito.when(clusterVO.getNodeCount()).thenReturn(2L); - Mockito.when(templateDao.findById(Mockito.anyLong())).thenReturn(Mockito.mock(VMTemplateVO.class)); - Mockito.when(templateJoinDao.newTemplateView(Mockito.any(VMTemplateVO.class), Mockito.anyLong(), Mockito.anyBoolean())).thenReturn(null); + when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); + when(clusterVO.getControlNodeCount()).thenReturn(1L); + when(clusterVO.getNodeCount()).thenReturn(2L); + when(templateDao.findById(Mockito.anyLong())).thenReturn(Mockito.mock(VMTemplateVO.class)); + when(templateJoinDao.newTemplateView(Mockito.any(VMTemplateVO.class), Mockito.anyLong(), Mockito.anyBoolean())).thenReturn(null); kubernetesClusterManager.validateKubernetesClusterScaleSize(clusterVO, 4L, 10, Mockito.mock(DataCenter.class)); } @Test public void testValidateKubernetesClusterScaleSizeUpscaleNoError() { KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); - Mockito.when(clusterVO.getControlNodeCount()).thenReturn(1L); - Mockito.when(clusterVO.getNodeCount()).thenReturn(2L); - Mockito.when(templateDao.findById(Mockito.anyLong())).thenReturn(Mockito.mock(VMTemplateVO.class)); - Mockito.when(templateJoinDao.newTemplateView(Mockito.any(VMTemplateVO.class), Mockito.anyLong(), Mockito.anyBoolean())).thenReturn(List.of(Mockito.mock(TemplateJoinVO.class))); + when(clusterVO.getState()).thenReturn(KubernetesCluster.State.Running); + when(clusterVO.getControlNodeCount()).thenReturn(1L); + when(clusterVO.getNodeCount()).thenReturn(2L); + when(templateDao.findById(Mockito.anyLong())).thenReturn(Mockito.mock(VMTemplateVO.class)); + when(templateJoinDao.newTemplateView(Mockito.any(VMTemplateVO.class), Mockito.anyLong(), Mockito.anyBoolean())).thenReturn(List.of(Mockito.mock(TemplateJoinVO.class))); kubernetesClusterManager.validateKubernetesClusterScaleSize(clusterVO, 4L, 10, Mockito.mock(DataCenter.class)); } @@ -247,7 +260,7 @@ public void testValidateKubernetesClusterScaleSizeUpscaleNoError() { public void setUp() throws Exception { CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); overrideDefaultConfigValue(KubernetesClusterService.KubernetesServiceEnabled, "_defaultValue", "true"); - Mockito.doNothing().when(accountManager).checkAccess( + doNothing().when(accountManager).checkAccess( Mockito.any(Account.class), Mockito.any(), Mockito.anyBoolean(), Mockito.any()); } @@ -269,13 +282,13 @@ public void addVmsToCluster() { AddVirtualMachinesToKubernetesClusterCmd cmd = Mockito.mock(AddVirtualMachinesToKubernetesClusterCmd.class); List vmIds = Arrays.asList(1L, 2L, 3L); - Mockito.when(cmd.getId()).thenReturn(1L); - Mockito.when(cmd.getVmIds()).thenReturn(vmIds); - Mockito.when(cmd.getActualCommandName()).thenReturn(BaseCmd.getCommandNameByClass(RemoveVirtualMachinesFromKubernetesClusterCmd.class)); - Mockito.when(cluster.getClusterType()).thenReturn(KubernetesCluster.ClusterType.ExternalManaged); - Mockito.when(vmInstanceDao.findById(Mockito.anyLong())).thenReturn(vm); - Mockito.when(kubernetesClusterDao.findById(Mockito.anyLong())).thenReturn(cluster); - Mockito.when(kubernetesClusterVmMapDao.listByClusterIdAndVmIdsIn(1L, vmIds)).thenReturn(Collections.emptyList()); + when(cmd.getId()).thenReturn(1L); + when(cmd.getVmIds()).thenReturn(vmIds); + when(cmd.getActualCommandName()).thenReturn(BaseCmd.getCommandNameByClass(RemoveVirtualMachinesFromKubernetesClusterCmd.class)); + when(cluster.getClusterType()).thenReturn(KubernetesCluster.ClusterType.ExternalManaged); + when(vmInstanceDao.findById(Mockito.anyLong())).thenReturn(vm); + when(kubernetesClusterDao.findById(Mockito.anyLong())).thenReturn(cluster); + when(kubernetesClusterVmMapDao.listByClusterIdAndVmIdsIn(1L, vmIds)).thenReturn(Collections.emptyList()); Assert.assertTrue(kubernetesClusterManager.addVmsToCluster(cmd)); } @@ -285,11 +298,53 @@ public void removeVmsFromCluster() { RemoveVirtualMachinesFromKubernetesClusterCmd cmd = Mockito.mock(RemoveVirtualMachinesFromKubernetesClusterCmd.class); List vmIds = Arrays.asList(1L, 2L, 3L); - Mockito.when(cmd.getId()).thenReturn(1L); - Mockito.when(cmd.getVmIds()).thenReturn(vmIds); - Mockito.when(cmd.getActualCommandName()).thenReturn(BaseCmd.getCommandNameByClass(RemoveVirtualMachinesFromKubernetesClusterCmd.class)); - Mockito.when(cluster.getClusterType()).thenReturn(KubernetesCluster.ClusterType.ExternalManaged); - Mockito.when(kubernetesClusterDao.findById(Mockito.anyLong())).thenReturn(cluster); + when(cmd.getId()).thenReturn(1L); + when(cmd.getVmIds()).thenReturn(vmIds); + when(cmd.getActualCommandName()).thenReturn(BaseCmd.getCommandNameByClass(RemoveVirtualMachinesFromKubernetesClusterCmd.class)); + when(cluster.getClusterType()).thenReturn(KubernetesCluster.ClusterType.ExternalManaged); + when(kubernetesClusterDao.findById(Mockito.anyLong())).thenReturn(cluster); Assert.assertTrue(kubernetesClusterManager.removeVmsFromCluster(cmd).size() > 0); } + + @Test + public void testCleanupForAccount_NoClusters() { + when(account.getId()).thenReturn(123L); + when(kubernetesClusterDao.listForCleanupByAccount(123L)).thenReturn(Collections.emptyList()); + + kubernetesClusterManager.cleanupForAccount(account); + + verify(kubernetesClusterDao).listForCleanupByAccount(123L); + verifyNoMoreInteractions(kubernetesClusterDao); + } + + @Test + public void testCleanupForAccount_SuccessfulCleanup() { + KubernetesClusterVO cluster = Mockito.mock(KubernetesClusterVO.class); + when(account.getId()).thenReturn(123L); + when(kubernetesClusterDao.listForCleanupByAccount(123L)).thenReturn(List.of(cluster)); + + doReturn(true).when(kubernetesClusterManager).destroyKubernetesCluster(cluster); + + kubernetesClusterManager.cleanupForAccount(account); + + verify(kubernetesClusterDao).listForCleanupByAccount(123L); + verify(kubernetesClusterManager).destroyKubernetesCluster(cluster); + } + + @Test + public void testCleanupForAccount_ClusterCleanupFails() { + KubernetesClusterVO cluster = Mockito.mock(KubernetesClusterVO.class); + when(account.getId()).thenReturn(123L); + when(cluster.getName()).thenReturn("failed-cluster"); + when(kubernetesClusterDao.listForCleanupByAccount(123L)).thenReturn(List.of(cluster)); + + doThrow(new CloudRuntimeException("Error destroying cluster")) + .when(kubernetesClusterManager) + .destroyKubernetesCluster(cluster); + + kubernetesClusterManager.cleanupForAccount(account); + + verify(kubernetesClusterDao).listForCleanupByAccount(123L); + verify(kubernetesClusterManager).destroyKubernetesCluster(cluster); + } } From 5a7d4a8c329b24895917b7bdb0c8dd5d9e4a64b9 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Sat, 15 Mar 2025 14:04:05 +0530 Subject: [PATCH 4/4] fix no bean found Signed-off-by: Abhishek Kumar --- .../java/com/cloud/user/AccountManagerImpl.java | 16 +++++++++++++--- .../cloud/user/AccountManagetImplTestBase.java | 3 --- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 464b866fb342..f629aeb49b6f 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -85,6 +85,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import com.cloud.api.ApiDBUtils; import com.cloud.api.auth.SetupUserTwoFactorAuthenticationCmd; @@ -172,6 +173,7 @@ import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; +import com.cloud.utils.component.ComponentContext; import com.cloud.utils.component.Manager; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.component.PluggableService; @@ -302,8 +304,6 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M private UserDataDao userDataDao; @Inject private NetworkPermissionDao networkPermissionDao; - @Inject - KubernetesServiceHelper kubernetesServiceHelper; private List _querySelectors; @@ -851,6 +851,16 @@ public boolean deleteAccount(AccountVO account, long callerUserId, Account calle return cleanupAccount(account, callerUserId, caller); } + protected void cleanupPluginsResourcesIfNeeded(Account account) { + try { + KubernetesServiceHelper kubernetesServiceHelper = + ComponentContext.getDelegateComponentOfType(KubernetesServiceHelper.class); + kubernetesServiceHelper.cleanupForAccount(account); + } catch (NoSuchBeanDefinitionException ignored) { + s_logger.debug("No KubernetesServiceHelper bean found"); + } + } + protected boolean cleanupAccount(AccountVO account, long callerUserId, Account caller) { long accountId = account.getId(); boolean accountCleanupNeeded = false; @@ -932,7 +942,7 @@ protected boolean cleanupAccount(AccountVO account, long callerUserId, Account c } } - kubernetesServiceHelper.cleanupForAccount(account); + cleanupPluginsResourcesIfNeeded(account); // Destroy the account's VMs List vms = _userVmDao.listByAccountId(accountId); diff --git a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java b/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java index 771f9e480547..2fa18221a940 100644 --- a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java +++ b/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java @@ -24,7 +24,6 @@ import com.cloud.dc.dao.DedicatedResourceDao; import com.cloud.domain.dao.DomainDao; import com.cloud.event.dao.UsageEventDao; -import com.cloud.kubernetes.cluster.KubernetesServiceHelper; import com.cloud.network.as.AutoScaleManager; import com.cloud.network.dao.AccountGuestVlanMapDao; import com.cloud.network.dao.IPAddressDao; @@ -199,8 +198,6 @@ public class AccountManagetImplTestBase { UserDataDao userDataDao; @Mock NetworkPermissionDao networkPermissionDaoMock; - @Mock - KubernetesServiceHelper kubernetesServiceHelper; @Spy @InjectMocks