From 3ecf1918f4fa28439a22c1a705fc2f4dcd2d29c2 Mon Sep 17 00:00:00 2001 From: andrijapanicsb Date: Wed, 21 Oct 2026 22:19:55 +0200 Subject: [PATCH] fix(cks): preserve node offerings during nodeids scale-in Do not infer a global resize when offering parameters are omitted. Keep node-type offerings in count-only accounting and continue processing explicit autoscaling changes without a resize. Add regression coverage for selected-worker removal, mixed and legacy offerings, and autoscaling-only requests. Fixes #14458 --- .../cluster/KubernetesClusterManagerImpl.java | 15 +-- .../KubernetesClusterScaleWorker.java | 7 +- .../KubernetesClusterManagerImplTest.java | 37 +++-- .../KubernetesClusterScaleWorkerTest.java | 126 ++++++++++++++++++ 4 files changed, 165 insertions(+), 20 deletions(-) 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 eb4ebb78ee48..51c762c6e9b2 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 @@ -2283,7 +2283,7 @@ public boolean scaleKubernetesCluster(ScaleKubernetesClusterCmd cmd) throws Clou } validateKubernetesClusterScaleParameters(cmd); KubernetesClusterVO kubernetesCluster = kubernetesClusterDao.findById(cmd.getId()); - Map nodeToOfferingMap = createNodeTypeToServiceOfferingMap(cmd.getServiceOfferingNodeTypeMap(), cmd.getServiceOfferingId(), kubernetesCluster); + Map nodeToOfferingMap = createNodeTypeToServiceOfferingMap(cmd.getServiceOfferingNodeTypeMap(), cmd.getServiceOfferingId()); String[] keys = getServiceUserKeys(kubernetesCluster); KubernetesClusterScaleWorker scaleWorker = @@ -2302,16 +2302,15 @@ public boolean scaleKubernetesCluster(ScaleKubernetesClusterCmd cmd) throws Clou /** * Creates a map for the requested node type service offering - * For the node type DEFAULT: Every node is scaled to the same offering + * For the node type DEFAULT: Every node is scaled to the same explicitly requested offering. + * Omitting offering parameters preserves the existing offerings during size-only operations. */ - protected Map createNodeTypeToServiceOfferingMap(Map idsMapping, - Long serviceOfferingId, KubernetesClusterVO kubernetesCluster) { + protected Map createNodeTypeToServiceOfferingMap(Map idsMapping, Long serviceOfferingId) { Map map = new HashMap<>(); if (MapUtils.isEmpty(idsMapping)) { - ServiceOfferingVO offering = serviceOfferingId != null ? - serviceOfferingDao.findById(serviceOfferingId) : - serviceOfferingDao.findById(kubernetesCluster.getServiceOfferingId()); - map.put(DEFAULT.name(), offering); + if (serviceOfferingId != null) { + map.put(DEFAULT.name(), serviceOfferingDao.findById(serviceOfferingId)); + } return map; } for (String key : CLUSTER_NODES_TYPES_LIST) { diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java index 003a852f3dbe..f2b8506dab3f 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java @@ -212,7 +212,7 @@ private KubernetesClusterVO updateKubernetesClusterEntryForNodeType(final Long n final ServiceOffering newServiceOffering, final boolean updateNodeOffering, boolean updateClusterOffering) throws CloudRuntimeException { final ServiceOffering serviceOffering = newServiceOffering == null ? - serviceOfferingDao.findById(kubernetesCluster.getServiceOfferingId()) : newServiceOffering; + getServiceOfferingForNodeTypeOnCluster(nodeType, kubernetesCluster) : newServiceOffering; final Long serviceOfferingId = newServiceOffering == null ? null : serviceOffering.getId(); Pair clusterCountAndCapacity = calculateNewClusterCountAndCapacity(newWorkerSize, nodeType, serviceOffering); @@ -394,7 +394,7 @@ private void scaleKubernetesClusterOffering(KubernetesClusterNodeType nodeType, kubernetesCluster = updateKubernetesClusterEntryForNodeType(null, nodeType, serviceOffering, updateNodeOffering, updateClusterOffering); } - private void removeNodesFromCluster(List vmMaps) throws CloudRuntimeException { + protected void removeNodesFromCluster(List vmMaps) throws CloudRuntimeException { for (KubernetesClusterVmMapVO vmMapVO : vmMaps) { UserVmVO userVM = userVmDao.findById(vmMapVO.getVmId()); logger.info("Removing vm {} from cluster {}", userVM, kubernetesCluster); @@ -605,7 +605,8 @@ public boolean scaleCluster() throws CloudRuntimeException { for (KubernetesClusterNodeType nodeType : Arrays.asList(CONTROL, ETCD, WORKER)) { boolean isWorkerNode = WORKER == nodeType; final long newVMRequired = (!isWorkerNode || clusterSize == null) ? 0 : clusterSize - originalClusterSize; - if (!scaleClusterDefaultOffering && !serviceOfferingNodeTypeMap.containsKey(nodeType.name()) && newVMRequired == 0) { + if (!scaleClusterDefaultOffering && !serviceOfferingNodeTypeMap.containsKey(nodeType.name()) && newVMRequired == 0 + && !(isWorkerNode && autoscalingChanged)) { continue; } 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 1fab5420c3c3..0e716d022d9b 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 @@ -57,7 +57,6 @@ import org.apache.cloudstack.api.response.KubernetesClusterResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; -import org.apache.commons.collections.MapUtils; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -408,15 +407,35 @@ public void testIsAnyNodeOfferingEmpty() { } @Test - public void testCreateNodeTypeToServiceOfferingMapNullMap() { - KubernetesClusterVO clusterVO = Mockito.mock(KubernetesClusterVO.class); - Mockito.when(clusterVO.getServiceOfferingId()).thenReturn(1L); + public void testMissingOfferingParametersDoNotRequestGlobalResize() { + Assert.assertTrue(kubernetesClusterManager.createNodeTypeToServiceOfferingMap(null, null).isEmpty()); + Mockito.verifyNoInteractions(serviceOfferingDao); + } + + @Test + public void testEmptyOfferingParametersDoNotRequestGlobalResize() { + Assert.assertTrue(kubernetesClusterManager.createNodeTypeToServiceOfferingMap(new HashMap<>(), null).isEmpty()); + Mockito.verifyNoInteractions(serviceOfferingDao); + } + + @Test + public void testExplicitGlobalOfferingStillRequestsGlobalResize() { + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(serviceOfferingDao.findById(1L)).thenReturn(offering); + + Map mapping = kubernetesClusterManager.createNodeTypeToServiceOfferingMap(null, 1L); + + Assert.assertEquals(Collections.singletonMap(DEFAULT.name(), offering), mapping); + } + + @Test + public void testExplicitGlobalOfferingWithEmptyNodeMap() { ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); Mockito.when(serviceOfferingDao.findById(1L)).thenReturn(offering); - Map mapping = kubernetesClusterManager.createNodeTypeToServiceOfferingMap(new HashMap<>(), null, clusterVO); - Assert.assertFalse(MapUtils.isEmpty(mapping)); - Assert.assertTrue(mapping.containsKey(DEFAULT.name())); - Assert.assertEquals(offering, mapping.get(DEFAULT.name())); + + Map mapping = kubernetesClusterManager.createNodeTypeToServiceOfferingMap(new HashMap<>(), 1L); + + Assert.assertEquals(Collections.singletonMap(DEFAULT.name(), offering), mapping); } @Test @@ -432,7 +451,7 @@ public void testCreateNodeTypeToServiceOfferingMap() { ServiceOfferingVO controlOffering = Mockito.mock(ServiceOfferingVO.class); Mockito.when(serviceOfferingDao.findById(controlOfferingId)).thenReturn(controlOffering); - Map mapping = kubernetesClusterManager.createNodeTypeToServiceOfferingMap(idsMap, null, null); + Map mapping = kubernetesClusterManager.createNodeTypeToServiceOfferingMap(idsMap, null); Assert.assertEquals(2, mapping.size()); Assert.assertTrue(mapping.containsKey(WORKER.name()) && mapping.containsKey(CONTROL.name())); Assert.assertEquals(workerOffering, mapping.get(WORKER.name())); diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorkerTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorkerTest.java index c9299bdbaa67..24687d849930 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorkerTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorkerTest.java @@ -19,12 +19,15 @@ import com.cloud.kubernetes.cluster.KubernetesCluster; import com.cloud.kubernetes.cluster.KubernetesClusterVmMapVO; import com.cloud.kubernetes.cluster.KubernetesClusterManagerImpl; +import com.cloud.kubernetes.cluster.KubernetesClusterVO; import com.cloud.kubernetes.cluster.dao.KubernetesClusterVmMapDao; import com.cloud.offering.ServiceOffering; +import com.cloud.network.Network; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.utils.Pair; import com.cloud.vm.UserVmVO; +import com.cloud.vm.UserVmManager; import com.cloud.vm.dao.UserVmDao; import org.junit.Assert; import org.junit.Before; @@ -34,12 +37,16 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; import java.util.Arrays; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.CONTROL; import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.DEFAULT; +import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.WORKER; @RunWith(MockitoJUnitRunner.class) public class KubernetesClusterScaleWorkerTest { @@ -54,6 +61,8 @@ public class KubernetesClusterScaleWorkerTest { private KubernetesClusterVmMapDao kubernetesClusterVmMapDao; @Mock private UserVmDao userVmDao; + @Mock + private UserVmManager userVmManager; private KubernetesClusterScaleWorker worker; @@ -67,6 +76,123 @@ public void setUp() { worker.userVmDao = userVmDao; } + @Test + public void testNodeIdsScaleInPreservesMixedOfferingsAndRemovesSelectedWorker() { + assertScaleInPreservesOfferings(true, false); + } + + @Test + public void testCountOnlyScaleInPreservesMixedOfferings() { + assertScaleInPreservesOfferings(false, false); + } + + @Test + public void testNodeIdsScaleInPreservesLegacyGlobalOffering() { + assertScaleInPreservesOfferings(true, true); + } + + private void assertScaleInPreservesOfferings(boolean selectedNode, boolean legacyOffering) { + Mockito.when(kubernetesCluster.getId()).thenReturn(7L); + Mockito.when(kubernetesCluster.getState()).thenReturn(KubernetesCluster.State.Running); + Mockito.when(kubernetesCluster.getNodeCount()).thenReturn(2L); + Mockito.when(kubernetesCluster.getTotalNodeCount()).thenReturn(3L); + Mockito.when(kubernetesCluster.getServiceOfferingId()).thenReturn(defaultOfferingId); + Mockito.when(kubernetesCluster.getMinSize()).thenReturn(null); + Mockito.when(kubernetesCluster.getMaxSize()).thenReturn(null); + Mockito.when(kubernetesCluster.getCores()).thenReturn(legacyOffering ? 6L : 9L); + Mockito.when(kubernetesCluster.getMemory()).thenReturn(legacyOffering ? 6144L : 11264L); + ServiceOfferingVO legacy = Mockito.mock(ServiceOfferingVO.class); + if (legacyOffering) { + Mockito.when(legacy.getCpu()).thenReturn(2); + Mockito.when(legacy.getRamSize()).thenReturn(2048); + } + Mockito.when(serviceOfferingDao.findById(defaultOfferingId)).thenReturn(legacy); + long workerOfferingId = legacyOffering ? defaultOfferingId : 12L; + Mockito.when(kubernetesCluster.getWorkerNodeServiceOfferingId()).thenReturn(legacyOffering ? null : workerOfferingId); + if (!legacyOffering) { + ServiceOfferingVO workerOffering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(workerOffering.getCpu()).thenReturn(3); + Mockito.when(workerOffering.getRamSize()).thenReturn(4096); + Mockito.when(serviceOfferingDao.findById(workerOfferingId)).thenReturn(workerOffering); + } + KubernetesClusterVmMapVO control = Mockito.mock(KubernetesClusterVmMapVO.class); + KubernetesClusterVmMapVO retained = Mockito.mock(KubernetesClusterVmMapVO.class); + KubernetesClusterVmMapVO removed = Mockito.mock(KubernetesClusterVmMapVO.class); + Mockito.when(retained.getVmId()).thenReturn(100L); + UserVmVO retainedVm = Mockito.mock(UserVmVO.class); + Mockito.when(retainedVm.getServiceOfferingId()).thenReturn(workerOfferingId); + Mockito.when(userVmDao.findById(100L)).thenReturn(retainedVm); + Mockito.when(kubernetesClusterVmMapDao.listByClusterId(7L)).thenReturn(Arrays.asList(control, retained, removed)); + Mockito.when(kubernetesClusterVmMapDao.listByClusterIdAndVmType(7L, WORKER)).thenReturn(Arrays.asList(retained, removed)); + KubernetesClusterScaleWorker scaleWorker = Mockito.spy(new KubernetesClusterScaleWorker(kubernetesCluster, + Collections.emptyMap(), selectedNode ? null : 1L, selectedNode ? List.of(101L) : null, null, null, null, clusterManager)); + scaleWorker.serviceOfferingDao = serviceOfferingDao; + scaleWorker.kubernetesClusterVmMapDao = kubernetesClusterVmMapDao; + scaleWorker.userVmDao = userVmDao; + scaleWorker.userVmManager = userVmManager; + Network network = Mockito.mock(Network.class); + ReflectionTestUtils.setField(scaleWorker, "network", network); + Mockito.doNothing().when(scaleWorker).init(); + Mockito.doReturn(new Pair<>("192.0.2.1", 2222)).when(scaleWorker).getKubernetesClusterServerIpSshPort(null); + Mockito.doReturn("192.0.2.1").when(scaleWorker).resolveApiPublicIpAddress(network); + Mockito.doReturn(true).when(scaleWorker).stateTransitTo(Mockito.anyLong(), Mockito.any()); + if (selectedNode) { + Mockito.doReturn(new ArrayList<>(List.of(removed))).when(scaleWorker).getKubernetesClusterVMMapsForNodes(List.of(101L)); + } else { + Mockito.doReturn(new ArrayList<>(List.of(removed))).when(scaleWorker).getWorkerNodesToRemove(); + } + Mockito.doNothing().when(scaleWorker).removeNodesFromCluster(Mockito.anyList()); + KubernetesClusterVO updatedCluster = Mockito.mock(KubernetesClusterVO.class); + Mockito.doReturn(updatedCluster).when(scaleWorker).updateKubernetesClusterEntry(Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean()); + + Assert.assertTrue(scaleWorker.scaleCluster()); + + Mockito.verify(scaleWorker).removeNodesFromCluster(List.of(removed)); + Mockito.verify(scaleWorker).updateKubernetesClusterEntry(legacyOffering ? 4L : 6L, legacyOffering ? 4096L : 7168L, + 1L, null, false, null, null, WORKER, false, false); + Mockito.verifyNoInteractions(userVmManager); + Mockito.verify(kubernetesClusterVmMapDao, Mockito.never()).listByClusterIdAndVmType(7L, CONTROL); + } + + @Test + public void testAutoscalingOnlyEnablePreservesOfferings() { + assertAutoscalingPreservesOfferings(false, true, false); + } + + @Test + public void testAutoscalingOnlyDisablePreservesOfferings() { + assertAutoscalingPreservesOfferings(true, false, false); + } + + @Test + public void testAutoscalingBoundsOnlyChangePreservesOfferings() { + assertAutoscalingPreservesOfferings(true, true, true); + } + + private void assertAutoscalingPreservesOfferings(boolean existingEnabled, boolean requestedEnabled, boolean boundsChange) { + Mockito.when(kubernetesCluster.getState()).thenReturn(KubernetesCluster.State.Running); + Mockito.when(kubernetesCluster.getAutoscalingEnabled()).thenReturn(existingEnabled); + if (boundsChange) { + Mockito.when(kubernetesCluster.getMinSize()).thenReturn(1L); + } + KubernetesClusterScaleWorker scaleWorker = Mockito.spy(new KubernetesClusterScaleWorker(kubernetesCluster, + Collections.emptyMap(), null, null, requestedEnabled, 2L, 5L, clusterManager)); + scaleWorker.serviceOfferingDao = serviceOfferingDao; + scaleWorker.userVmManager = userVmManager; + ServiceOfferingVO existingOffering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(serviceOfferingDao.findById(12L)).thenReturn(existingOffering); + Mockito.doNothing().when(scaleWorker).init(); + Mockito.doReturn(12L).when(scaleWorker).getExistingOfferingIdForNodeType(WORKER, kubernetesCluster); + Mockito.doReturn(true).when(scaleWorker).autoscaleCluster(requestedEnabled, 2L, 5L); + Mockito.doReturn(true).when(scaleWorker).stateTransitTo(Mockito.anyLong(), Mockito.any()); + + Assert.assertTrue(scaleWorker.scaleCluster()); + + Mockito.verify(scaleWorker).autoscaleCluster(requestedEnabled, 2L, 5L); + Mockito.verifyNoInteractions(userVmManager); + } + @Test public void testCalculateNewClusterCountAndCapacityAllNodesScaleSize() { long controlNodes = 3L;