[bhouse-nexthop]

Description

Fixes how a CKS cluster drains a node on scale-down and upgrade, and lets a failed upgrade be retried and pick up where it stopped.

Fixes #14610

The drain fixes and the resumable upgrade are in one PR because they change the same upgrade loop in KubernetesClusterUpgradeWorker: the drain, the Ready wait, the per-node SSH address and the skip of finished nodes are all steps of that loop, and splitting them would mean two PRs conflicting on every line of it.

Drain (scale-down and upgrade)

Both workers had their own drain loop, and neither handled a drain that fails or takes a while. The most common cause is a PodDisruptionBudget that doesn't allow an eviction yet.

• Scale-down gave up at once on a non-zero exit, counted an attempt twice after an exception, left the node cordoned, and sent the cluster to Alert. Nothing had been removed at that point.
• Upgrade retried with no wait and left the node cordoned on the last failure. It now uncordons it, and also when it times out after the drain, unless the node was cordoned before the upgrade.
• Both ran kubectl drain without --timeout, so a fixed 60s SSH timeout cut it off.

There is now one helper, KubernetesClusterActionWorker.drainKubernetesClusterNode:

• Each attempt counts once. A non-zero exit or an exception is retried after a 30s wait, unless that would go past the operation's deadline.
• It runs kubectl drain --timeout=<cloud.kubernetes.cluster.node.drain.timeout>s. The new setting defaults to 300 and is dynamic; values below 1 use the default.
• The timeout is cut down when less time is left before the operation's deadline, so that pods aren't evicted by an attempt that can't be used. The SSH wait is the drain timeout plus 60s.
• The error includes the end of kubectl's output, up to 2000 characters, as a drain blocked by a PodDisruptionBudget logs every retry.
• The helper leaves the node as it is. The caller uncordons it, and only if the caller cordoned it.
• Attempts: 3 on scale-down, as before; cloud.kubernetes.cluster.upgrade.retries + 1 on upgrade, as before.

Scale-down now cordons every node it is going to remove, then drains them one by one. A node that was cordoned already, for example by an operator, is left cordoned on a rollback. So is a node whose state can't be read. Cordoning them all first stops pods evicted from one being rescheduled onto another that is also going away.

• If any node can't be cordoned or drained, every cordoned node is uncordoned. The cluster goes back to the state it was in, and the error names the node. Nothing has been removed, so Alert would be wrong.
• After that, the nodes are deleted and their VMs destroyed as before. kubectl delete node gets its own retries, with --ignore-not-found so that a retry after a lost reply doesn't fail.
• If the scale times out before every node is drained, it is rolled back in the same way, with nothing removed. Once the nodes are drained, the timeout no longer stops the removal part way. Before, a timeout could leave some nodes removed and the node count not updated.
• If deleting a node or destroying its VM fails, the cluster goes to Alert as before. The drained nodes that are still in the cluster, and that were cordoned by the scale-down, are uncordoned, so they aren't left unschedulable.

Upgrade

• Ready check: every node is now waited on to be Ready after it is uncordoned, not only the first control node. Before, the next node could be drained while the previous one was still coming back.
• SSH address: the script for node i was run on sshPort + i. That only works when the upgrade's VM list and the port-forwarding rules are in the same order. They aren't when there are manually upgraded nodes, which are left out of the list, or nodes added later, which get the next free port. The script could then run on the wrong node. Each node is now reached through its own SSH port-forwarding rule, the same lookup KubernetesClusterRemoveWorker already uses. Shared networks, where nodes are reached on their own address, are unchanged.
• Error messages name the Kubernetes node as well as the VM.

Retrying a failed upgrade

A failed upgrade left the cluster in Alert. upgradeKubernetesCluster only accepted a Running cluster, so the upgrade could neither be retried nor finished. No schema change is needed to fix this.

• Recording the upgrade: an upgrade now records the id of the version it is upgrading to as a cluster detail, upgradeKubernetesVersionId, not for display. That detail lives in the existing kubernetes_cluster_details key/value table. It is removed once the cluster is Running or Stopped again, for example because the upgrade succeeded or the Alert state scanner recovered the cluster, so it can't make a later, unrelated Alert look like a failed upgrade.
• Accepting the retry: upgradeKubernetesCluster accepts a cluster in Alert when that detail is the requested version, or another version with the same semantic version (for example one registered with a fixed ISO), and all of the cluster's VMs are running. A version with the same semantic version as the cluster's own is never taken for a retry. There is a new transition Alert -UpgradeRequested-> Upgrading. Any other cluster in Alert is refused, with a message saying why.
• Upgrades that failed before this change: these have no detail, so the nodes' versions are used instead.
  • The cluster's version only changes once every node has been upgraded.
  • Each node's version (kubernetes_cluster_vm_map.kubernetes_node_version) changes as soon as that node has been upgraded, uncordoned and found Ready. It is already shown as kubernetesnodeversion in the cluster's VM list, so progress can be followed there.
  • So a cluster still at the old version with a node at the target, not counting manually upgraded nodes, had an upgrade to that version stop part way.
