Skip to content

Commit 8a8b89e

Browse files
CKS: fix node drain on scale-down and upgrade, make a failed upgrade resumable
Scale-down and upgrade both drain a node before touching it, each with its own copy of the drain loop, and neither works well: - Scale-down gives up at once when kubectl drain exits non-zero (a PodDisruptionBudget that does not allow an eviction yet, say), and when it does retry after an exception it counts the attempt twice. The node is left cordoned, and the cluster goes to Alert although nothing has been removed and every node still works. - Upgrade retries straight away, with no wait for the eviction to become possible, and on the last failure leaves the node cordoned. - In both, kubectl drain has no --timeout, so it is cut off by a fixed 60s SSH timeout rather than a deadline of its own. Both workers now use one drain helper in KubernetesClusterActionWorker: one count per attempt, a retry (after a wait) on a non-zero exit or an exception, kubectl drain --timeout from the new setting cloud.kubernetes.cluster.node.drain.timeout (300s), cut down so that no attempt runs past the operation's deadline, with the SSH wait derived from it, and the end of the drain output in the error. The caller uncordons the node, and only if it cordoned it. Scale-down cordons every node it is going to remove (leaving alone the ones that were cordoned already), drains them, and if one cannot be drained uncordons them all and puts the cluster back in the state it was in, with an error that names the node, instead of Alert; the same happens if the scale times out before every node is drained. Once they are drained, the removal is not stopped part way by the timeout any more. If removing a node fails, the nodes still in the cluster are uncordoned. The upgrade had two more problems: - It only waited for the first control node to be Ready, so the next node could be drained while the previous one was still coming back. It now waits for every node, after it has been uncordoned. - It reached node i on sshPort + i. The port-forwarding rules are not created in the order of the upgrade's VM list (manually upgraded nodes are left out of that list, and nodes added later get the next free port), so the script could run on the wrong node. It now looks up the node's own SSH port-forwarding rule, as the remove worker already does. A failed upgrade left the cluster in Alert with no way to finish it, as upgradeKubernetesCluster only accepts a Running cluster. An upgrade now records the version it is upgrading to as a cluster detail (an existing key/value table) and removes it once the cluster is Running or Stopped again. upgradeKubernetesCluster accepts a cluster in Alert (Alert -> Upgrading) when that detail is the requested version (or another one with the same semantic version), which is a real upgrade of the cluster's own version, and all of the cluster's VMs are running, and refuses any other Alert cluster with a clear message. For an upgrade that failed before this change, a cluster still at the old version with nodes recorded at the target version (kubernetesnodeversion, which is set as each node is done) is accepted too. Nodes that are already at the target version, both as recorded and as reported by kubectl get node, and that are Ready, are skipped. The first control node is still upgraded first; if it is already done, kubeadm upgrade apply is not run again. The node version check now compares versions exactly, so 1.28.1 no longer matches 1.28.10, line by line, as the SSH output can hold stderr too, and ignoring semver build metadata. Failure messages name the node the upgrade stopped on. The upgrade stops if it can't move the cluster to Upgrading, and the Alert state scanner no longer marks a cluster Running when it couldn't move it to Recovering, and a failed upgrade detaches its ISO before going to Alert, so a retry and the scanner or the failing job don't undo each other's work. No schema change. Signed-off-by: Brad House <bhouse@nexthop.ai> (cherry picked from commit 0dcaa8f)
1 parent 3c8049c commit 8a8b89e

13 files changed

Lines changed: 929 additions & 102 deletions

File tree

