Skip to content

CKS: preserve node offerings during nodeids scale-in #14459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andrijapanicsb wants to merge 1 commit into
base: main
Choose a base branch
from
+165 −20
Open

CKS: preserve node offerings during nodeids scale-in #14459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions ...etes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2283,7 +2283,7 @@ public boolean scaleKubernetesCluster(ScaleKubernetesClusterCmd cmd) throws Clou
}
validateKubernetesClusterScaleParameters(cmd);
KubernetesClusterVO kubernetesCluster = kubernetesClusterDao.findById(cmd.getId());
Map<String, ServiceOffering> nodeToOfferingMap = createNodeTypeToServiceOfferingMap(cmd.getServiceOfferingNodeTypeMap(), cmd.getServiceOfferingId(), kubernetesCluster);
Map<String, ServiceOffering> nodeToOfferingMap = createNodeTypeToServiceOfferingMap(cmd.getServiceOfferingNodeTypeMap(), cmd.getServiceOfferingId());

String[] keys = getServiceUserKeys(kubernetesCluster);
KubernetesClusterScaleWorker scaleWorker =
Expand All @@ -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<String, ServiceOffering> createNodeTypeToServiceOfferingMap(Map<String, Long> idsMapping,
Long serviceOfferingId, KubernetesClusterVO kubernetesCluster) {
protected Map<String, ServiceOffering> createNodeTypeToServiceOfferingMap(Map<String, Long> idsMapping, Long serviceOfferingId) {
Map<String, ServiceOffering> 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) {
Expand Down
7 changes: 4 additions & 3 deletions ...rc/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Long, Long> clusterCountAndCapacity = calculateNewClusterCountAndCapacity(newWorkerSize, nodeType, serviceOffering);
Expand Down Expand Up @@ -394,7 +394,7 @@ private void scaleKubernetesClusterOffering(KubernetesClusterNodeType nodeType,
kubernetesCluster = updateKubernetesClusterEntryForNodeType(null, nodeType, serviceOffering, updateNodeOffering, updateClusterOffering);
}

private void removeNodesFromCluster(List<KubernetesClusterVmMapVO> vmMaps) throws CloudRuntimeException {
protected void removeNodesFromCluster(List<KubernetesClusterVmMapVO> vmMaps) throws CloudRuntimeException {
for (KubernetesClusterVmMapVO vmMapVO : vmMaps) {
UserVmVO userVM = userVmDao.findById(vmMapVO.getVmId());
logger.info("Removing vm {} from cluster {}", userVM, kubernetesCluster);
Expand Down Expand Up @@ -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;
}

Expand Down
37 changes: 28 additions & 9 deletions ...-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, ServiceOffering> 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<String, ServiceOffering> 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<String, ServiceOffering> mapping = kubernetesClusterManager.createNodeTypeToServiceOfferingMap(new HashMap<>(), 1L);

Assert.assertEquals(Collections.singletonMap(DEFAULT.name(), offering), mapping);
}

@Test
Expand All @@ -432,7 +451,7 @@ public void testCreateNodeTypeToServiceOfferingMap() {
ServiceOfferingVO controlOffering = Mockito.mock(ServiceOfferingVO.class);
Mockito.when(serviceOfferingDao.findById(controlOfferingId)).thenReturn(controlOffering);

Map<String, ServiceOffering> mapping = kubernetesClusterManager.createNodeTypeToServiceOfferingMap(idsMap, null, null);
Map<String, ServiceOffering> 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()));
Expand Down
126 changes: 126 additions & 0 deletions ...est/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorkerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -54,6 +61,8 @@ public class KubernetesClusterScaleWorkerTest {
private KubernetesClusterVmMapDao kubernetesClusterVmMapDao;
@Mock
private UserVmDao userVmDao;
@Mock
private UserVmManager userVmManager;

private KubernetesClusterScaleWorker worker;

Expand All @@ -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;
Expand Down
Loading