Skip to content

Commit d78b470

Browse files
feat: add rollback recovery for control plane resize (#4865)
* feat(frontend): add rollback recovery for control plane resize Add automatic rollback to /resizecontrolplane so failed master resizes restore the cluster to a healthy state with three running, schedulable control plane nodes at matching SKUs. Key behaviors: - Per-node snapshot captures original VM size, Machine spec, Node labels, and schedulability before mutation - On failure, nodes roll back in reverse order and restore VM size, Machine metadata, Node labels, and schedulability - Etcd health is checked between each node resize in forward and rollback paths - Context detachment lets the operation complete even if the admin HTTP connection drops - Step journaling captures forward and rollback timing details for diagnostics Fixes ARO-27429 * fix(frontend): reduce rollback PR scope and harden rollback gate Keep rollback-specific logic in this PR by dropping pre-validation inventory carryover and duplicated rollback matrix tests, while making rollback stop when etcd is unhealthy between nodes and documenting the behavior. * fix(frontend): guard resize error normalization and immediate retry cancellation Prevent nil CloudErrorBody panics when normalizing wrapped resize errors and stop zero-delay retry loops from issuing extra Azure calls after context cancellation. Co-authored-by: Cursor <cursoragent@cursor.com> * test(frontend): keep default-delay retry semantics Revert the zero-delay cancellation behavior change and its extra test to keep runtime semantics aligned with the existing production retry policy. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(frontend): simplify control plane node label updates Keep rollback label restoration on the same aligned-label path used during resize so the helper surface matches the fail-closed validation model. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(frontend): remove rollback retry policy indirection Keep the Azure retry helper on the single production code path so tests verify the same retry semantics used during resize recovery. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(frontend): remove empty node label fallback Fail fast when the internal node label helper is called without a VM size so resize and rollback only exercise the validated non-empty path. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(frontend): move resize inventory checks back to preflight Reuse the live Machine/Azure VM/Node inventory validator in pre-flight so snapshot capture only records rollback state and no longer duplicates those consistency checks. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(frontend): reuse steps in resize happy path Move the linear control plane resize flow onto pkg/util/steps while keeping rollback state, retry policy, and operator-facing journaling local to frontend. This answers the reviewer request with a small behavioral diff and preserves the admin flow's existing error contract. Co-authored-by: Cursor <cursoragent@cursor.com> * test(frontend): pin etcd wait timeout in resize recovery Cover waitForEtcdHealthy with a deadline-aware test so future refactors keep the per-check timeout boundary. This protects the admin resize flow from accidentally broadening the etcd wait semantics through pkg/util/steps. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(frontend): normalize resize admin error messages Keep resize admin replies readable by reusing the wrapped CloudError body text instead of duplicating status and target prefixes in the message field. Also normalize joined control plane inventory validation errors so Geneva-facing admin responses do not leak embedded newlines. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(frontend): preserve rebased resize preflight validation Keep the rebased control plane resize branch on the validated inventory-preflight path and update the rebased tests to assert the normalized inventory errors without leaving a standalone follow-up fix commit. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent e548a0d commit d78b470

7 files changed

Lines changed: 1948 additions & 260 deletions

pkg/frontend/admin_openshiftcluster_resize_controlplane.go

Lines changed: 95 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"cmp"
88
"context"
99
"encoding/json"
10+
"errors"
1011
"fmt"
1112
"maps"
1213
"net/http"
@@ -32,6 +33,7 @@ import (
3233
"github.com/Azure/ARO-RP/pkg/database/cosmosdb"
3334
"github.com/Azure/ARO-RP/pkg/frontend/adminactions"
3435
"github.com/Azure/ARO-RP/pkg/frontend/middleware"
36+
"github.com/Azure/ARO-RP/pkg/util/stringutils"
3537
)
3638

3739
const (
@@ -47,10 +49,60 @@ func (f *frontend) postAdminResizeControlPlane(w http.ResponseWriter, r *http.Re
4749
r.URL.Path = filepath.Dir(r.URL.Path)
4850

4951
err := f._postAdminResizeControlPlane(log, ctx, r)
52+
err = normalizeResizeControlPlaneErrorForAdminReply(err)
5053

5154
adminReply(log, w, nil, nil, err)
5255
}
5356

57+
func normalizeResizeControlPlaneErrorForAdminReply(err error) error {
58+
if err == nil {
59+
return nil
60+
}
61+
62+
var resizeErr *resizeControlPlaneError
63+
var cloudErr *api.CloudError
64+
if errors.As(err, &resizeErr) && errors.As(err, &cloudErr) {
65+
code := api.CloudErrorCodeInternalServerError
66+
target := ""
67+
details := []api.CloudErrorBody(nil)
68+
message := err.Error()
69+
if cloudErr.CloudErrorBody != nil {
70+
code = cloudErr.Code
71+
target = cloudErr.Target
72+
details = cloudErr.Details
73+
message = formatResizeControlPlaneAdminReplyMessage(cloudErr.Message, resizeErr)
74+
}
75+
76+
return &api.CloudError{
77+
StatusCode: cloudErr.StatusCode,
78+
CloudErrorBody: &api.CloudErrorBody{
79+
Code: code,
80+
Message: message,
81+
Target: target,
82+
Details: details,
83+
},
84+
}
85+
}
86+
87+
return err
88+
}
89+
90+
func formatResizeControlPlaneAdminReplyMessage(baseMessage string, resizeErr *resizeControlPlaneError) string {
91+
var b strings.Builder
92+
b.WriteString(baseMessage)
93+
94+
if len(resizeErr.steps) > 0 {
95+
b.WriteString(". Steps: ")
96+
b.WriteString(strings.Join(resizeErr.steps, ", "))
97+
}
98+
if resizeErr.rollbackErr != nil {
99+
b.WriteString(". Rollback errors: ")
100+
b.WriteString(resizeErr.rollbackErr.Error())
101+
}
102+
103+
return b.String()
104+
}
105+
54106
func (f *frontend) _postAdminResizeControlPlane(log *logrus.Entry, ctx context.Context, r *http.Request) error {
55107
resType := chi.URLParam(r, "resourceType")
56108
resName := chi.URLParam(r, "resourceName")
@@ -110,12 +162,16 @@ func (f *frontend) _postAdminResizeControlPlane(log *logrus.Entry, ctx context.C
110162
return err
111163
}
112164

113-
return resizeControlPlane(ctx, log, k, a, vmSize, deallocateVM)
165+
clusterResourceGroupName := stringutils.LastTokenByte(doc.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/')
166+
executionCtx, cancel := newResizeControlPlaneExecutionContext(ctx)
167+
defer cancel()
168+
169+
return resizeControlPlane(executionCtx, log, k, a, vmSize, deallocateVM, clusterResourceGroupName)
114170
}
115171

116172
// resizeControlPlane orchestrates the full control plane resize operation,
117173
// processing each master node sequentially in reverse name order.
118-
func resizeControlPlane(ctx context.Context, log *logrus.Entry, k adminactions.KubeActions, a adminactions.AzureActions, desiredVMSize string, deallocateVM bool) error {
174+
func resizeControlPlane(ctx context.Context, log *logrus.Entry, k adminactions.KubeActions, a adminactions.AzureActions, desiredVMSize string, deallocateVM bool, clusterResourceGroupName string) error {
119175
// getControlPlaneMachines filters by machine.openshift.io/cluster-api-machine-role=master,
120176
// so the returned map only contains control plane machines.
121177
machines, err := getControlPlaneMachines(ctx, k)
@@ -135,82 +191,51 @@ func resizeControlPlane(ctx context.Context, log *logrus.Entry, k adminactions.K
135191
return cmp.Compare(b, a)
136192
})
137193

138-
// Guard the whole operation before touching any VM. Even when a machine
139-
// already matches the target SKU, we must not continue to the next resize
140-
// while another control plane node is NotReady or still cordoned.
141-
if err := ensureControlPlaneNodesReadyAndSchedulable(ctx, k, sortedNames); err != nil {
142-
return err
194+
operation := newResizeControlPlaneOperation(log, k, a, desiredVMSize, deallocateVM, clusterResourceGroupName)
195+
196+
triggerRollback := func(baseErr error) error {
197+
rollbackErr := operation.rollbackAll(ctx)
198+
return &resizeControlPlaneError{
199+
baseErr: baseErr,
200+
steps: operation.steps,
201+
rollbackErr: rollbackErr,
202+
}
143203
}
144204

145205
for _, name := range sortedNames {
206+
if err := ensureControlPlaneAndEtcdHealthy(ctx, k, sortedNames); err != nil {
207+
if len(operation.nodes) == 0 {
208+
return err
209+
}
210+
return triggerRollback(err)
211+
}
212+
146213
machine := machines[name]
147214
if machine.size == desiredVMSize {
148215
log.Infof("%s is already running %s, skipping", name, desiredVMSize)
149216
continue
150217
}
151218

219+
snapshot, err := operation.captureNodeSnapshot(ctx, name, machine)
220+
if err != nil {
221+
if len(operation.nodes) == 0 {
222+
return err
223+
}
224+
return triggerRollback(err)
225+
}
226+
nodeState := &controlPlaneNodeProgress{snapshot: snapshot}
227+
operation.nodes = append(operation.nodes, nodeState)
228+
152229
log.Infof("Resizing control plane node %s from %s to %s", name, machine.size, desiredVMSize)
153-
if err := resizeControlPlaneNode(ctx, log, k, a, name, desiredVMSize, deallocateVM); err != nil {
154-
return fmt.Errorf("failed to resize node %s: %w", name, err)
230+
if err := operation.resizeNode(ctx, nodeState); err != nil {
231+
return triggerRollback(fmt.Errorf("failed to resize node %s: %w", name, err))
155232
}
156233
log.Infof("Successfully resized node %s to %s", name, desiredVMSize)
157234
}
158235

159236
return nil
160237
}
161238

162-
// resizeControlPlaneNode performs the full resize sequence for a single
163-
// control plane node: cordon → drain → stop → resize → start → wait
164-
// ready → uncordon → update Machine metadata → update Node labels.
165-
func resizeControlPlaneNode(ctx context.Context, log *logrus.Entry, k adminactions.KubeActions, a adminactions.AzureActions, machineName, desiredVMSize string, deallocateVM bool) error {
166-
log.Infof("Cordoning node %s", machineName)
167-
if err := cordonNode(ctx, k, machineName); err != nil {
168-
return fmt.Errorf("cordoning node: %w", err)
169-
}
170-
171-
log.Infof("Draining node %s", machineName)
172-
if err := k.DrainNodeWithRetries(ctx, machineName); err != nil {
173-
return fmt.Errorf("draining node: %w", err)
174-
}
175-
176-
log.Infof("Stopping VM %s (deallocate=%v)", machineName, deallocateVM)
177-
if err := a.VMStopAndWait(ctx, machineName, deallocateVM); err != nil {
178-
return fmt.Errorf("stopping VM: %w", err)
179-
}
180-
181-
log.Infof("Resizing VM %s to %s", machineName, desiredVMSize)
182-
if err := a.VMResize(ctx, machineName, desiredVMSize); err != nil {
183-
return fmt.Errorf("resizing VM: %w", err)
184-
}
185-
186-
log.Infof("Starting VM %s", machineName)
187-
if err := a.VMStartAndWait(ctx, machineName); err != nil {
188-
return fmt.Errorf("starting VM: %w", err)
189-
}
190-
191-
log.Infof("Waiting for node %s to become Ready", machineName)
192-
if err := waitForNodeReady(ctx, log, k, machineName); err != nil {
193-
return fmt.Errorf("waiting for node ready: %w", err)
194-
}
195-
196-
log.Infof("Uncordoning node %s", machineName)
197-
if err := uncordonNode(ctx, k, machineName); err != nil {
198-
return fmt.Errorf("uncordoning node: %w", err)
199-
}
200-
201-
log.Infof("Updating Machine object for %s", machineName)
202-
if err := updateMachineVMSize(ctx, k, machineName, desiredVMSize); err != nil {
203-
return fmt.Errorf("updating Machine object: %w", err)
204-
}
205-
206-
log.Infof("Updating Node labels for %s", machineName)
207-
if err := updateNodeInstanceTypeLabels(ctx, k, machineName, desiredVMSize); err != nil {
208-
return fmt.Errorf("updating Node labels: %w", err)
209-
}
210-
211-
return nil
212-
}
213-
214239
func cordonNode(ctx context.Context, k adminactions.KubeActions, nodeName string) error {
215240
return k.CordonNode(ctx, nodeName, true)
216241
}
@@ -368,9 +393,14 @@ func doUpdateMachineVMSize(ctx context.Context, k adminactions.KubeActions, mach
368393
return k.KubeCreateOrUpdate(ctx, &obj)
369394
}
370395

371-
func updateNodeInstanceTypeLabels(ctx context.Context, k adminactions.KubeActions, nodeName, vmSize string) error {
396+
// Resize and rollback both update the stable and beta node labels together because validation rejects divergence.
397+
func setNodeInstanceTypeLabels(ctx context.Context, k adminactions.KubeActions, nodeName, vmSize string) error {
398+
if vmSize == "" {
399+
return fmt.Errorf("node instance type labels require a non-empty VM size")
400+
}
401+
372402
return retryKubeObjectUpdate(ctx, "Node", func() error {
373-
return doUpdateNodeInstanceTypeLabels(ctx, k, nodeName, vmSize)
403+
return doSetNodeInstanceTypeLabels(ctx, k, nodeName, vmSize)
374404
})
375405
}
376406

@@ -396,7 +426,8 @@ func retryKubeObjectUpdate(ctx context.Context, objectType string, updateFn func
396426
return fmt.Errorf("could not update %s object after %d attempts: %w", objectType, kubeObjectUpdateMaxAttempts, lastErr)
397427
}
398428

399-
func doUpdateNodeInstanceTypeLabels(ctx context.Context, k adminactions.KubeActions, nodeName, vmSize string) error {
429+
// Callers pass a validated VM size, so this helper only needs to mirror that value onto both node labels.
430+
func doSetNodeInstanceTypeLabels(ctx context.Context, k adminactions.KubeActions, nodeName, vmSize string) error {
400431
rawNode, err := k.KubeGet(ctx, "Node", "", nodeName)
401432
if err != nil {
402433
return err

0 commit comments

Comments
 (0)