Skip to content

Commit 99fa86c

Browse files
fix(cpo): delete terminated MCD pods and requeue to retry in-place upgrades
When an in-place MCD upgrade pod terminates (Failed/Succeeded) but the node still needs an upgrade, the controller now deletes the terminated pod so a fresh one can be recreated on the next reconcile loop. A periodic requeue (upgradeRequeueInterval = 30s) ensures the controller re-evaluates nodes that still need upgrades rather than waiting for an external event. Additionally: - Extract deleteUpgradePodIfExists helper to reduce duplication across reconcileUpgradePods and deleteUpgradeManifests - Add test coverage for PodPending phase, multi-node mixed states, NotFound on Delete, RequeueAfter assertion, and Delete failure scenarios Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c19746e commit 99fa86c

2 files changed

Lines changed: 580 additions & 31 deletions

File tree

control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.go

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ const (
5858
TokenSecretPayloadKey = "payload"
5959
TokenSecretReleaseKey = "release"
6060
TokenSecretReleaseVersionKey = "release-version"
61+
62+
// upgradeRequeueInterval is how often the controller rechecks while an
63+
// upgrade is in progress, closing the gap when a force-deleted pod's
64+
// deletion event is missed.
65+
upgradeRequeueInterval = 30 * time.Second
6166
)
6267

6368
type Reconciler struct {
@@ -146,7 +151,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
146151
return ctrl.Result{}, fmt.Errorf("token secret %s/%s is missing %q key", tokenSecret.Namespace, tokenSecret.Name, TokenSecretReleaseVersionKey)
147152
}
148153

149-
return ctrl.Result{}, r.reconcileInPlaceUpgrade(ctx, nodePoolUpgradeAPI, tokenSecret, mcoImage, releaseVersion)
154+
if err := r.reconcileInPlaceUpgrade(ctx, nodePoolUpgradeAPI, tokenSecret, mcoImage, releaseVersion); err != nil {
155+
return ctrl.Result{}, err
156+
}
157+
// Requeue periodically while an upgrade is in progress. The controller only
158+
// watches Nodes and MachineSets, so if an upgrade pod is force-deleted the
159+
// deletion event is missed and the replacement pod is never created. A
160+
// periodic recheck closes that gap.
161+
return ctrl.Result{RequeueAfter: upgradeRequeueInterval}, nil
150162
}
151163

