Skip to content

Commit 4af6d4e

Browse files
committed
DNM: migration: add SynchronizedAPI status field support and migration cancellation
This change introduces the SynchronizedAPI status field to track the last successfully synchronized API during migrations. The field enables: - Migration cancellation: Users can cancel an in-progress migration by setting spec.AuthoritativeAPI back to match status.SynchronizedAPI, allowing rollback to the previously stable state - Status initialization: Proper handling of empty AuthoritativeAPI or SynchronizedAPI status fields during first reconciliation - Clear migration state tracking: The SynchronizedAPI captures which API was authoritative before entering the Migrating state Key changes: - Add handleMigrationStatusInitialization for proper status bootstrap - Add IsMigrationCancellationRequested detection logic - Update applyMigrationStatusWithPatch to set both status fields atomically - Add comprehensive tests for cancellation and initialization scenarios - Update vendor dependencies for SynchronizedAPI type support
1 parent 6792064 commit 4af6d4e

File tree

12 files changed

+783
-49
lines changed

12 files changed

+783
-49
lines changed

pkg/controllers/machinemigration/machine_migration_controller.go

Lines changed: 91 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -123,19 +123,36 @@ func (r *MachineMigrationReconciler) Reconcile(ctx context.Context, req reconcil
123123
return ctrl.Result{}, nil
124124
}
125125