‎api/src/main/java/com/cloud/kubernetes/cluster/KubernetesCluster.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ enum State {
104104
s_fsm.addTransition(State.ScalingStoppedCluster, Event.OperationFailed, State.Alert);
105105

106106
s_fsm.addTransition(State.Running, Event.UpgradeRequested, State.Upgrading);
107+
s_fsm.addTransition(State.Alert, Event.UpgradeRequested, State.Upgrading);
107108
s_fsm.addTransition(State.Upgrading, Event.OperationSucceeded, State.Running);
108109
s_fsm.addTransition(State.Upgrading, Event.OperationFailed, State.Alert);
109110

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@
131131
import org.apache.commons.collections.MapUtils;
132132
import org.apache.commons.lang3.ObjectUtils;
133133
import org.apache.commons.lang3.StringUtils;
134+
import org.apache.commons.lang3.math.NumberUtils;
134135
import org.apache.logging.log4j.Level;
135136

136137
import com.cloud.api.ApiDBUtils;
@@ -175,6 +176,7 @@
175176
import com.cloud.kubernetes.cluster.actionworkers.KubernetesClusterStopWorker;
176177
import com.cloud.kubernetes.cluster.actionworkers.KubernetesClusterUpgradeWorker;
177178
import com.cloud.kubernetes.cluster.dao.KubernetesClusterAffinityGroupMapDao;
179+
import com.cloud.kubernetes.cluster.utils.KubernetesClusterUtil;
178180
import com.cloud.kubernetes.cluster.dao.KubernetesClusterDao;
179181
import com.cloud.kubernetes.cluster.dao.KubernetesClusterDetailsDao;
180182
import com.cloud.kubernetes.cluster.dao.KubernetesClusterVmMapDao;
@@ -1570,6 +1572,40 @@ protected boolean isAnyNodeOfferingEmpty(Map<String, Long> map) {
15701572
return map.values().stream().anyMatch(Objects::isNull);
15711573
}
15721574

1575+
/**
1576+
* Tells whether upgrading a cluster in Alert state to a version retries an upgrade to that version that failed.
1577+
* An upgrade records the version it is upgrading to as a detail of the cluster, which is removed once the cluster
1578+
* is Running again.
1579+
* For an upgrade that failed before that detail was recorded, the nodes' versions are used: the version of the cluster is only changed once every node has been upgraded, while the version recorded for
1580+
* each node is changed as soon as that node has been upgraded. So, a cluster that is still at an older version
1581+
* but has nodes at the target version is one whose upgrade to that version stopped part way. Nodes that are
1582+
* upgraded manually are left out, as they may be at any version.
1583+
*/
1584+
protected boolean isRetryOfFailedUpgrade(KubernetesCluster kubernetesCluster, KubernetesSupportedVersion upgradeVersion) {
1585+
if (Objects.equals(upgradeVersion.getId(), kubernetesCluster.getKubernetesVersionId())) {
1586+
return false;
1587+
}
1588+
// Another version with the same semantic version as the cluster's is not an upgrade either
1589+
KubernetesSupportedVersionVO clusterVersion = kubernetesSupportedVersionDao.findById(kubernetesCluster.getKubernetesVersionId());
1590+
if (clusterVersion == null || KubernetesClusterUtil.isSameNodeVersion(clusterVersion.getSemanticVersion(), upgradeVersion.getSemanticVersion())) {
1591+
return false;
1592+
}
1593+
KubernetesClusterDetailsVO upgradeDetail = kubernetesClusterDetailsDao.findDetail(kubernetesCluster.getId(), KubernetesClusterUpgradeWorker.UPGRADE_VERSION_DETAIL);
1594+
if (upgradeDetail != null) {
1595+
if (String.valueOf(upgradeVersion.getId()).equals(upgradeDetail.getValue())) {
1596+
return true;
1597+
}
1598+
// Another version with the same semantic version, for example one registered with a fixed ISO
1599+
KubernetesSupportedVersionVO failedVersion = NumberUtils.isDigits(upgradeDetail.getValue()) ?
1600+
kubernetesSupportedVersionDao.findByIdIncludingRemoved(Long.parseLong(upgradeDetail.getValue())) : null;
1601+
return failedVersion != null && KubernetesClusterUtil.isSameNodeVersion(failedVersion.getSemanticVersion(), upgradeVersion.getSemanticVersion());
1602+
}
1603+
List<KubernetesClusterVmMapVO> vmMaps = kubernetesClusterVmMapDao.listByClusterId(kubernetesCluster.getId());
1604+
return vmMaps != null && vmMaps.stream()
1605+
.filter(vmMap -> !vmMap.isManualUpgrade())
1606+
.anyMatch(vmMap -> KubernetesClusterUtil.isSameNodeVersion(vmMap.getNodeVersion(), upgradeVersion.getSemanticVersion()));
1607+
}
1608+
15731609
private void validateKubernetesClusterUpgradeParameters(UpgradeKubernetesClusterCmd cmd) {
15741610
// Validate parameters
15751611
ApiServiceConfiguration.validateEndpointUrl();
@@ -1590,7 +1626,8 @@ private void validateKubernetesClusterUpgradeParameters(UpgradeKubernetesCluster
15901626
throw new InvalidParameterValueException(String.format("Upgrade kubernetes cluster is not supported for an externally managed cluster (%s)", kubernetesCluster.getName()));
15911627
}
15921628
accountManager.checkAccess(CallContext.current().getCallingAccount(), SecurityChecker.AccessType.OperateEntry, false, kubernetesCluster);
1593-
if (!KubernetesCluster.State.Running.equals(kubernetesCluster.getState())) {
1629+
boolean inAlert = KubernetesCluster.State.Alert.equals(kubernetesCluster.getState());
1630+
if (!inAlert && !KubernetesCluster.State.Running.equals(kubernetesCluster.getState())) {
15941631
throw new InvalidParameterValueException(String.format("Kubernetes cluster : %s is not in running state", kubernetesCluster.getName()));
15951632
}
15961633
final DataCenter zone = dataCenterDao.findById(kubernetesCluster.getZoneId());
@@ -1604,6 +1641,16 @@ private void validateKubernetesClusterUpgradeParameters(UpgradeKubernetesCluster
16041641
if (!KubernetesSupportedVersion.State.Enabled.equals(upgradeVersion.getState())) {
16051642
throw new InvalidParameterValueException(String.format("Kubernetes version ID: %s for upgrade is in %s state", upgradeVersion.getUuid(), upgradeVersion.getState()));
16061643
}
1644+
if (inAlert) {
1645+
if (!isRetryOfFailedUpgrade(kubernetesCluster, upgradeVersion)) {
1646+
throw new InvalidParameterValueException(String.format("Kubernetes cluster : %s is in Alert state. It can be upgraded in that state only " +
1647+
"to retry an upgrade that failed, to the same Kubernetes version as that upgrade", kubernetesCluster.getName()));
1648+
}
1649+
if (!isClusterVMsInDesiredState(kubernetesCluster, VirtualMachine.State.Running)) {
1650+
throw new InvalidParameterValueException(String.format("Kubernetes cluster : %s is in Alert state and not all of its VMs are running. " +
1651+
"Start them before retrying the upgrade", kubernetesCluster.getName()));
1652+
}
1653+
}
16071654
KubernetesSupportedVersionVO clusterVersion = kubernetesSupportedVersionDao.findById(kubernetesCluster.getKubernetesVersionId());
16081655
if (clusterVersion == null || clusterVersion.getRemoved() != null) {
16091656
throw new InvalidParameterValueException(String.format("Invalid Kubernetes version associated with cluster : %s",
@@ -1640,7 +1687,13 @@ private void validateKubernetesClusterUpgradeParameters(UpgradeKubernetesCluster
16401687
protected boolean stateTransitTo(long kubernetesClusterId, KubernetesCluster.Event e) {
16411688
KubernetesClusterVO kubernetesCluster = kubernetesClusterDao.findById(kubernetesClusterId);
16421689
try {
1643-
return _stateMachine.transitTo(kubernetesCluster, e, null, kubernetesClusterDao);
1690+
boolean transited = _stateMachine.transitTo(kubernetesCluster, e, null, kubernetesClusterDao);
1691+
if (transited && (KubernetesCluster.State.Running.equals(kubernetesCluster.getState()) ||
1692+
KubernetesCluster.State.Stopped.equals(kubernetesCluster.getState()))) {
1693+
// As in KubernetesClusterActionWorker.stateTransitTo: no longer in Alert because of a failed upgrade
1694+
kubernetesClusterDetailsDao.removeDetail(kubernetesClusterId, KubernetesClusterUpgradeWorker.UPGRADE_VERSION_DETAIL);
1695+
}
1696+
return transited;
16441697
} catch (NoTransitionException nte) {
16451698
logger.warn("Failed to transition state of the Kubernetes cluster: {} in state {} on event {}", kubernetesCluster, kubernetesCluster.getState().toString(), e.toString(), nte);
16461699
return false;
@@ -3135,6 +3188,7 @@ public ConfigKey<?>[] getConfigKeys() {
31353188
KubernetesClusterScaleTimeout,
31363189
KubernetesClusterUpgradeTimeout,
31373190
KubernetesClusterUpgradeRetries,
3191+
KubernetesClusterNodeDrainTimeout,
31383192
KubernetesClusterAddNodeTimeout,
31393193
KubernetesClusterRemoveNodeTimeout,
31403194
KubernetesClusterExperimentalFeaturesEnabled,

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,13 @@ public interface KubernetesClusterService extends PluggableService, Configurable
8888
"The number of retries if fail to upgrade kubernetes cluster due to some reasons (e.g. drain node, etcdserver leader changed)",
8989
true,
9090
KubernetesServiceEnabled.key());
91+
static final ConfigKey<Long> KubernetesClusterNodeDrainTimeout = new ConfigKey<Long>("Advanced", Long.class,
92+
"cloud.kubernetes.cluster.node.drain.timeout",
93+
"300",
94+
"Timeout interval (in seconds) for draining a Kubernetes node before it is removed from a cluster or upgraded. " +
95+
"A drain that does not finish in time, for example because a PodDisruptionBudget does not allow an eviction, is retried, and the node is uncordoned if it still fails. Must be at least 1, smaller values use the default",
96+
true,
97+
KubernetesServiceEnabled.key());
9198
static final ConfigKey<Long> KubernetesClusterAddNodeTimeout = new ConfigKey<Long>("Advanced", Long.class,
9299
"cloud.kubernetes.cluster.add.node.timeout",
93100
"3600",

0 commit comments

Comments
 (0)