Skip to content

Commit

Permalink
Fix reconcilers where error condition leads to infinite reconciliation (
Browse files Browse the repository at this point in the history 
oracle#316)

* Fix reconcilers where error condition leads to infinite reconciliation without exponential backoff
  • Loading branch information
@shyamradhakrishnan
shyamradhakrishnan committed Aug 8, 2023
1 parent e7f3ec8 commit 84976e0
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 14 deletions.
14 changes: 8 additions & 6 deletions exp/controllers/ocimachinepool_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,6 @@ func (r *OCIMachinePoolReconciler) reconcileNormal(ctx context.Context, logger l
// https://docs.oracle.com/en-us/iaas/api/#/en/iaas/20160918/InstanceConfiguration/
if err := machinePoolScope.ReconcileInstanceConfiguration(ctx); err != nil {
r.Recorder.Eventf(machinePoolScope.OCIMachinePool, corev1.EventTypeWarning, "FailedLaunchTemplateReconcile", "Failed to reconcile launch template: %v", err)
machinePoolScope.Error(err, "failed to reconcile launch template")
return ctrl.Result{}, err
}

Expand All @@ -305,13 +304,15 @@ func (r *OCIMachinePoolReconciler) reconcileNormal(ctx context.Context, logger l
// Find existing Instance Pool
instancePool, err := machinePoolScope.FindInstancePool(ctx)
if err != nil {
conditions.MarkUnknown(machinePoolScope.OCIMachinePool, infrav2exp.InstancePoolReadyCondition, infrav2exp.InstancePoolNotFoundReason, err.Error())
r.Recorder.Event(machinePoolScope.OCIMachinePool, corev1.EventTypeWarning, "ReconcileError", err.Error())
conditions.MarkUnknown(machinePoolScope.OCIMachinePool, infrav2exp.InstancePoolReadyCondition, infrav2exp.InstancePoolNotFoundReason, "")
return ctrl.Result{}, err
}

if instancePool == nil {
if _, err := machinePoolScope.CreateInstancePool(ctx); err != nil {
conditions.MarkFalse(machinePoolScope.OCIMachinePool, infrav2exp.InstancePoolReadyCondition, infrav2exp.InstancePoolProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error())
r.Recorder.Event(machinePoolScope.OCIMachinePool, corev1.EventTypeWarning, "ReconcileError", err.Error())
conditions.MarkFalse(machinePoolScope.OCIMachinePool, infrav2exp.InstancePoolReadyCondition, infrav2exp.InstancePoolProvisionFailedReason, clusterv1.ConditionSeverityError, "")
return ctrl.Result{}, err
}
r.Recorder.Eventf(machinePoolScope.OCIMachinePool, corev1.EventTypeNormal, "InstancePoolCreated", "Created new Instance Pool: %s", machinePoolScope.OCIMachinePool.GetName())
Expand Down Expand Up @@ -398,7 +399,7 @@ func (r *OCIMachinePoolReconciler) reconcileDelete(ctx context.Context, machineP
if instancePool == nil {
machinePoolScope.OCIMachinePool.Status.Ready = false
conditions.MarkFalse(machinePoolScope.OCIMachinePool, infrav2exp.InstancePoolReadyCondition, infrav2exp.InstancePoolNotFoundReason, clusterv1.ConditionSeverityWarning, "")
machinePoolScope.Info("Instance Pool may already be deleted", "displayName", instancePool.DisplayName, "id", instancePool.Id)
machinePoolScope.Info("Instance Pool may already be deleted")
r.Recorder.Eventf(machinePoolScope.OCIMachinePool, corev1.EventTypeNormal, infrav2exp.InstancePoolNotFoundReason, "Unable to find matching instance pool")
} else {
switch instancePool.LifecycleState {
Expand Down Expand Up @@ -462,13 +463,14 @@ func (r *OCIMachinePoolReconciler) reconcileMachines(ctx context.Context, err er
}
err = cloudutil.CreateMachinePoolMachinesIfNotExists(ctx, params)
if err != nil {
conditions.MarkFalse(machinePoolScope.OCIMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, err.Error())
r.Recorder.Event(machinePoolScope.OCIMachinePool, corev1.EventTypeWarning, "FailedToCreateNewMachines", err.Error())
conditions.MarkFalse(machinePoolScope.OCIMachinePool, clusterv1.ReadyCondition, "FailedToCreateNewMachines", clusterv1.ConditionSeverityWarning, "")
return errors.Wrap(err, "failed to create missing machines")
}

err = cloudutil.DeleteOrphanedMachinePoolMachines(ctx, params)
if err != nil {
conditions.MarkFalse(machinePoolScope.OCIMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, err.Error())
conditions.MarkFalse(machinePoolScope.OCIMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, "")
return errors.Wrap(err, "failed to delete orphaned machines")
}
return nil
Expand Down
11 changes: 7 additions & 4 deletions exp/controllers/ocimanaged_machinepool_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,15 @@ func (r *OCIManagedMachinePoolReconciler) reconcileNormal(ctx context.Context, l
// Find existing Node Pool
nodePool, err := machinePoolScope.FindNodePool(ctx)
if err != nil {
conditions.MarkUnknown(machinePoolScope.OCIManagedMachinePool, infrav2exp.NodePoolReadyCondition, infrav2exp.NodePoolNotFoundReason, err.Error())
r.Recorder.Event(machinePoolScope.OCIManagedMachinePool, corev1.EventTypeWarning, "ReconcileError", err.Error())
conditions.MarkUnknown(machinePoolScope.OCIManagedMachinePool, infrav2exp.NodePoolReadyCondition, infrav2exp.NodePoolNotFoundReason, "")
return ctrl.Result{}, err
}

if nodePool == nil {
if nodePool, err = machinePoolScope.CreateNodePool(ctx); err != nil {
conditions.MarkFalse(machinePoolScope.OCIManagedMachinePool, infrav2exp.NodePoolReadyCondition, infrav2exp.NodePoolProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error())
r.Recorder.Event(machinePoolScope.OCIManagedMachinePool, corev1.EventTypeWarning, "ReconcileError", err.Error())
conditions.MarkFalse(machinePoolScope.OCIManagedMachinePool, infrav2exp.NodePoolReadyCondition, infrav2exp.NodePoolProvisionFailedReason, clusterv1.ConditionSeverityError, "")
return ctrl.Result{}, err
}
// record the event only when node pool is created
Expand Down Expand Up @@ -391,13 +393,14 @@ func (r *OCIManagedMachinePoolReconciler) reconcileManagedMachines(ctx context.C
}
err = cloudutil.CreateMachinePoolMachinesIfNotExists(ctx, params)
if err != nil {
conditions.MarkFalse(machinePoolScope.OCIManagedMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, err.Error())
r.Recorder.Event(machinePoolScope.OCIManagedMachinePool, corev1.EventTypeWarning, "FailedToCreateNewMachines", err.Error())
conditions.MarkFalse(machinePoolScope.OCIManagedMachinePool, clusterv1.ReadyCondition, "FailedToCreateNewMachines", clusterv1.ConditionSeverityWarning, "")
return errors.Wrap(err, "failed to create missing machines")
}

err = cloudutil.DeleteOrphanedMachinePoolMachines(ctx, params)
if err != nil {
conditions.MarkFalse(machinePoolScope.OCIManagedMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, err.Error())
conditions.MarkFalse(machinePoolScope.OCIManagedMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, "")
return errors.Wrap(err, "failed to delete orphaned machines")
}
return nil
Expand Down
12 changes: 8 additions & 4 deletions exp/controllers/ocivirtual_machinepool_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,15 @@ func (r *OCIVirtualMachinePoolReconciler) reconcileNormal(ctx context.Context, l
// Find existing Virtual Node Pool
nodePool, err := machinePoolScope.FindVirtualNodePool(ctx)
if err != nil {
conditions.MarkUnknown(machinePoolScope.OCIVirtualMachinePool, infrav2exp.VirtualNodePoolReadyCondition, infrav2exp.VirtualNodePoolNotFoundReason, err.Error())
r.Recorder.Event(machinePoolScope.OCIVirtualMachinePool, corev1.EventTypeWarning, "ReconcileError", err.Error())
conditions.MarkUnknown(machinePoolScope.OCIVirtualMachinePool, infrav2exp.VirtualNodePoolReadyCondition, infrav2exp.VirtualNodePoolNotFoundReason, "")
return ctrl.Result{}, err
}

if nodePool == nil {
if nodePool, err = machinePoolScope.CreateVirtualNodePool(ctx); err != nil {
conditions.MarkFalse(machinePoolScope.OCIVirtualMachinePool, infrav2exp.VirtualNodePoolReadyCondition, infrav2exp.VirtualNodePoolProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error())
r.Recorder.Event(machinePoolScope.OCIVirtualMachinePool, corev1.EventTypeWarning, "ReconcileError", err.Error())
conditions.MarkFalse(machinePoolScope.OCIVirtualMachinePool, infrav2exp.VirtualNodePoolReadyCondition, infrav2exp.VirtualNodePoolProvisionFailedReason, clusterv1.ConditionSeverityError, "")
return ctrl.Result{}, err
}
// record the event only when node pool is created
Expand Down Expand Up @@ -405,13 +407,15 @@ func (r *OCIVirtualMachinePoolReconciler) reconcileVirtualMachines(ctx context.C
}
err = cloudutil.CreateMachinePoolMachinesIfNotExists(ctx, params)
if err != nil {
conditions.MarkFalse(machinePoolScope.OCIVirtualMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, err.Error())
r.Recorder.Event(machinePoolScope.OCIVirtualMachinePool, corev1.EventTypeWarning, "FailedToCreateNewMachines", err.Error())
conditions.MarkFalse(machinePoolScope.OCIVirtualMachinePool, clusterv1.ReadyCondition, "FailedToCreateNewMachines", clusterv1.ConditionSeverityWarning, "")
return errors.Wrap(err, "failed to create missing machines")
}

err = cloudutil.DeleteOrphanedMachinePoolMachines(ctx, params)
if err != nil {
conditions.MarkFalse(machinePoolScope.OCIVirtualMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, err.Error())
r.Recorder.Event(machinePoolScope.OCIVirtualMachinePool, corev1.EventTypeWarning, "FailedToDeleteOrphanedMachines", err.Error())
conditions.MarkFalse(machinePoolScope.OCIVirtualMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, "")
return errors.Wrap(err, "failed to delete orphaned machines")
}
return nil
Expand Down

0 comments on commit 84976e0

Please sign in to comment.