Skip to content

Commit fc03fea

Browse files
fabriziopandinifabriziopandini
committed
conditions for reconcile delete
1 parent 3881021 commit fc03fea

File tree

12 files changed

+350
-158
lines changed

12 files changed

+350
-158
lines changed

‎api/v1alpha3/condition_consts.go

+9
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

‎controllers/cluster_controller.go

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

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

@@ -142,7 +134,7 @@ func (r *ClusterReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
142134
if reterr == nil {
143135
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
144136
}
145-
if err := patchHelper.Patch(ctx, cluster, patchOpts...); err != nil {
137+
if err := patchCluster(ctx, patchHelper, cluster, patchOpts...); err != nil {
146138
reterr = kerrors.NewAggregate([]error{reterr, err})
147139
}
148140
}()
@@ -162,6 +154,28 @@ func (r *ClusterReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
162154
return r.reconcile(ctx, cluster)
163155
}
164156

157+
func patchCluster(ctx context.Context, patchHelper *patch.Helper, cluster *clusterv1.Cluster, options ...patch.Option) error {
158+
// Always update the readyCondition by summarizing the state of other conditions.
159+
conditions.SetSummary(cluster,
160+
conditions.WithConditions(
161+
clusterv1.ControlPlaneReadyCondition,
162+
clusterv1.InfrastructureReadyCondition,
163+
),
164+
)
165+
166+
// Patch the object, ignoring conflicts on the conditions owned by this controller.
167+
// Also, if requested, we are adding additional options like e.g. Patch ObservedGeneration when issuing the
168+
// patch at the end of the reconcile loop.
169+
options = append(options,
170+
patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
171+
clusterv1.ReadyCondition,
172+
clusterv1.ControlPlaneReadyCondition,
173+
clusterv1.InfrastructureReadyCondition,
174+
}},
175+
)
176+
return patchHelper.Patch(ctx, cluster, options...)
177+
}
178+
165179
// reconcile handles cluster reconciliation.
166180
func (r *ClusterReconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster) (ctrl.Result, error) {
167181
logger := r.Log.WithValues("cluster", cluster.Name, "namespace", cluster.Namespace)
@@ -253,11 +267,18 @@ func (r *ClusterReconciler) reconcileDelete(ctx context.Context, cluster *cluste
253267
switch {
254268
case apierrors.IsNotFound(errors.Cause(err)):
255269
// All good - the control plane resource has been deleted
270+
conditions.MarkFalse(cluster, clusterv1.ControlPlaneReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
256271
case err != nil:
257272
return reconcile.Result{}, errors.Wrapf(err, "failed to get %s %q for Cluster %s/%s",
258273
path.Join(cluster.Spec.ControlPlaneRef.APIVersion, cluster.Spec.ControlPlaneRef.Kind),
259274
cluster.Spec.ControlPlaneRef.Name, cluster.Namespace, cluster.Name)
260275
default:
276+
// Report a summary of current status of the control plane object defined for this cluster.
277+
conditions.SetMirror(cluster, clusterv1.ControlPlaneReadyCondition,
278+
conditions.UnstructuredGetter(obj),
279+
conditions.WithFallbackValue(false, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, ""),
280+
)
281+
261282
// Issue a deletion request for the control plane object.
262283
// Once it's been deleted, the cluster will get processed again.
263284
if err := r.Client.Delete(ctx, obj); err != nil {
@@ -277,11 +298,18 @@ func (r *ClusterReconciler) reconcileDelete(ctx context.Context, cluster *cluste
277298
switch {
278299
case apierrors.IsNotFound(errors.Cause(err)):
279300
// All good - the infra resource has been deleted
301+
conditions.MarkFalse(cluster, clusterv1.InfrastructureReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
280302
case err != nil:
281303
return ctrl.Result{}, errors.Wrapf(err, "failed to get %s %q for Cluster %s/%s",
282304
path.Join(cluster.Spec.InfrastructureRef.APIVersion, cluster.Spec.InfrastructureRef.Kind),
283305
cluster.Spec.InfrastructureRef.Name, cluster.Namespace, cluster.Name)
284306
default:
307+
// Report a summary of current status of the infrastructure object defined for this cluster.
308+
conditions.SetMirror(cluster, clusterv1.InfrastructureReadyCondition,
309+
conditions.UnstructuredGetter(obj),
310+
conditions.WithFallbackValue(false, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, ""),
311+
)
312+
285313
// Issue a deletion request for the infrastructure object.
286314
// Once it's been deleted, the cluster will get processed again.
287315
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)
@@ -270,20 +289,38 @@ func (r *MachineReconciler) reconcileDelete(ctx context.Context, cluster *cluste
270289
}
271290

272291
if isDeleteNodeAllowed {
273-
// Drain node before deletion.
292+
// Drain node before deletion and issue a patch in order to make this operation visible to the users.
274293
if _, exists := m.ObjectMeta.Annotations[clusterv1.ExcludeNodeDrainingAnnotation]; !exists {
294+
patchHelper, err := patch.NewHelper(m, r.Client)
295+
if err != nil {
296+
return ctrl.Result{}, err
297+
}
298+
275299
logger.Info("Draining node", "node", m.Status.NodeRef.Name)
300+
conditions.MarkFalse(m, "DrainingSucceeded", "Draining", clusterv1.ConditionSeverityInfo, "Draining the node before deletion")
301+
if err := patchMachine(ctx, patchHelper, m); err != nil {
302+
return ctrl.Result{}, errors.Wrap(err, "failed to patch Machine")
303+
}
304+
276305
if err := r.drainNode(ctx, cluster, m.Status.NodeRef.Name, m.Name); err != nil {
306+
conditions.MarkFalse(m, "DrainingSucceeded", "DrainingError", clusterv1.ConditionSeverityWarning, err.Error())
277307
r.recorder.Eventf(m, corev1.EventTypeWarning, "FailedDrainNode", "error draining Machine's node %q: %v", m.Status.NodeRef.Name, err)
278308
return ctrl.Result{}, err
279309
}
310+
311+
conditions.MarkTrue(m, "DrainingSucceeded")
280312
r.recorder.Eventf(m, corev1.EventTypeNormal, "SuccessfulDrainNode", "success draining Machine's node %q", m.Status.NodeRef.Name)
281313
}
282314
}
283315

284-
if ok, err := r.reconcileDeleteExternal(ctx, m); !ok || err != nil {
285-
// Return early and don't remove the finalizer if we got an error or
286-
// the external reconciliation deletion isn't ready.
316+
// Return early and don't remove the finalizer if we got an error or
317+
// the external reconciliation deletion isn't ready.
318+
319+
if ok, err := r.reconcileDeleteInfrastructure(ctx, m); !ok || err != nil {
320+
return ctrl.Result{}, err
321+
}
322+
323+
if ok, err := r.reconcileDeleteBootstrap(ctx, m); !ok || err != nil {
287324
return ctrl.Result{}, err
288325
}
289326

@@ -430,41 +467,70 @@ func (r *MachineReconciler) deleteNode(ctx context.Context, cluster *clusterv1.C
430467
return nil
431468
}
432469

433-
// reconcileDeleteExternal tries to delete external references, returning true if it cannot find any.
434-
func (r *MachineReconciler) reconcileDeleteExternal(ctx context.Context, m *clusterv1.Machine) (bool, error) {
435-
objects := []*unstructured.Unstructured{}
436-
references := []*corev1.ObjectReference{
437-
m.Spec.Bootstrap.ConfigRef,
438-
&m.Spec.InfrastructureRef,
470+
func (r *MachineReconciler) reconcileDeleteBootstrap(ctx context.Context, m *clusterv1.Machine) (bool, error) {
471+
obj, err := r.reconcileDeleteExternal(ctx, m, m.Spec.Bootstrap.ConfigRef)
472+
if err != nil {
473+
return false, err
439474
}
440475

441-
// Loop over the references and try to retrieve it with the client.
442-
for _, ref := range references {
443-
if ref == nil {
444-
continue
445-
}
476+
if obj == nil {
477+
// Marks the bootstrap as deleted
478+
conditions.MarkFalse(m, clusterv1.BootstrapReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
479+
return true, nil
480+
}
446481

447-
obj, err := external.Get(ctx, r.Client, ref, m.Namespace)
448-
if err != nil && !apierrors.IsNotFound(errors.Cause(err)) {
449-
return false, errors.Wrapf(err, "failed to get %s %q for Machine %q in namespace %q",
450-
ref.GroupVersionKind(), ref.Name, m.Name, m.Namespace)
451-
}
452-
if obj != nil {
453-
objects = append(objects, obj)
454-
}
482+
// Report a summary of current status of the bootstrap object defined for this machine.
483+
conditions.SetMirror(m, clusterv1.BootstrapReadyCondition,
484+
conditions.UnstructuredGetter(obj),
485+
conditions.WithFallbackValue(false, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, ""),
486+
)
487+
return false, nil
488+
}
489+
490+
func (r *MachineReconciler) reconcileDeleteInfrastructure(ctx context.Context, m *clusterv1.Machine) (bool, error) {
491+
obj, err := r.reconcileDeleteExternal(ctx, m, &m.Spec.InfrastructureRef)
492+
if err != nil {
493+
return false, err
494+
}
495+
496+
if obj == nil {
497+
// Marks the infrastructure as deleted
498+
conditions.MarkFalse(m, clusterv1.InfrastructureReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
499+
return true, nil
500+
}
501+
502+
// Report a summary of current status of the bootstrap object defined for this machine.
503+
conditions.SetMirror(m, clusterv1.InfrastructureReadyCondition,
504+
conditions.UnstructuredGetter(obj),
505+
conditions.WithFallbackValue(false, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, ""),
506+
)
507+
return false, nil
508+
}
509+
510+
// reconcileDeleteExternal tries to delete external references.
511+
func (r *MachineReconciler) reconcileDeleteExternal(ctx context.Context, m *clusterv1.Machine, ref *corev1.ObjectReference) (*unstructured.Unstructured, error) {
512+
if ref == nil {
513+
return nil, nil
514+
}
515+
516+
// get the external object
517+
obj, err := external.Get(ctx, r.Client, ref, m.Namespace)
518+
if err != nil && !apierrors.IsNotFound(errors.Cause(err)) {
519+
return nil, errors.Wrapf(err, "failed to get %s %q for Machine %q in namespace %q",
520+
ref.GroupVersionKind(), ref.Name, m.Name, m.Namespace)
455521
}
456522

457-
// Issue a delete request for any object that has been found.
458-
for _, obj := range objects {
523+
if obj != nil {
524+
// Issue a delete request.
459525
if err := r.Client.Delete(ctx, obj); err != nil && !apierrors.IsNotFound(err) {
460-
return false, errors.Wrapf(err,
526+
return obj, errors.Wrapf(err,
461527
"failed to delete %v %q for Machine %q in namespace %q",
462528
obj.GroupVersionKind(), obj.GetName(), m.Name, m.Namespace)
463529
}
464530
}
465531

466532
// Return true if there are no more external objects.
467-
return len(objects) == 0, nil
533+
return obj, nil
468534
}
469535

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

0 commit comments

Comments
 (0)