Skip to content

Commit f6441cb

Browse files
fabriziopandinifabriziopandini
committed
conditions for reconcile delete
1 parent 7e445a1 commit f6441cb

File tree

12 files changed

+360
-158
lines changed

12 files changed

+360
-158
lines changed

‎api/v1alpha3/condition_consts.go

+19
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ const (
2424
ReadyCondition ConditionType = "Ready"
2525
)
2626

27+
// Common ConditionReason used by Cluster API objects.
28+
const (
29+
// DeletingReason (Severity=Info) documents an condition not in Status=True because the underlying object it is currently being deleted.
30+
DeletingReason = "Deleting"
31+
32+
// DeletedReason (Severity=Info) documents an condition not in Status=True because the underlying object was deleted.
33+
DeletedReason = "Deleted"
34+
)
35+
2736
const (
2837
// InfrastructureReadyCondition reports a summary of current status of the infrastructure object defined for this cluster/machine.
2938
// This condition is mirrored from the Ready condition in the infrastructure ref object, and
@@ -67,6 +76,16 @@ const (
6776
// to be available.
6877
// NOTE: This reason is used only as a fallback when the bootstrap object is not reporting its own ready condition.
6978
WaitingForDataSecretFallbackReason = "WaitingForDataSecret"
79+
80+
// DrainingSucceededCondition provide evidence of the status of the node drain operation which happens during the machine
81+
// deletion process.
82+
DrainingSucceededCondition ConditionType = "DrainingSucceeded"
83+
84+
// DrainingReason (Severity=Info) documents a machine node being drained.
85+
DrainingReason = "Draining"
86+
87+
// DrainingFailedReason (Severity=Warning) documents a machine node drain operation failed.
88+
DrainingFailedReason = "DrainingFailed"
7089
)
7190

7291
const (

‎controllers/cluster_controller.go

+37-9
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,6 @@ func (r *ClusterReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
124124
}
125125

126126
defer func() {
127-
// Always update the readyCondition with the summary of the cluster conditions.
128-
conditions.SetSummary(cluster,
129-
conditions.WithConditions(
130-
clusterv1.ControlPlaneReadyCondition,
131-
clusterv1.InfrastructureReadyCondition,
132-
),
133-
)
134-
135127
// Always reconcile the Status.Phase field.
136128
r.reconcilePhase(ctx, cluster)
137129

@@ -141,7 +133,7 @@ func (r *ClusterReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
141133
if reterr == nil {
142134
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
143135
}
144-
if err := patchHelper.Patch(ctx, cluster, patchOpts...); err != nil {
136+
if err := patchCluster(ctx, patchHelper, cluster, patchOpts...); err != nil {
145137
reterr = kerrors.NewAggregate([]error{reterr, err})
146138
}
147139
}()
@@ -161,6 +153,28 @@ func (r *ClusterReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
161153
return r.reconcile(ctx, cluster)
162154
}
163155

156+
func patchCluster(ctx context.Context, patchHelper *patch.Helper, cluster *clusterv1.Cluster, options ...patch.Option) error {
157+
// Always update the readyCondition by summarizing the state of other conditions.
158+
conditions.SetSummary(cluster,
159+
conditions.WithConditions(
160+
clusterv1.ControlPlaneReadyCondition,
161+
clusterv1.InfrastructureReadyCondition,
162+
),
163+
)
164+
165+
// Patch the object, ignoring conflicts on the conditions owned by this controller.
166+
// Also, if requested, we are adding additional options like e.g. Patch ObservedGeneration when issuing the
167+
// patch at the end of the reconcile loop.
168+
options = append(options,
169+
patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
170+
clusterv1.ReadyCondition,
171+
clusterv1.ControlPlaneReadyCondition,
172+
clusterv1.InfrastructureReadyCondition,
173+
}},
174+
)
175+
return patchHelper.Patch(ctx, cluster, options...)
176+
}
177+
164178
// reconcile handles cluster reconciliation.
165179
func (r *ClusterReconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster) (ctrl.Result, error) {
166180
phases := []func(context.Context, *clusterv1.Cluster) (ctrl.Result, error){
@@ -246,11 +260,18 @@ func (r *ClusterReconciler) reconcileDelete(ctx context.Context, cluster *cluste
246260
switch {
247261
case apierrors.IsNotFound(errors.Cause(err)):
248262
// All good - the control plane resource has been deleted
263+
conditions.MarkFalse(cluster, clusterv1.ControlPlaneReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
249264
case err != nil:
250265
return reconcile.Result{}, errors.Wrapf(err, "failed to get %s %q for Cluster %s/%s",
251266
path.Join(cluster.Spec.ControlPlaneRef.APIVersion, cluster.Spec.ControlPlaneRef.Kind),
252267
cluster.Spec.ControlPlaneRef.Name, cluster.Namespace, cluster.Name)
253268
default:
269+
// Report a summary of current status of the control plane object defined for this cluster.
270+
conditions.SetMirror(cluster, clusterv1.ControlPlaneReadyCondition,
271+
conditions.UnstructuredGetter(obj),
272+
conditions.WithFallbackValue(false, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, ""),
273+
)
274+
254275
// Issue a deletion request for the control plane object.
255276
// Once it's been deleted, the cluster will get processed again.
256277
if err := r.Client.Delete(ctx, obj); err != nil {
@@ -270,11 +291,18 @@ func (r *ClusterReconciler) reconcileDelete(ctx context.Context, cluster *cluste
270291
switch {
271292
case apierrors.IsNotFound(errors.Cause(err)):
272293
// All good - the infra resource has been deleted
294+
conditions.MarkFalse(cluster, clusterv1.InfrastructureReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
273295
case err != nil:
274296
return ctrl.Result{}, errors.Wrapf(err, "failed to get %s %q for Cluster %s/%s",
275297
path.Join(cluster.Spec.InfrastructureRef.APIVersion, cluster.Spec.InfrastructureRef.Kind),
276298
cluster.Spec.InfrastructureRef.Name, cluster.Namespace, cluster.Name)
277299
default:
300+
// Report a summary of current status of the infrastructure object defined for this cluster.
301+
conditions.SetMirror(cluster, clusterv1.InfrastructureReadyCondition,
302+
conditions.UnstructuredGetter(obj),
303+
conditions.WithFallbackValue(false, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, ""),
304+
)
305+
278306
// Issue a deletion request for the infrastructure object.
279307
// Once it's been deleted, the cluster will get processed again.
280308
if err := r.Client.Delete(ctx, obj); err != nil {

‎controllers/machine_controller.go

+106-40
Original file line numberDiff line numberDiff line change
@@ -167,18 +167,6 @@ func (r *MachineReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
167167
}
168168

169169
defer func() {
170-
// Always update the readyCondition with the summary of the machine conditions.
171-
conditions.SetSummary(m,
172-
conditions.WithConditions(
173-
clusterv1.BootstrapReadyCondition,
174-
clusterv1.InfrastructureReadyCondition,
175-
// TODO: add MHC conditions here
176-
),
177-
conditions.WithStepCounterIfOnly(
178-
clusterv1.BootstrapReadyCondition,
179-
clusterv1.InfrastructureReadyCondition,
180-
),
181-
)
182170

183171
r.reconcilePhase(ctx, m)
184172

@@ -188,7 +176,7 @@ func (r *MachineReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
188176
if reterr == nil {
189177
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
190178
}
191-
if err := patchHelper.Patch(ctx, m, patchOpts...); err != nil {
179+
if err := patchMachine(ctx, patchHelper, m, patchOpts...); err != nil {
192180
reterr = kerrors.NewAggregate([]error{reterr, err})
193181
}
194182
}()
@@ -214,6 +202,37 @@ func (r *MachineReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
214202
return r.reconcile(ctx, cluster, m)
215203
}
216204

205+
func patchMachine(ctx context.Context, patchHelper *patch.Helper, machine *clusterv1.Machine, options ...patch.Option) error {
206+
// Always update the readyCondition by summarizing the state of other conditions.
207+
// A step counter is added to represent progress during the provisioning process (instead we are hiding it
208+
// after provisioning - e.g. when a MHC condition exists - or during the deletion process).
209+
conditions.SetSummary(machine,
210+
conditions.WithConditions(
211+
clusterv1.BootstrapReadyCondition,
212+
clusterv1.InfrastructureReadyCondition,
213+
// TODO: add MHC conditions here
214+
),
215+
conditions.WithStepCounterIf(machine.ObjectMeta.DeletionTimestamp.IsZero()),
216+
conditions.WithStepCounterIfOnly(
217+
clusterv1.BootstrapReadyCondition,
218+
clusterv1.InfrastructureReadyCondition,
219+
),
220+
)
221+
222+
// Patch the object, ignoring conflicts on the conditions owned by this controller.
223+
// Also, if requested, we are adding additional options like e.g. Patch ObservedGeneration when issuing the
224+
// patch at the end of the reconcile loop.
225+
options = append(options,
226+
patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
227+
clusterv1.ReadyCondition,
228+
clusterv1.BootstrapReadyCondition,
229+
clusterv1.InfrastructureReadyCondition,
230+
}},
231+
)
232+
233+
return patchHelper.Patch(ctx, machine, options...)
234+
}
235+
217236
func (r *MachineReconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster, m *clusterv1.Machine) (ctrl.Result, error) {
218237
logger := r.Log.WithValues("machine", m.Name, "namespace", m.Namespace)
219238
logger = logger.WithValues("cluster", cluster.Name)
@@ -275,13 +294,26 @@ func (r *MachineReconciler) reconcileDelete(ctx context.Context, cluster *cluste
275294
if annotations.HasWithPrefix(clusterv1.PreDrainDeleteHookAnnotationPrefix, m.ObjectMeta.Annotations) {
276295
return ctrl.Result{}, nil
277296
}
278-
// Drain node before deletion.
297+
// Drain node before deletion and issue a patch in order to make this operation visible to the users.
279298
if _, exists := m.ObjectMeta.Annotations[clusterv1.ExcludeNodeDrainingAnnotation]; !exists {
299+
patchHelper, err := patch.NewHelper(m, r.Client)
300+
if err != nil {
301+
return ctrl.Result{}, err
302+
}
303+
280304
logger.Info("Draining node", "node", m.Status.NodeRef.Name)
305+
conditions.MarkFalse(m, clusterv1.DrainingSucceededCondition, "Draining", clusterv1.DrainingReason, "Draining the node before deletion")
306+
if err := patchMachine(ctx, patchHelper, m); err != nil {
307+
return ctrl.Result{}, errors.Wrap(err, "failed to patch Machine")
308+
}
309+
281310
if err := r.drainNode(ctx, cluster, m.Status.NodeRef.Name, m.Name); err != nil {
311+
conditions.MarkFalse(m, clusterv1.DrainingSucceededCondition, clusterv1.DrainingFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
282312
r.recorder.Eventf(m, corev1.EventTypeWarning, "FailedDrainNode", "error draining Machine's node %q: %v", m.Status.NodeRef.Name, err)
283313
return ctrl.Result{}, err
284314
}
315+
316+
conditions.MarkTrue(m, clusterv1.DrainingSucceededCondition)
285317
r.recorder.Eventf(m, corev1.EventTypeNormal, "SuccessfulDrainNode", "success draining Machine's node %q", m.Status.NodeRef.Name)
286318
}
287319
}
@@ -292,9 +324,14 @@ func (r *MachineReconciler) reconcileDelete(ctx context.Context, cluster *cluste
292324
return ctrl.Result{}, nil
293325
}
294326

295-
if ok, err := r.reconcileDeleteExternal(ctx, m); !ok || err != nil {
296-
// Return early and don't remove the finalizer if we got an error or
297-
// the external reconciliation deletion isn't ready.
327+
// Return early and don't remove the finalizer if we got an error or
328+
// the external reconciliation deletion isn't ready.
329+
330+
if ok, err := r.reconcileDeleteInfrastructure(ctx, m); !ok || err != nil {
331+
return ctrl.Result{}, err
332+
}
333+
334+
if ok, err := r.reconcileDeleteBootstrap(ctx, m); !ok || err != nil {
298335
return ctrl.Result{}, err
299336
}
300337

@@ -441,41 +478,70 @@ func (r *MachineReconciler) deleteNode(ctx context.Context, cluster *clusterv1.C
441478
return nil
442479
}
443480

444-
// reconcileDeleteExternal tries to delete external references, returning true if it cannot find any.
445-
func (r *MachineReconciler) reconcileDeleteExternal(ctx context.Context, m *clusterv1.Machine) (bool, error) {
446-
objects := []*unstructured.Unstructured{}
447-
references := []*corev1.ObjectReference{
448-
m.Spec.Bootstrap.ConfigRef,
449-
&m.Spec.InfrastructureRef,
481+
func (r *MachineReconciler) reconcileDeleteBootstrap(ctx context.Context, m *clusterv1.Machine) (bool, error) {
482+
obj, err := r.reconcileDeleteExternal(ctx, m, m.Spec.Bootstrap.ConfigRef)
483+
if err != nil {
484+
return false, err
450485
}
451486

452-
// Loop over the references and try to retrieve it with the client.
453-
for _, ref := range references {
454-
if ref == nil {
455-
continue
456-
}
487+
if obj == nil {
488+
// Marks the bootstrap as deleted
489+
conditions.MarkFalse(m, clusterv1.BootstrapReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
490+
return true, nil
491+
}
457492

458-
obj, err := external.Get(ctx, r.Client, ref, m.Namespace)
459-
if err != nil && !apierrors.IsNotFound(errors.Cause(err)) {
460-
return false, errors.Wrapf(err, "failed to get %s %q for Machine %q in namespace %q",
461-
ref.GroupVersionKind(), ref.Name, m.Name, m.Namespace)
462-
}
463-
if obj != nil {
464-
objects = append(objects, obj)
465-
}
493+
// Report a summary of current status of the bootstrap object defined for this machine.
494+
conditions.SetMirror(m, clusterv1.BootstrapReadyCondition,
495+
conditions.UnstructuredGetter(obj),
496+
conditions.WithFallbackValue(false, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, ""),
497+
)
498+
return false, nil
499+
}
500+
501+
func (r *MachineReconciler) reconcileDeleteInfrastructure(ctx context.Context, m *clusterv1.Machine) (bool, error) {
502+
obj, err := r.reconcileDeleteExternal(ctx, m, &m.Spec.InfrastructureRef)
503+
if err != nil {
504+
return false, err
505+
}
506+
507+
if obj == nil {
508+
// Marks the infrastructure as deleted
509+
conditions.MarkFalse(m, clusterv1.InfrastructureReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
510+
return true, nil
511+
}
512+
513+
// Report a summary of current status of the bootstrap object defined for this machine.
514+
conditions.SetMirror(m, clusterv1.InfrastructureReadyCondition,
515+
conditions.UnstructuredGetter(obj),
516+
conditions.WithFallbackValue(false, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, ""),
517+
)
518+
return false, nil
519+
}
520+
521+
// reconcileDeleteExternal tries to delete external references.
522+
func (r *MachineReconciler) reconcileDeleteExternal(ctx context.Context, m *clusterv1.Machine, ref *corev1.ObjectReference) (*unstructured.Unstructured, error) {
523+
if ref == nil {
524+
return nil, nil
525+
}
526+
527+
// get the external object
528+
obj, err := external.Get(ctx, r.Client, ref, m.Namespace)
529+
if err != nil && !apierrors.IsNotFound(errors.Cause(err)) {
530+
return nil, errors.Wrapf(err, "failed to get %s %q for Machine %q in namespace %q",
531+
ref.GroupVersionKind(), ref.Name, m.Name, m.Namespace)
466532
}
467533

468-
// Issue a delete request for any object that has been found.
469-
for _, obj := range objects {
534+
if obj != nil {
535+
// Issue a delete request.
470536
if err := r.Client.Delete(ctx, obj); err != nil && !apierrors.IsNotFound(err) {
471-
return false, errors.Wrapf(err,
537+
return obj, errors.Wrapf(err,
472538
"failed to delete %v %q for Machine %q in namespace %q",
473539
obj.GroupVersionKind(), obj.GetName(), m.Name, m.Namespace)
474540
}
475541
}
476542

477543
// Return true if there are no more external objects.
478-
return len(objects) == 0, nil
544+
return obj, nil
479545
}
480546

481547
func (r *MachineReconciler) shouldAdopt(m *clusterv1.Machine) bool {

0 commit comments

Comments
 (0)