Address more review comments

This commit is contained in:
nvazquez 2025-05-27 22:51:36 -03:00
parent 6264c2181b
commit bcf7b6df25
No known key found for this signature in database
GPG Key ID: 656E1BCC8CB54F84
4 changed files with 34 additions and 41 deletions

View File

@ -1323,7 +1323,11 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne
throw new InvalidParameterValueException("Failed to find service offering ID: " + serviceOfferingId);
}
checkServiceOfferingForNodesScale(serviceOffering, kubernetesCluster, clusterVersion);
final ServiceOffering existingServiceOffering = serviceOfferingDao.findById(kubernetesCluster.getServiceOfferingId());
Long nodeTypeOfferingId = getExistingServiceOfferingIdForNodeType(key, kubernetesCluster);
if (nodeTypeOfferingId == null) {
nodeTypeOfferingId = kubernetesCluster.getServiceOfferingId();
}
final ServiceOffering existingServiceOffering = serviceOfferingDao.findById(nodeTypeOfferingId);
if (KubernetesCluster.State.Running.equals(kubernetesCluster.getState()) && (serviceOffering.getRamSize() < existingServiceOffering.getRamSize() ||
serviceOffering.getCpu() * serviceOffering.getSpeed() < existingServiceOffering.getCpu() * existingServiceOffering.getSpeed())) {
logAndThrow(Level.WARN, String.format("Kubernetes cluster cannot be scaled down for service offering. Service offering : %s offers lesser resources as compared to service offering : %s of Kubernetes cluster : %s",
@ -1333,6 +1337,17 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne
}
}
private Long getExistingServiceOfferingIdForNodeType(String key, KubernetesClusterVO kubernetesCluster) {
if (key.equalsIgnoreCase(WORKER.name())) {
return kubernetesCluster.getWorkerServiceOfferingId();
} else if (key.equalsIgnoreCase(CONTROL.name())) {
return kubernetesCluster.getControlServiceOfferingId();
} else if (key.equalsIgnoreCase(ETCD.name())) {
return kubernetesCluster.getEtcdServiceOfferingId();
}
return kubernetesCluster.getServiceOfferingId();
}
protected void checkServiceOfferingForNodesScale(ServiceOffering serviceOffering, KubernetesClusterVO kubernetesCluster, KubernetesSupportedVersion clusterVersion) {
if (serviceOffering.isDynamic()) {
throw new InvalidParameterValueException(String.format("Custom service offerings are not supported for Kubernetes clusters. Kubernetes cluster : %s, service offering : %s", kubernetesCluster.getName(), serviceOffering.getName()));

View File

@ -852,7 +852,9 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
}
updatedCluster.setMinSize(minSize);
updatedCluster.setMaxSize(maxSize);
return kubernetesClusterDao.persist(updatedCluster);
kubernetesClusterDao.persist(updatedCluster);
// Prevent null attributes set by the createForUpdate method
return kubernetesClusterDao.findById(kubernetesCluster.getId());
});
}

View File