152164
type nodePoolUpgradeAPI struct {
@@ -276,7 +288,7 @@ func (r *Reconciler) reconcileInPlaceUpgrade(ctx context.Context, nodePoolUpgrad
276288

277289
err = r.reconcileUpgradePods(ctx, r.guestClusterClient, nodes, nodePoolUpgradeAPI.spec.poolRef.GetName(), mcoImage, nodePoolUpgradeAPI.proxy)
278290
if err != nil {
279-
return fmt.Errorf("failed to delete idle upgrade pods: %w", err)
291+
return fmt.Errorf("failed to reconcile upgrade pods: %w", err)
280292
}
281293
return nil
282294
}
@@ -311,22 +323,11 @@ func (r *Reconciler) reconcileUpgradePods(ctx context.Context, hostedClusterClie
311323
pod := inPlaceUpgradePod(namespace.Name, node.Name)
312324

313325
if node.Annotations[CurrentMachineConfigAnnotationKey] == node.Annotations[DesiredMachineConfigAnnotationKey] &&
314-
node.Annotations[DesiredDrainerAnnotationKey] == node.Annotations[LastAppliedDrainerAnnotationKey] {
326+
node.Annotations[DesiredDrainerAnnotationKey] == node.Annotations[LastAppliedDrainerAnnotationKey] &&
327+
node.Annotations[MachineConfigDaemonStateAnnotationKey] == MachineConfigDaemonStateDone {
315328
// the node is updated and does not require a MCD running
316-
if err := hostedClusterClient.Get(ctx, client.ObjectKeyFromObject(pod), pod); err != nil {
317-
if apierrors.IsNotFound(err) {
318-
continue
319-
}
320-
return fmt.Errorf("error getting upgrade MCD pod: %w", err)
321-
}
322-
if pod.DeletionTimestamp != nil {
323-
continue
324-
}
325-
if err := hostedClusterClient.Delete(ctx, pod); err != nil {
326-
if apierrors.IsNotFound(err) {
327-
continue
328-
}
329-
return fmt.Errorf("error deleting upgrade MCD pod: %w", err)
329+
if err := deleteUpgradePodIfExists(ctx, hostedClusterClient, pod); err != nil {
330+
return err
330331
}
331332
log.Info("Deleted idle upgrade pod")
332333
} else {
@@ -348,12 +349,41 @@ func (r *Reconciler) reconcileUpgradePods(ctx context.Context, hostedClusterClie
348349
} else {
349350
log.Info("create upgrade pod", "result", result)
350351
}
352+
// A pod with RestartPolicy=OnFailure only reaches a terminal phase after kubelet has exhausted its restart attempts (e.g. eviction or node loss), so deleting it here does not interrupt an active retry.
353+
} else if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed {
354+
if pod.DeletionTimestamp != nil {
355+
continue
356+
}
357+
log.Info("Detected terminated upgrade pod on node that still needs upgrade, deleting for retry",
358+
"node", node.Name, "podPhase", pod.Status.Phase)
359+
if err := hostedClusterClient.Delete(ctx, pod); err != nil {
360+
if apierrors.IsNotFound(err) {
361+
continue
362+
}
363+
return fmt.Errorf("error deleting terminated upgrade MCD pod for node %s: %w", node.Name, err)
364+
}
351365
}
352366
}
353367
}
354368
return nil
355369
}
356370

371+
func deleteUpgradePodIfExists(ctx context.Context, c client.Client, pod *corev1.Pod) error {
372+
if err := c.Get(ctx, client.ObjectKeyFromObject(pod), pod); err != nil {
373+
if apierrors.IsNotFound(err) {
374+
return nil
375+
}
376+
return fmt.Errorf("error getting upgrade MCD pod %s: %w", pod.Name, err)
377+
}
378+
if pod.DeletionTimestamp != nil {
379+
return nil
380+
}
381+
if err := c.Delete(ctx, pod); err != nil && !apierrors.IsNotFound(err) {
382+
return fmt.Errorf("error deleting upgrade MCD pod %s: %w", pod.Name, err)
383+
}
384+
return nil
385+
}
386+
357387
// getPayloadImage gets the specified image reference from the payload.
358388
func (r *Reconciler) getPayloadImage(ctx context.Context, imageName string) (string, error) {
359389
hcp := manifests.HostedControlPlane(r.hcpNamespace, r.hcpName)
@@ -483,20 +513,8 @@ func deleteUpgradeManifests(ctx context.Context, hostedClusterClient client.Clie
483513
namespace := inPlaceUpgradeNamespace(poolName)
484514
for _, node := range nodes {
485515
pod := inPlaceUpgradePod(namespace.Name, node.Name)
486-
if err := hostedClusterClient.Get(ctx, client.ObjectKeyFromObject(pod), pod); err != nil {
487-
if apierrors.IsNotFound(err) {
488-
continue
489-
}
490-
return fmt.Errorf("error getting upgrade MCD pod: %w", err)
491-
}
492-
if pod.DeletionTimestamp != nil {
493-
continue
494-
}
495-
if err := hostedClusterClient.Delete(ctx, pod); err != nil {
496-
if apierrors.IsNotFound(err) {
497-
continue
498-
}
499-
return fmt.Errorf("error deleting upgrade MCD pod: %w", err)
516+
if err := deleteUpgradePodIfExists(ctx, hostedClusterClient, pod); err != nil {
517+
return err
500518
}
501519
}
502520
return nil

0 commit comments

Comments
 (0)