fix(talosctl): reboot after a failed drain during upgrade

talosctl upgrade writes the new installer image to disk before it
drains the Kubernetes node and reboots into it. When the drain step
failed - for example because the apiserver is unreachable or the
cluster is not yet bootstrapped - the command returned early, before
the reboot. That left the upgrade half-applied: the new image was
staged on disk while the node kept running the old version, with no
reboot to activate it and no signal to the operator, since the earlier
progress output already read "upgrade completed". The next unrelated
reboot would then silently boot into the staged image.

Once the image is committed to disk, aborting before the reboot is the
worst outcome. The drain is a best-effort courtesy and must not strand
the upgrade, so warn about the failed drain and continue to the reboot
so the staged image is actually applied.

Draining cordons each node before evicting its pods, so a drain that
fails after the cordon leaves the node unschedulable. drainNodes threw
away the collected node map on error, so the deferred uncordon was
skipped: the node rebooted into the new version but stayed
SchedulingDisabled, and with the reboot succeeding the command still
exited 0 - an orphaned cordon with no error surfaced. Return the
partial map on error so the existing uncordon step restores the node
to schedulable once it is Ready again.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

Author: lexfrei

cmd/talosctl/cmd/talos/drain.go

@@ -30,6 +30,12 @@ type nodeUpdate struct {
 // and performs cordon + drain on all of them in parallel.
 //
 // It returns a map of talosIP -> k8sNodeName for use in the uncordon phase.
+//
+// On error the map is still returned (partially populated): each node name is
+// recorded before that node is cordoned, so the map holds every node that may
+// have been cordoned before the failure. A caller that proceeds past a failed
+// drain (e.g. the upgrade path, which has already staged a new image on disk)
+// can then still uncordon those nodes instead of leaving them SchedulingDisabled.
 func drainNodes(ctx context.Context, clientFactory *global.ClientFactory, drainTimeout time.Duration, rep *reporter.Reporter) (map[string]string, error) {
 	// For kubeconfig - build a random endpoint client (to go to the controlplane).
 	c, err := clientFactory.BuildRandomEndpointClient(ctx)
@@ -103,7 +109,10 @@ func drainNodes(ctx context.Context, clientFactory *global.ClientFactory, drainT
 	<-aggregatorDone
 
 	if err != nil {
-		return nil, err
+		// Return the partially-populated map alongside the error: it holds the
+		// nodes that were cordoned before the failure, so a caller that continues
+		// past a failed drain can still uncordon them.
+		return k8sNames, err
 	}
 
 	return k8sNames, nil

cmd/talosctl/cmd/talos/upgrade.go

@@ -147,7 +147,15 @@ func upgradeViaLifecycleService(ctx context.Context, clientFactory *global.Clien
 	if upgradeCmdFlags.drain {
 		nodeNames, err = drainNodes(ctx, clientFactory, upgradeCmdFlags.drainTimeout, rep)
 		if err != nil {
-			return err
+			// upgradeInternal above has already written the new installer image to disk;
+			// the node only switches to it on the reboot below. Aborting here would leave
+			// the upgrade half-applied: the new image is staged on disk, the node keeps
+			// running the old version, and the next unrelated reboot silently boots into
+			// the new one - all while the earlier progress output reads "upgrade completed".
+			// A failed drain (e.g. the Kubernetes apiserver is unreachable, or the cluster
+			// is not yet bootstrapped) is a best-effort courtesy and must not strand the
+			// upgrade. Warn loudly and continue to the reboot so the staged image is applied.
+			cli.Warning("failed to drain Kubernetes node(s) before reboot, continuing without drain: %s", err)
 		}
 	}