• Skipping finished nodes: a node is skipped when its recorded version is the target, kubectl get nodes also reports the target version for it, and it is Ready. The recorded value is never trusted on its own, and a node that went NotReady is upgraded again. A node that is Ready but was cordoned after its upgrade is skipped, and stays cordoned.
• Exact version match: the existing check after each node is upgraded now uses the same comparison. It is exact, so 1.28.1 no longer matches 1.28.10, and it works line by line, because the SSH output also holds stderr, for example a warning from sudo. Semver build metadata (+vendor.1) is ignored.
• Order: the first control node still comes first. If it was already done, it is skipped, and kubeadm upgrade apply doesn't run again. The other nodes only ever run kubeadm upgrade node.
• UI: the Upgrade action is now also shown for a cluster in Alert.
• Racing the Alert state scanner: the scanner tries to recover clusters in Alert, and could otherwise overwrite the state of an upgrade that has just started.
  • The upgrade now stops if it can't move the cluster to Upgrading.
  • reconcileAlertCluster no longer moves the cluster to Running unless it managed to move it to Recovering first.
  • A failed upgrade now detaches the binaries ISO before moving the cluster to Alert. Otherwise a quick retry could have its ISO detached by the job that failed.
  • These guards narrow the race, but they don't close it, because cluster state updates are not compare-and-set today (KubernetesClusterDaoImpl.updateState writes the new state without checking the old one). Stop concurrent CKS operations clobbering each other, recover stuck states #14609 adds compare-and-set state updates and stuck-state recovery, and closes this race. The two PRs are independent; whichever merges second needs a small rebase.

Overlap with #14604

#14604 ("Allow scaling a Kubernetes cluster that is in Alert") also changes KubernetesClusterScaleWorker.removeKubernetesClusterNode, to skip the drain for a VM that is down. Whichever of the two is merged second will need a small rebase. The skip moves into drainNodesToRemove here, so a down VM is neither cordoned nor drained.

The two don't depend on each other. When this PR rolls back a scale whose drain failed, a cluster that started in Alert goes back to Alert, not Running, so it stays correct once #14604 allows scaling from Alert. On 4.22 alone that branch is not reached.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• Build/CI
• Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

How Has This Been Tested?

Unit tests only. This has not been run against a live cluster or hypervisor.

New tests:

• KubernetesClusterActionWorkerTest:
  • the drain is retried after a non-zero exit and succeeds, with one wait and no uncordon;
  • an exception and then failures use every attempt and wait between them;
  • kubectl's timeout is cut down to the time left before the deadline, no attempt is made without time left, and there is no retry after an interrupted wait;
  • the exact kubectl drain --timeout=300s command and SSH wait are checked;
  • getNodeSshAddress uses the node's own port-forwarding rule on the cluster's public IP, ignores a rule that is being revoked, falls back to sshPort + index when there is none, and uses the private IP on a shared network.
• KubernetesClusterScaleWorkerTest:
  • every node is cordoned before any is drained;
  • when the second node can't be drained, the first is uncordoned (the second already was, by the helper), the cluster gets OperationSucceeded (back to Running), not OperationFailed, and the error names the node and includes kubectl's output;
  • a cluster that started in Alert goes back to Alert;
  • a scale that is past its deadline drains nothing, uncordons the nodes and rolls back;
  • a node that was cordoned before the scale-down, as reported even with a line of stderr, is not uncordoned by it, and neither is a node whose state can't be read.
• KubernetesClusterUpgradeWorkerTest: a node is skipped only when both its recorded and its reported version are the target and it is Ready, also when it is cordoned. It is not skipped when only the recorded one is, when the node is NotReady, when the node can't be reached, when the recorded version is older, or when the versions only share a prefix.
• KubernetesClusterManagerImplTest: the Alert retry check:
  • accepts the recorded upgrade version even when no node has reached it, and another version with the same semantic version, and refuses a different version;
  • without that record, accepts a node at the target version, and refuses a cluster with no node there or where only a manually upgraded node is there;
  • refuses a target that is the cluster's current version, or another version with the same semantic version.
• KubernetesClusterUtilTest: the version match ignores a trailing newline, a line of stderr and build metadata, and is otherwise exact.

mvn -pl plugins/integrations/kubernetes-service -am test -Dtest='KubernetesCluster*Test' passes: 84 tests across 7 classes. Checkstyle passes on api and kubernetes-service.

How did you try to break this feature and the system with this change?

• A normal upgrade from Running: every node is still at the old recorded version, so the skip check makes no SSH call and nothing changes.
• Shared networks (SSH port 22): node addressing is unchanged.
• Port-forwarding rules that are being revoked are ignored when looking up a node's SSH port.
• An interrupted wait between drain attempts stops the retries instead of firing the rest with no wait.
• A scale-down where every drain succeeds: nodes are deleted and their VMs destroyed in the same order as before. The difference is that all of them are cordoned and drained first.
• Not covered: a scale-down that fails after some nodes have been removed still goes to Alert, as before. The nodes that are still there are uncordoned.

[codecov]

Codecov Report

❌ Patch coverage is 53.13653% with 127 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.34%. Comparing base (d02adcd) to head (0dcaa8f).

Files with missing lines	Patch %	Lines
...er/actionworkers/KubernetesClusterScaleWorker.java	42.85%	44 Missing ⚠️
.../actionworkers/KubernetesClusterUpgradeWorker.java	40.38%	29 Missing and 2 partials ⚠️
...r/actionworkers/KubernetesClusterActionWorker.java	68.08%	23 Missing and 7 partials ⚠️
...bernetes/cluster/KubernetesClusterManagerImpl.java	43.75%	14 Missing and 4 partials ⚠️
...er/actionworkers/KubernetesClusterStartWorker.java	0.00%	2 Missing ⚠️
...ubernetes/cluster/utils/KubernetesClusterUtil.java	81.81%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.22   #14611      +/-   ##
============================================
+ Coverage     18.32%   18.34%   +0.01%     
- Complexity    16644    16681      +37     
============================================
  Files          5939     5939              
  Lines        537768   537959     +191     
  Branches      65929    65965      +36     
============================================
+ Hits          98551    98666     +115     
- Misses       428008   428079      +71     
- Partials      11209    11214       +5     

Flag	Coverage Δ
uitests	4.08% <ø> (ø)
unittests	19.43% <53.13%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.