Simplify logic for scaling CKS cluster

nvazquez · nvazquez · commit e673c8bdd3c9 · 2025-06-20T18:07:03.000-03:00
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
@@ -532,6 +532,8 @@ public boolean scaleCluster() throws CloudRuntimeException {
         }
         scaleTimeoutTime = System.currentTimeMillis() + KubernetesClusterService.KubernetesClusterScaleTimeout.value() * 1000;
         final long originalClusterSize = kubernetesCluster.getNodeCount();
+
+        // DEFAULT node type means only the global service offering has been set for the Kubernetes cluster
         boolean scaleClusterDefaultOffering = serviceOfferingNodeTypeMap.containsKey(DEFAULT.name());
         if (scaleClusterDefaultOffering) {
             final ServiceOffering existingServiceOffering = serviceOfferingDao.findById(kubernetesCluster.getServiceOfferingId());
@@ -542,44 +544,40 @@ public boolean scaleCluster() throws CloudRuntimeException {
             }
         }
 
-        checkScalingKubernetesClusterOfferingsPerNodeType(serviceOfferingNodeTypeMap, kubernetesCluster);
-
         final boolean autoscalingChanged = isAutoscalingChanged();
         ServiceOffering defaultServiceOffering = serviceOfferingNodeTypeMap.getOrDefault(DEFAULT.name(), null);
 
         for (KubernetesClusterNodeType nodeType : Arrays.asList(CONTROL, ETCD, WORKER)) {
-            boolean isWorkerNodeOrAllNodes = WORKER == nodeType;
-            final long newVMRequired = (!isWorkerNodeOrAllNodes || clusterSize == null) ? 0 : clusterSize - originalClusterSize;
+            boolean isWorkerNode = WORKER == nodeType;
+            final long newVMRequired = (!isWorkerNode || clusterSize == null) ? 0 : clusterSize - originalClusterSize;
             if (!scaleClusterDefaultOffering && !serviceOfferingNodeTypeMap.containsKey(nodeType.name()) && newVMRequired == 0) {
                 continue;
             }
 
-            Long existingNodeTypeOfferingId = getKubernetesClusterNodeTypeOfferingId(kubernetesCluster, nodeType);
-            boolean clusterHasExistingOfferingForNodeType = existingNodeTypeOfferingId != null;
-            boolean serviceOfferingScalingNeeded = isServiceOfferingScalingNeededForNodeType(nodeType, serviceOfferingNodeTypeMap, kubernetesCluster);
-            ServiceOffering serviceOffering = serviceOfferingNodeTypeMap.getOrDefault(nodeType.name(), defaultServiceOffering);
-            boolean updateNodeOffering = serviceOfferingNodeTypeMap.containsKey(nodeType.name()) ||
-                    scaleClusterDefaultOffering && clusterHasExistingOfferingForNodeType;
-            boolean updateClusterOffering = isWorkerNodeOrAllNodes && scaleClusterDefaultOffering;
-            if (isWorkerNodeOrAllNodes && autoscalingChanged) {
+            ServiceOffering existingServiceOffering = getExistingServiceOfferingForNodeType(nodeType, kubernetesCluster);
+            ServiceOffering scalingServiceOffering = serviceOfferingNodeTypeMap.getOrDefault(nodeType.name(), defaultServiceOffering);
+            boolean isNodeOfferingScalingNeeded = isServiceOfferingScalingNeededForNodeType(existingServiceOffering, scalingServiceOffering);
+
+            boolean updateClusterOffering = isWorkerNode && scaleClusterDefaultOffering;
+            if (isWorkerNode && autoscalingChanged) {
                 boolean autoScaled = autoscaleCluster(this.isAutoscalingEnabled, minSize, maxSize);
-                if (autoScaled && serviceOfferingScalingNeeded) {
-                    scaleKubernetesClusterOffering(nodeType, serviceOffering, updateNodeOffering, updateClusterOffering);
+                if (autoScaled && isNodeOfferingScalingNeeded) {
+                    scaleKubernetesClusterOffering(nodeType, scalingServiceOffering, true, updateClusterOffering);
                 }
                 stateTransitTo(kubernetesCluster.getId(), KubernetesCluster.Event.OperationSucceeded);
                 return autoScaled;
             }
-            final boolean clusterSizeScalingNeeded = isWorkerNodeOrAllNodes && clusterSize != null && clusterSize != originalClusterSize;
-            if (serviceOfferingScalingNeeded && clusterSizeScalingNeeded) {
+            final boolean clusterSizeScalingNeeded = isWorkerNode && clusterSize != null && clusterSize != originalClusterSize;
+            if (isNodeOfferingScalingNeeded && clusterSizeScalingNeeded) {
                 if (newVMRequired > 0) {
-                    scaleKubernetesClusterOffering(nodeType, serviceOffering, updateNodeOffering, updateClusterOffering);
+                    scaleKubernetesClusterOffering(nodeType, scalingServiceOffering, true, updateClusterOffering);
                     scaleKubernetesClusterSize(nodeType);
                 } else {
                     scaleKubernetesClusterSize(nodeType);
-                    scaleKubernetesClusterOffering(nodeType, serviceOffering, updateNodeOffering, updateClusterOffering);
+                    scaleKubernetesClusterOffering(nodeType, scalingServiceOffering, true, updateClusterOffering);
                 }
-            } else if (serviceOfferingScalingNeeded) {
-                scaleKubernetesClusterOffering(nodeType, serviceOffering, updateNodeOffering, updateClusterOffering);
+            } else if (isNodeOfferingScalingNeeded) {
+                scaleKubernetesClusterOffering(nodeType, scalingServiceOffering, true, updateClusterOffering);
             } else if (clusterSizeScalingNeeded) {
                 scaleKubernetesClusterSize(nodeType);
             }
@@ -589,6 +587,20 @@ public boolean scaleCluster() throws CloudRuntimeException {
         return true;
     }
 
+    private ServiceOffering getExistingServiceOfferingForNodeType(KubernetesClusterNodeType nodeType, KubernetesCluster kubernetesCluster) {
+        Long existingOfferingId = getExistingOfferingIdForNodeType(nodeType, kubernetesCluster);
+        if (existingOfferingId == null) {
+            logAndThrow(Level.ERROR, String.format("The Kubernetes cluster %s does not have a service offering set for node type %s",
+                    kubernetesCluster.getName(), nodeType.name()));
+        }
+        ServiceOffering existingOffering = serviceOfferingDao.findById(existingOfferingId);
+        if (existingOffering == null) {
+            logAndThrow(Level.ERROR, String.format("Cannot find service offering with ID %s set on the Kubernetes cluster %s node type %s",
+                    existingOfferingId, kubernetesCluster.getName(), nodeType.name()));
+        }
+        return existingOffering;
+    }
+
     protected void compareExistingToScalingServiceOfferingForNodeType(Long existingOfferingId, Long scalingOfferingId,
                                                                       KubernetesClusterNodeType nodeType) {
         if (existingOfferingId.equals(scalingOfferingId)) {
@@ -599,65 +611,21 @@ protected void compareExistingToScalingServiceOfferingForNodeType(Long existingO
         }
     }
 
-    protected void checkScalingKubernetesClusterOfferingsPerNodeType(Map<String, ServiceOffering> scalingNodeTypeMap,
-                                                                     KubernetesCluster kubernetesCluster) {
-        for (KubernetesClusterNodeType nodeType : Arrays.asList(WORKER, CONTROL, ETCD)) {
-            Long existingNodeTypeOfferingId = getKubernetesClusterNodeTypeOfferingId(kubernetesCluster, nodeType);
-            if (existingNodeTypeOfferingId == null) {
-                existingNodeTypeOfferingId = kubernetesCluster.getServiceOfferingId();
-            }
-            if (ETCD == nodeType && (kubernetesCluster.getEtcdNodeCount() == null || kubernetesCluster.getEtcdNodeCount() == 0)) {
-                continue;
-            }
-            String scalingMapKey = scalingNodeTypeMap.containsKey(nodeType.name()) ? nodeType.name() : DEFAULT.name();
-            ServiceOffering scalingServiceOffering = scalingNodeTypeMap.get(scalingMapKey);
-            if (scalingServiceOffering == null) {
-                String err = String.format("Cannot find a service offering to scale the nodes of type %s", nodeType.name());
-                logger.error(err);
-                throw new CloudRuntimeException(err);
-            }
-            compareExistingToScalingServiceOfferingForNodeType(existingNodeTypeOfferingId,
-                    scalingServiceOffering.getId(), nodeType);
-        }
-    }
-
-    private Long getKubernetesClusterNodeTypeOfferingId(KubernetesCluster kubernetesCluster, KubernetesClusterNodeType nodeType) {
-        if (nodeType == WORKER) {
-            return kubernetesCluster.getWorkerNodeServiceOfferingId();
-        } else if (nodeType == ETCD) {
-            return kubernetesCluster.getEtcdNodeServiceOfferingId();
-        } else if (nodeType == CONTROL) {
-            return kubernetesCluster.getControlNodeServiceOfferingId();
-        }
-        return kubernetesCluster.getServiceOfferingId();
-    }
-
-    protected boolean isServiceOfferingScalingNeededForNodeType(KubernetesClusterNodeType nodeType,
-                                                                Map<String, ServiceOffering> map, KubernetesCluster kubernetesCluster) {
-        // DEFAULT node type means only the global service offering has been set for the Kubernetes cluster
-        Long existingOfferingId = map.containsKey(DEFAULT.name()) ?
-                kubernetesCluster.getServiceOfferingId() :
-                getExistingOfferingIdForNodeType(nodeType, kubernetesCluster);
-        if (existingOfferingId == null) {
-            logAndThrow(Level.ERROR, String.format("The Kubernetes cluster %s does not have a global service offering set", kubernetesCluster.getName()));
-        }
-        ServiceOffering existingOffering = serviceOfferingDao.findById(existingOfferingId);
-        if (existingOffering == null) {
-            logAndThrow(Level.ERROR, String.format("Cannot find the global service offering with ID %s set on the Kubernetes cluster %s", existingOfferingId, kubernetesCluster.getName()));
-        }
-        ServiceOffering newOffering = map.containsKey(DEFAULT.name()) ? map.get(DEFAULT.name()) : map.get(nodeType.name());
-        return newOffering != null && newOffering.getId() != existingOffering.getId();
+    protected boolean isServiceOfferingScalingNeededForNodeType(ServiceOffering existingServiceOffering,
+                                                                ServiceOffering scalingServiceOffering) {
+        return scalingServiceOffering != null && existingServiceOffering != null &&
+                scalingServiceOffering.getId() != existingServiceOffering.getId();
     }
 
     protected Long getExistingOfferingIdForNodeType(KubernetesClusterNodeType nodeType, KubernetesCluster kubernetesCluster) {
         List<KubernetesClusterVmMapVO> clusterVms = kubernetesClusterVmMapDao.listByClusterIdAndVmType(kubernetesCluster.getId(), nodeType);
         if (CollectionUtils.isEmpty(clusterVms)) {
-            return null;
+            return kubernetesCluster.getServiceOfferingId();
         }
         KubernetesClusterVmMapVO clusterVm = clusterVms.get(0);
         UserVmVO clusterUserVm = userVmDao.findById(clusterVm.getVmId());
         if (clusterUserVm == null) {
-            return null;
+            return kubernetesCluster.getServiceOfferingId();
         }
         return clusterUserVm.getServiceOfferingId();
     }
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
@@ -24,7 +24,6 @@
 import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.utils.Pair;
-import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.dao.UserVmDao;
 import org.junit.Assert;
@@ -35,14 +34,10 @@
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.DEFAULT;
 import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.CONTROL;
-import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.ETCD;
-import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.WORKER;
 
 @RunWith(MockitoJUnitRunner.class)
 public class KubernetesClusterScaleWorkerTest {
@@ -130,120 +125,4 @@ public void testCalculateNewClusterCountAndCapacityNodeTypeScaleControlOffering(
         Assert.assertEquals(expectedCores, newClusterCapacity.first().longValue());
         Assert.assertEquals(expectedMemory, newClusterCapacity.second().longValue());
     }
-
-    private KubernetesCluster createExistingKubernetesClusterForTesting(Long defaultOfferingId, Long workerOfferingId,
-                                                                        Long controlOfferingId) {
-        KubernetesCluster kubernetesClusterMock = Mockito.mock(KubernetesCluster.class);
-        Mockito.when(kubernetesClusterMock.getServiceOfferingId()).thenReturn(defaultOfferingId);
-        Mockito.when(kubernetesClusterMock.getWorkerNodeServiceOfferingId()).thenReturn(workerOfferingId);
-        Mockito.when(kubernetesClusterMock.getControlNodeServiceOfferingId()).thenReturn(controlOfferingId);
-        return kubernetesClusterMock;
-    }
-
-    private ServiceOffering createServiceOfferingForTesting(Long offeringId) {
-        ServiceOffering cksOffering = Mockito.mock(ServiceOffering.class);
-        Mockito.when(cksOffering.getId()).thenReturn(offeringId);
-        return cksOffering;
-    }
-
-    @Test
-    public void testCheckScalingKubernetesClusterOfferingsPerNodeTypeToNewDefaultOffering() {
-        Long cksOfferingId = 20L;
-
-        ServiceOffering newCksOffering = createServiceOfferingForTesting(21L);
-
-        KubernetesCluster kubernetesClusterMock = createExistingKubernetesClusterForTesting(cksOfferingId,
-                null, null);
-
-        Map<String, ServiceOffering> scalingMap = new HashMap<>();
-        scalingMap.put(DEFAULT.name(), newCksOffering);
-
-        worker.checkScalingKubernetesClusterOfferingsPerNodeType(scalingMap, kubernetesClusterMock);
-    }
-
-    @Test(expected = CloudRuntimeException.class)
-    public void testCheckScalingKubernetesClusterOfferingsPerNodeTypeToSameDefaultOffering() {
-        Long cksOfferingId = 20L;
-        ServiceOffering cksOffering = createServiceOfferingForTesting(cksOfferingId);
-
-        KubernetesCluster kubernetesClusterMock = createExistingKubernetesClusterForTesting(cksOfferingId,
-                null, null);
-
-        Map<String, ServiceOffering> scalingMap = new HashMap<>();
-        scalingMap.put(DEFAULT.name(), cksOffering);
-
-        worker.checkScalingKubernetesClusterOfferingsPerNodeType(scalingMap, kubernetesClusterMock);
-    }
-
-    @Test
-    public void testCheckScalingKubernetesClusterOfferingsPerNodeTypeWorkerAndControlNodes() {
-        Long cksOfferingId = 20L;
-        Long workerOfferingId = 21L;
-        Long controlOfferingId = 22L;
-        ServiceOffering scalingWorkerOffering = createServiceOfferingForTesting(30L);
-        ServiceOffering scalingControlOffering = createServiceOfferingForTesting(31L);
-
-        KubernetesCluster kubernetesClusterMock = createExistingKubernetesClusterForTesting(cksOfferingId,
-                workerOfferingId, controlOfferingId);
-
-        Map<String, ServiceOffering> scalingMap = new HashMap<>();
-        scalingMap.put(WORKER.name(), scalingWorkerOffering);
-        scalingMap.put(CONTROL.name(), scalingControlOffering);
-
-        worker.checkScalingKubernetesClusterOfferingsPerNodeType(scalingMap, kubernetesClusterMock);
-    }
-
-    @Test(expected = CloudRuntimeException.class)
-    public void testCheckScalingKubernetesClusterOfferingsPerNodeTypeWorkerAndControlNodesUseSameOffering() {
-        Long cksOfferingId = 20L;
-        Long workerOfferingId = 21L;
-        Long controlOfferingId = 22L;
-        ServiceOffering scalingWorkerOffering = createServiceOfferingForTesting(30L);
-        ServiceOffering controlOffering = createServiceOfferingForTesting(controlOfferingId);
-
-        KubernetesCluster kubernetesClusterMock = createExistingKubernetesClusterForTesting(cksOfferingId,
-                workerOfferingId, controlOfferingId);
-
-        Map<String, ServiceOffering> scalingMap = new HashMap<>();
-        scalingMap.put(WORKER.name(), scalingWorkerOffering);
-        scalingMap.put(CONTROL.name(), controlOffering);
-
-        worker.checkScalingKubernetesClusterOfferingsPerNodeType(scalingMap, kubernetesClusterMock);
-    }
-
-    @Test
-    public void testCheckScalingKubernetesClusterOfferingsPerNodeTypeWorkerAndControlNodesByDefaultOffering() {
-        Long cksOfferingId = 20L;
-        Long workerOfferingId = 21L;
-        Long controlOfferingId = 22L;
-        ServiceOffering scalingOffering = createServiceOfferingForTesting(30L);
-
-        KubernetesCluster kubernetesClusterMock = createExistingKubernetesClusterForTesting(cksOfferingId,
-                workerOfferingId, controlOfferingId);
-
-        Map<String, ServiceOffering> scalingMap = new HashMap<>();
-        scalingMap.put(DEFAULT.name(), scalingOffering);
-
-        worker.checkScalingKubernetesClusterOfferingsPerNodeType(scalingMap, kubernetesClusterMock);
-    }
-
-    @Test
-    public void testCheckScalingKubernetesClusterOfferingsPerNodeTypeWorkerEtcdAndControlNodes() {
-        Long cksOfferingId = 20L;
-        Long workerOfferingId = 21L;
-        Long controlOfferingId = 22L;
-        Long etcdOfferingId = 23L;
-        ServiceOffering scalingOffering = createServiceOfferingForTesting(30L);
-
-        KubernetesCluster kubernetesClusterMock = createExistingKubernetesClusterForTesting(cksOfferingId,
-                workerOfferingId, controlOfferingId);
-        Mockito.when(kubernetesClusterMock.getEtcdNodeServiceOfferingId()).thenReturn(etcdOfferingId);
-
-        Map<String, ServiceOffering> scalingMap = new HashMap<>();
-        scalingMap.put(WORKER.name(), scalingOffering);
-        scalingMap.put(CONTROL.name(), scalingOffering);
-        scalingMap.put(ETCD.name(), scalingOffering);
-
-        worker.checkScalingKubernetesClusterOfferingsPerNodeType(scalingMap, kubernetesClusterMock);
-    }
 }
