andrijapanicsb commented on code in PR #14223:
URL: https://github.com/apache/cloudstack/pull/14223#discussion_r4394969622
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java:
##########
@@ -229,32 +236,36 @@ private KubernetesClusterVO
updateKubernetesClusterEntryForNodeType(final Long n
}
protected Pair<Long, Long> calculateNewClusterCountAndCapacity(Long
newWorkerSize, KubernetesClusterNodeType nodeType, ServiceOffering
serviceOffering) {
- long cores;
- long memory;
- long totalClusterSize = newWorkerSize == null ?
kubernetesCluster.getTotalNodeCount() : (newWorkerSize +
kubernetesCluster.getControlNodeCount() + kubernetesCluster.getEtcdNodeCount());
-
- if (nodeType == DEFAULT) {
- cores = serviceOffering.getCpu() * totalClusterSize;
- memory = serviceOffering.getRamSize() * totalClusterSize;
- } else {
- long nodeCount = getNodeCountForType(nodeType, kubernetesCluster);
- Long existingOfferingId =
getExistingOfferingIdForNodeType(nodeType, kubernetesCluster);
- if (existingOfferingId == null) {
- existingOfferingId = serviceOffering.getId();
- }
- ServiceOfferingVO previousOffering =
serviceOfferingDao.findById(existingOfferingId);
- Pair<Long, Long> previousNodesCapacity =
calculateNodesCapacity(previousOffering, nodeCount);
- if (WORKER == nodeType) {
- nodeCount = newWorkerSize == null ?
kubernetesCluster.getNodeCount() : newWorkerSize;
+ long cores = 0;
+ long memory = 0;
+ // VM offerings have already changed here. Recompute the total from the
+ // configured node types instead of subtracting a supposedly old VM
offering.
+ for (KubernetesClusterNodeType type : List.of(CONTROL, WORKER, ETCD)) {
Review Comment:
Done, the capacity sum now uses `CLUSTER_NODES_TYPES_LIST`. I kept the
CONTROL, ETCD, WORKER order in the scale loop itself, since the order matters
there.
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java:
##########
@@ -229,32 +236,36 @@ private KubernetesClusterVO
updateKubernetesClusterEntryForNodeType(final Long n
}
protected Pair<Long, Long> calculateNewClusterCountAndCapacity(Long
newWorkerSize, KubernetesClusterNodeType nodeType, ServiceOffering
serviceOffering) {
- long cores;
- long memory;
- long totalClusterSize = newWorkerSize == null ?
kubernetesCluster.getTotalNodeCount() : (newWorkerSize +
kubernetesCluster.getControlNodeCount() + kubernetesCluster.getEtcdNodeCount());
-
- if (nodeType == DEFAULT) {
- cores = serviceOffering.getCpu() * totalClusterSize;
- memory = serviceOffering.getRamSize() * totalClusterSize;
- } else {
- long nodeCount = getNodeCountForType(nodeType, kubernetesCluster);
- Long existingOfferingId =
getExistingOfferingIdForNodeType(nodeType, kubernetesCluster);
- if (existingOfferingId == null) {
- existingOfferingId = serviceOffering.getId();
- }
- ServiceOfferingVO previousOffering =
serviceOfferingDao.findById(existingOfferingId);
- Pair<Long, Long> previousNodesCapacity =
calculateNodesCapacity(previousOffering, nodeCount);
- if (WORKER == nodeType) {
- nodeCount = newWorkerSize == null ?
kubernetesCluster.getNodeCount() : newWorkerSize;
+ long cores = 0;
+ long memory = 0;
+ // VM offerings have already changed here. Recompute the total from the
+ // configured node types instead of subtracting a supposedly old VM
offering.
+ for (KubernetesClusterNodeType type : List.of(CONTROL, WORKER, ETCD)) {
+ long nodeCount = WORKER == type && newWorkerSize != null ?
newWorkerSize : getNodeCountForType(type, kubernetesCluster);
+ if (nodeCount == 0) {
+ continue;
}
- Pair<Long, Long> newNodesCapacity =
calculateNodesCapacity(serviceOffering, nodeCount);
- Pair<Long, Long> newClusterCapacity =
calculateClusterNewCapacity(kubernetesCluster, previousNodesCapacity,
newNodesCapacity);
- cores = newClusterCapacity.first();
- memory = newClusterCapacity.second();
+ ServiceOffering offering = nodeType == DEFAULT || nodeType == type
? serviceOffering :
+
serviceOfferingDao.findByIdIncludingRemoved(getConfiguredOfferingIdForNodeType(type));
Review Comment:
Done. The per-type to cluster fallback is in one place now
(`getServiceOfferingIdForNodeTypeOnCluster`), and
`getServiceOfferingForNodeTypeOnCluster` uses it.
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java:
##########
@@ -229,32 +236,36 @@ private KubernetesClusterVO
updateKubernetesClusterEntryForNodeType(final Long n
}
protected Pair<Long, Long> calculateNewClusterCountAndCapacity(Long
newWorkerSize, KubernetesClusterNodeType nodeType, ServiceOffering
serviceOffering) {
- long cores;
- long memory;
- long totalClusterSize = newWorkerSize == null ?
kubernetesCluster.getTotalNodeCount() : (newWorkerSize +
kubernetesCluster.getControlNodeCount() + kubernetesCluster.getEtcdNodeCount());
-
- if (nodeType == DEFAULT) {
- cores = serviceOffering.getCpu() * totalClusterSize;
- memory = serviceOffering.getRamSize() * totalClusterSize;
- } else {
- long nodeCount = getNodeCountForType(nodeType, kubernetesCluster);
- Long existingOfferingId =
getExistingOfferingIdForNodeType(nodeType, kubernetesCluster);
- if (existingOfferingId == null) {
- existingOfferingId = serviceOffering.getId();
- }
- ServiceOfferingVO previousOffering =
serviceOfferingDao.findById(existingOfferingId);
- Pair<Long, Long> previousNodesCapacity =
calculateNodesCapacity(previousOffering, nodeCount);
- if (WORKER == nodeType) {
- nodeCount = newWorkerSize == null ?
kubernetesCluster.getNodeCount() : newWorkerSize;
+ long cores = 0;
+ long memory = 0;
+ // VM offerings have already changed here. Recompute the total from the
+ // configured node types instead of subtracting a supposedly old VM
offering.
+ for (KubernetesClusterNodeType type : List.of(CONTROL, WORKER, ETCD)) {
+ long nodeCount = WORKER == type && newWorkerSize != null ?
newWorkerSize : getNodeCountForType(type, kubernetesCluster);
+ if (nodeCount == 0) {
+ continue;
}
- Pair<Long, Long> newNodesCapacity =
calculateNodesCapacity(serviceOffering, nodeCount);
- Pair<Long, Long> newClusterCapacity =
calculateClusterNewCapacity(kubernetesCluster, previousNodesCapacity,
newNodesCapacity);
- cores = newClusterCapacity.first();
- memory = newClusterCapacity.second();
+ ServiceOffering offering = nodeType == DEFAULT || nodeType == type
? serviceOffering :
+
serviceOfferingDao.findByIdIncludingRemoved(getConfiguredOfferingIdForNodeType(type));
+ Pair<Long, Long> capacity = calculateNodesCapacity(offering,
nodeCount);
+ cores += capacity.first();
+ memory += capacity.second();
}
return new Pair<>(cores, memory);
}
+ private Long getConfiguredOfferingIdForNodeType(KubernetesClusterNodeType
nodeType) {
+ Long offeringId = null;
+ if (WORKER == nodeType) {
+ offeringId = kubernetesCluster.getWorkerNodeServiceOfferingId();
+ } else if (CONTROL == nodeType) {
+ offeringId = kubernetesCluster.getControlNodeServiceOfferingId();
+ } else if (ETCD == nodeType) {
+ offeringId = kubernetesCluster.getEtcdNodeServiceOfferingId();
+ }
+ return offeringId == null ? kubernetesCluster.getServiceOfferingId() :
offeringId;
+ }
Review Comment:
Agreed, the copy is gone. One difference: for the VM's current offering and
for the cluster totals I now look the offering up including removed ones, so
the totals no longer break when an offering has been deleted. Other callers
keep their current lookup. A missing offering ID now fails with a clear
message, and the offerings of all node types in the request are checked before
any VM is touched.
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java:
##########
@@ -376,24 +377,171 @@ private void
scaleKubernetesClusterOffering(KubernetesClusterNodeType nodeType,
List<KubernetesClusterVmMapVO> vmList =
kubernetesClusterVmMapDao.listByClusterIdAndVmType(kubernetesCluster.getId(),
nodeType);
final long tobeScaledVMCount = Math.min(vmList.size(), size);
for (long i = 0; i < tobeScaledVMCount; i++) {
- KubernetesClusterVmMapVO vmMapVO = vmList.get((int) i);
- UserVmVO userVM = userVmDao.findById(vmMapVO.getVmId());
- boolean result = false;
- try {
- result = userVmManager.upgradeVirtualMachine(userVM.getId(),
serviceOffering.getId(), new HashMap<String, String>());
- } catch (RuntimeException | ResourceUnavailableException |
ManagementServerException | VirtualMachineMigrationException e) {
- logTransitStateAndThrow(Level.ERROR, String.format("Scaling
Kubernetes cluster : %s failed, unable to scale cluster VM : %s due to %s",
kubernetesCluster.getName(), userVM.getDisplayName(), e.getMessage()),
kubernetesCluster.getId(), KubernetesCluster.Event.OperationFailed, e);
- }
- if (!result) {
- logTransitStateAndThrow(Level.WARN, String.format("Scaling
Kubernetes cluster : %s failed, unable to scale cluster VM : %s",
kubernetesCluster.getName(),
userVM.getDisplayName()),kubernetesCluster.getId(),
KubernetesCluster.Event.OperationFailed);
- }
+ new NodeOfferingResize(vmList.get((int) i), nodeType,
serviceOffering).execute();
Review Comment:
I kept the single scale timeout, the same way the upgrade worker uses its
timeout, so the setting keeps its meaning. To make it go further, the repeated
cordon/uncordon polling is gone, and the time is now checked before each node
starts instead of after it finishes, so a node that already succeeded is no
longer reported as a timeout. Once the time is up, no further node is started
and kubelet is not restarted. If you'd still prefer a separate per-node budget,
I can add it as its own setting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