126-
// If authoritativeAPI status is empty, it means it is the first time we see this resource.
127-
// Set the status.authoritativeAPI to match the spec.authoritativeAPI.
128-
//
129126
// N.B. Very similar logic is also present in the Machine API machine/machineset controllers
130127
// to cover for the cases when the migration controller is not running (e.g. on not yet supported platforms),
131128
// as such if any change is done to this logic, please consider changing it also there. See:
132129
// https://github.com/openshift/machine-api-operator/pull/1386/files#diff-8a4a734efbb8fef769f9f6ba5d30d94f19433a0b1eaeb1be4f2a55aa226c3b3dR180-R197
133-
if mapiMachine.Status.AuthoritativeAPI == "" {
134-
if err := r.applyStatusAuthoritativeAPIWithPatch(ctx, mapiMachine, mapiMachine.Spec.AuthoritativeAPI); err != nil {
135-
return ctrl.Result{}, fmt.Errorf("unable to apply authoritativeAPI to status with patch: %w", err)
130+
// Handle the cases where AuthoritativeAPI or SynchronizedAPI status fields are empty.
131+
if result, err := r.handleMigrationStatusInitialization(ctx, mapiMachine); err != nil {
132+
return ctrl.Result{}, err
133+
} else if result != nil {
134+
return *result, nil
135+
}
136+
137+
if synccommon.IsMigrationCancellationRequested(mapiMachine.Spec.AuthoritativeAPI, mapiMachine.Status.AuthoritativeAPI, mapiMachine.Status.SynchronizedAPI) {
138+
logger.Info("Detected migration cancellation request for machine, user wants to return to last synchronized state",
139+
"synchronizedAPI", mapiMachine.Status.SynchronizedAPI)
140+
141+
// Make sure the new authoritative resource (the one we are rolling back to) has been requested to unpause.
142+
// We cannot assume it is unpaused if we are cancelling midway through a migration where we might have already
143+
// paused the old authority.
144+
if err := r.ensureUnpauseRequestedOnNewAuthoritativeResource(ctx, mapiMachine); err != nil {
145+
return ctrl.Result{}, fmt.Errorf("unable to ensure the rollback AuthoritativeAPI has been un-paused: %w", err)
136146
}
137147

138-
// Wait for the patching to take effect.
148+
// Directly transition back to the synchronized API.
149+
// The resources are already in the correct pause state from the stuck migration.
150+
if err := r.applyMigrationStatusWithPatch(ctx, mapiMachine, mapiMachine.Spec.AuthoritativeAPI, mapiMachine.Status.SynchronizedAPI); err != nil {
151+
return ctrl.Result{}, fmt.Errorf("failed to apply authoritativeAPI for migration cancellation: %w", err)
152+
}
153+
154+
logger.Info("Machine migration cancelled successfully, returned to previous synchronized state")
155+
139156
return ctrl.Result{}, nil
140157
}
141158

@@ -157,8 +174,10 @@ func (r *MachineMigrationReconciler) Reconcile(ctx context.Context, req reconcil
157174
if mapiMachine.Status.AuthoritativeAPI != mapiv1beta1.MachineAuthorityMigrating {
158175
logger.Info("Detected migration request for machine")
159176

160-
if err := r.applyStatusAuthoritativeAPIWithPatch(ctx, mapiMachine, mapiv1beta1.MachineAuthorityMigrating); err != nil {
161-
return ctrl.Result{}, fmt.Errorf("unable to set authoritativeAPI %q to status: %w", mapiv1beta1.MachineAuthorityMigrating, err)
177+
// Before transitioning to Migrating, capture the current authoritativeAPI as synchronizedAPI.
178+
synchronizedAPI := synccommon.AuthoritativeAPIToSynchronizedAPI(mapiMachine.Status.AuthoritativeAPI)
179+
if err := r.applyMigrationStatusWithPatch(ctx, mapiMachine, mapiv1beta1.MachineAuthorityMigrating, synchronizedAPI); err != nil {
180+
return ctrl.Result{}, fmt.Errorf("unable to set authoritativeAPI to Migrating and synchronizedAPI: %w", err)
162181
}
163182

164183
logger.Info("Acknowledged migration request for machine")
@@ -195,17 +214,68 @@ func (r *MachineMigrationReconciler) Reconcile(ctx context.Context, req reconcil
195214
return ctrl.Result{}, fmt.Errorf("unable to ensure the new AuthoritativeAPI has been un-paused: %w", err)
196215
}
197216

198-
// Set the actual AuthoritativeAPI to the desired one, reset the synchronized generation and condition.
199-
if err := synccommon.ApplyAuthoritativeAPIAndResetSyncStatus[*machinev1applyconfigs.MachineStatusApplyConfiguration](ctx, r.Client, controllerName, machinev1applyconfigs.Machine, mapiMachine, mapiMachine.Spec.AuthoritativeAPI); err != nil {
200-
return ctrl.Result{}, fmt.Errorf("failed to apply authoritativeAPI and reset sync status: %w", err)
217+
// Set the actual AuthoritativeAPI to the desired one, update synchronizedAPI, reset the synchronized generation and condition.
218+
newSynchronizedAPI := synccommon.AuthoritativeAPIToSynchronizedAPI(mapiMachine.Spec.AuthoritativeAPI)
219+
220+
if err := r.applyMigrationStatusAndResetSyncStatusWithPatch(ctx, mapiMachine, mapiMachine.Spec.AuthoritativeAPI, newSynchronizedAPI); err != nil {
221+
return ctrl.Result{}, fmt.Errorf("failed to apply authoritativeAPI, synchronizedAPI and reset sync status: %w", err)
201222
}
202223

203-
logger.Info("Machine authority switch has now been completed and the resource unpaused")
224+
logger.Info("Machine authority switch has now been completed and the resource unpaused", "authoritativeAPI", mapiMachine.Spec.AuthoritativeAPI, "synchronizedAPI", newSynchronizedAPI)
204225
logger.Info("Machine migrated successfully")
205226

206227
return ctrl.Result{}, nil
207228
}
208229

230+
// handleMigrationStatusInitialization handles the initialization of the migration status.
231+
// It guarantees that both authoritativeAPI and SynchronizedAPI have valid values.
232+
func (r *MachineMigrationReconciler) handleMigrationStatusInitialization(ctx context.Context, mapiMachine *mapiv1beta1.Machine) (*ctrl.Result, error) {
233+
if mapiMachine.Status.AuthoritativeAPI == "" && mapiMachine.Status.SynchronizedAPI == "" {
234+
// If authoritativeAPI status or synchronizedAPI is empty, it means it is the first time we see this resource.
235+
// Set the status.authoritativeAPI and status.synchronizedAPI to match the spec.authoritativeAPI.
236+
synchronizedAPI := synccommon.AuthoritativeAPIToSynchronizedAPI(mapiMachine.Spec.AuthoritativeAPI)
237+
if err := r.applyMigrationStatusWithPatch(ctx, mapiMachine, mapiMachine.Spec.AuthoritativeAPI, synchronizedAPI); err != nil {
238+
return &ctrl.Result{}, fmt.Errorf("unable to apply authoritativeAPI to status with patch: %w", err)
239+
}
240+
241+
return &ctrl.Result{}, nil
242+
}
243+
244+
if mapiMachine.Status.AuthoritativeAPI == "" {
245+
// We assume the AuthoritativeAPI matches the last SynchronizedAPI.
246+
if err := r.applyMigrationStatusWithPatch(ctx, mapiMachine, mapiv1beta1.MachineAuthority(mapiMachine.Status.SynchronizedAPI), mapiMachine.Status.SynchronizedAPI); err != nil {
247+
return &ctrl.Result{}, fmt.Errorf("unable to apply authoritativeAPI to status with patch: %w", err)
248+
}
249+
250+
return &ctrl.Result{}, nil
251+
}
252+
253+
if mapiMachine.Status.SynchronizedAPI == "" {
254+
// We are in a migration (Status.AuthoritativeAPI is Migrating) but we don't have SynchronizedAPI.
255+
// Assuming this is a standard forward migration (not a cancellation), the Spec tells us the Target.
256+
// Therefore, the Source (SynchronizedAPI) must be the opposite of the Spec.
257+
targetAPI := mapiMachine.Spec.AuthoritativeAPI
258+
259+
var synchronizedAPI mapiv1beta1.SynchronizedAPI
260+
261+
if targetAPI == mapiv1beta1.MachineAuthorityMachineAPI {
262+
synchronizedAPI = mapiv1beta1.ClusterAPISynchronized
263+
} else {
264+
synchronizedAPI = mapiv1beta1.MachineAPISynchronized
265+
}
266+
267+
if err := r.applyMigrationStatusWithPatch(ctx, mapiMachine, mapiMachine.Status.AuthoritativeAPI, synchronizedAPI); err != nil {
268+
return &ctrl.Result{}, fmt.Errorf("unable to apply synchronizedAPI to status with patch: %w", err)
269+
}
270+
271+
// Wait for the patching to take effect.
272+
return &ctrl.Result{}, nil
273+
}
274+
275+
//nolint:nilnil
276+
return nil, nil
277+
}
278+
209279
// isOldAuthoritativeResourcePaused checks whether the old authoritative resource is paused.
210280
func (r *MachineMigrationReconciler) isOldAuthoritativeResourcePaused(ctx context.Context, m *mapiv1beta1.Machine) (bool, error) {
211281
if m.Spec.AuthoritativeAPI == mapiv1beta1.MachineAuthorityClusterAPI {
@@ -389,7 +459,12 @@ func (r *MachineMigrationReconciler) isSynchronized(ctx context.Context, mapiMac
389459
return false, fmt.Errorf("%w: %s", controllers.ErrInvalidSpecAuthoritativeAPI, mapiMachine.Spec.AuthoritativeAPI)
390460
}
391461

392-
// applyStatusAuthoritativeAPIWithPatch updates the resource status.authoritativeAPI using a server-side apply patch.
393-
func (r *MachineMigrationReconciler) applyStatusAuthoritativeAPIWithPatch(ctx context.Context, m *mapiv1beta1.Machine, authority mapiv1beta1.MachineAuthority) error {
394-
return synccommon.ApplyAuthoritativeAPI[*machinev1applyconfigs.MachineStatusApplyConfiguration](ctx, r.Client, controllerName, machinev1applyconfigs.Machine, m, authority)
462+
// applyMigrationStatusWithPatch updates the migration controller status fields using a server-side apply patch.
463+
func (r *MachineMigrationReconciler) applyMigrationStatusWithPatch(ctx context.Context, m *mapiv1beta1.Machine, authority mapiv1beta1.MachineAuthority, synchronizedAPI mapiv1beta1.SynchronizedAPI) error {
464+
return synccommon.ApplyMigrationStatus[*machinev1applyconfigs.MachineStatusApplyConfiguration](ctx, r.Client, controllerName, machinev1applyconfigs.Machine, m, authority, synchronizedAPI)
465+
}
466+
467+
// applyMigrationStatusAndResetSyncStatusWithPatch updates the migration controller status and resets sync status.
468+
func (r *MachineMigrationReconciler) applyMigrationStatusAndResetSyncStatusWithPatch(ctx context.Context, m *mapiv1beta1.Machine, authority mapiv1beta1.MachineAuthority, synchronizedAPI mapiv1beta1.SynchronizedAPI) error {
469+
return synccommon.ApplyMigrationStatusAndResetSyncStatus[*machinev1applyconfigs.MachineStatusApplyConfiguration](ctx, r.Client, controllerName, machinev1applyconfigs.Machine, m, authority, synchronizedAPI)
395470
}

0 commit comments

Comments
 (0)