@ -35,6 +35,7 @@ import org.apache.cloudstack.api.ApiCommandResourceType;
import org.apache.cloudstack.api.InternalIdentity;
import org.apache.cloudstack.context.CallContext;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import com.cloud.dc.DataCenter;
@ -531,13 +532,14 @@ public class KubernetesClusterScaleWorker extends KubernetesClusterResourceModif
boolean hasDefaultOffering = serviceOfferingNodeTypeMap.containsKey(DEFAULT.name());
if (hasDefaultOffering) {
final ServiceOffering existingServiceOffering = serviceOfferingDao.findById(kubernetesCluster.getServiceOfferingId());
if (existingServiceOffering == null) {
final ServiceOffering existingControlOffering = serviceOfferingDao.findById(kubernetesCluster.getControlServiceOfferingId());
final ServiceOffering existingWorkerOffering = serviceOfferingDao.findById(kubernetesCluster.getWorkerServiceOfferingId());
if (existingServiceOffering == null && ObjectUtils.anyNull(existingControlOffering, existingWorkerOffering)) {
logAndThrow(Level.ERROR, String.format("Scaling Kubernetes cluster : %s failed, service offering for the Kubernetes cluster not found!", kubernetesCluster.getName()));
}
}
final boolean autoscalingChanged = isAutoscalingChanged();
Long existingDefaultOfferingId = kubernetesCluster.getServiceOfferingId();
ServiceOffering defaultServiceOffering = serviceOfferingNodeTypeMap.getOrDefault(DEFAULT.name(), null);
for (KubernetesClusterNodeType nodeType : Arrays.asList(CONTROL, ETCD, WORKER)) {
@ -547,7 +549,7 @@ public class KubernetesClusterScaleWorker extends KubernetesClusterResourceModif
continue;
}
boolean serviceOfferingScalingNeeded = isServiceOfferingScalingNeededForNodeType(nodeType, serviceOfferingNodeTypeMap, kubernetesCluster, existingDefaultOfferingId);
boolean serviceOfferingScalingNeeded = isServiceOfferingScalingNeededForNodeType(nodeType, serviceOfferingNodeTypeMap, kubernetesCluster);
ServiceOffering serviceOffering = serviceOfferingNodeTypeMap.getOrDefault(nodeType.name(), defaultServiceOffering);
boolean updateNodeOffering = serviceOfferingNodeTypeMap.containsKey(nodeType.name());
boolean updateClusterOffering = isWorkerNodeOrAllNodes && hasDefaultOffering;
@ -580,11 +582,8 @@ public class KubernetesClusterScaleWorker extends KubernetesClusterResourceModif
}
protected boolean isServiceOfferingScalingNeededForNodeType(KubernetesClusterNodeType nodeType,
Map<String, ServiceOffering> map, KubernetesCluster kubernetesCluster,
Long existingDefaultOfferingId) {
Long existingOfferingId = map.containsKey(DEFAULT.name()) ?
existingDefaultOfferingId :
getExistingOfferingIdForNodeType(nodeType, kubernetesCluster);
Map<String, ServiceOffering> map, KubernetesCluster kubernetesCluster) {
Long existingOfferingId = 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()));
}
@ -597,17 +596,15 @@ public class KubernetesClusterScaleWorker extends KubernetesClusterResourceModif
}
protected Long getExistingOfferingIdForNodeType(KubernetesClusterNodeType nodeType, KubernetesCluster kubernetesCluster) {
Long offeringId = null;
if (WORKER == nodeType) {
offeringId = kubernetesCluster.getWorkerServiceOfferingId();
} else if (CONTROL == nodeType) {
offeringId = kubernetesCluster.getControlServiceOfferingId();
} else if (ETCD == nodeType) {
offeringId = kubernetesCluster.getEtcdServiceOfferingId();
List<KubernetesClusterVmMapVO> clusterVms = kubernetesClusterVmMapDao.listByClusterIdAndVmType(kubernetesCluster.getId(), nodeType);
if (CollectionUtils.isEmpty(clusterVms)) {
return null;
}
if (offeringId == null) {
offeringId = kubernetesCluster.getServiceOfferingId();
KubernetesClusterVmMapVO clusterVm = clusterVms.get(0);
UserVmVO clusterUserVm = userVmDao.findById(clusterVm.getVmId());
if (clusterUserVm == null) {
return null;
}
return offeringId;
return clusterUserVm.getServiceOfferingId();
}
}

View File

@ -30,8 +30,6 @@ import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
import java.util.Map;
import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.DEFAULT;
import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.CONTROL;
@ -55,25 +53,6 @@ public class KubernetesClusterScaleWorkerTest {
worker.serviceOfferingDao = serviceOfferingDao;
}
@Test
public void testIsServiceOfferingScalingNeededForNodeTypeAllNodesSameOffering() {
ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
Map<String, ServiceOffering> map = Map.of(DEFAULT.name(), serviceOffering);
Mockito.when(serviceOfferingDao.findById(defaultOfferingId)).thenReturn(serviceOffering);
Assert.assertFalse(worker.isServiceOfferingScalingNeededForNodeType(DEFAULT, map, kubernetesCluster, defaultOfferingId));
}
@Test
public void testIsServiceOfferingScalingNeededForNodeTypeAllNodesDifferentOffering() {
ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
Mockito.when(serviceOffering.getId()).thenReturn(defaultOfferingId);
ServiceOfferingVO newOffering = Mockito.mock(ServiceOfferingVO.class);
Mockito.when(newOffering.getId()).thenReturn(4L);
Map<String, ServiceOffering> map = Map.of(DEFAULT.name(), newOffering);
Mockito.when(serviceOfferingDao.findById(defaultOfferingId)).thenReturn(serviceOffering);
Assert.assertTrue(worker.isServiceOfferingScalingNeededForNodeType(DEFAULT, map, kubernetesCluster, defaultOfferingId));
}
@Test
public void testCalculateNewClusterCountAndCapacityAllNodesScaleSize() {
long controlNodes = 3L;