Skip to content

[control-operator] finalizer blocks deletion of task until all pods are gone #830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
justonedev1 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[control-operator] finalizer blocks deletion of task until all pods are gone #830

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b26f2a
[control-operator] finalizer blocks deletion of task until all pods a…
justonedev1 Jul 3, 2026
ca5ca50
fixup! [control-operator] finalizer blocks deletion of task until all…
justonedev1 Jul 14, 2026
d4bd623
fixup! fixup! [control-operator] finalizer blocks deletion of task un…
justonedev1 Aug 6, 2026
ca235da
fixup! fixup! fixup! [control-operator] finalizer blocks deletion of …
justonedev1 Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 52 additions & 22 deletions control-operator/internal/controller/task_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,35 +270,65 @@ func (r *TaskReconciler) consumeGRPCConsumerIfReady(ctx context.Context, t *alie
return ctrl.Result{}
}

// handleFinalizer sets the finalizer, when Task isn't marked for deletion, and when deletion reconcile is triggered
// it then clears the gRPC connection and deletes the underlying POD and doesn't allow Task to be deleted
// until pod is gone.
// Note: if you add finalizer to a task you cannot delete it unless you remove the finalizer, run:
// kubectl patch task --type=json -p='[{"op": "remove", "path": "/metadata/finalizers"}]'
func (r *TaskReconciler) handleFinalizer(ctx context.Context, t *aliecsv1alpha1.Task, log logr.Logger) (ctrl.Result, bool, error) {
func (r *TaskReconciler) handleFinalizer(ctx context.Context, t *aliecsv1alpha1.Task, log logr.Logger) (result ctrl.Result, stopReconciliation bool, err error) {
if t.DeletionTimestamp.IsZero() {
if !controllerutil.ContainsFinalizer(t, taskFinalizer) {
controllerutil.AddFinalizer(t, taskFinalizer)
if err := r.Update(ctx, t); err != nil {
return ctrl.Result{}, true, err
}
return ctrl.Result{}, true, nil
if controllerutil.AddFinalizer(t, taskFinalizer) {
return ctrl.Result{}, true, r.Update(ctx, t)
}
} else {
if controllerutil.ContainsFinalizer(t, taskFinalizer) {
log.Info("Cleaning up gRPC connection before deletion")
if client, exists := clientsForContainers[t.Name]; exists {
if err := client.Close(); err != nil {
log.Error(err, "Failed to close gRPC client during deletion")
}
delete(clientsForContainers, t.Name)
}
return ctrl.Result{}, false, nil
}

controllerutil.RemoveFinalizer(t, taskFinalizer)
if err := r.Update(ctx, t); err != nil {
return ctrl.Result{}, true, err
}
}
if !controllerutil.ContainsFinalizer(t, taskFinalizer) {
return ctrl.Result{}, true, nil
}

@knopers8 knopers8 Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which situation could lead us to this case, i.e. scheduled for deletion, but no finalizer? Given that we ask Reconcile to return, won't it result in an infinite loop?

@justonedev1 justonedev1 Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would not result in infinite loop because it can happen only when the object was finalised but not deleted, aka in a process of deletion. Basically indempotency check. From the point of view of this controller this object is done/gone and shouldn't be acted up. But Reconciliation can be triggered by k8s and this causes Reconcile loop for given object to be no-op as it should be

return ctrl.Result{}, false, nil

log.Info("Finalizer found, starting cleanup")

@knopers8 knopers8 Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.Info("Finalizer found, starting cleanup")
log.Info("Deletion timestamp exists and finalizer found, starting cleanup")

I would propose to be more clear about conditions for starting cleanup, so one is not misled into thinking that it's enough to have finalizer present to start cleanup.

@justonedev1 justonedev1 Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timestamp is automatically added by k8s, not by us when there is a request to delete any resource, so in my opinion it isn't necessary to repeat it.

r.cleargRPC(t, log)

if podDeleted, err := r.deletePod(ctx, t, log); err != nil || !podDeleted {
return ctrl.Result{}, true, err
}

controllerutil.RemoveFinalizer(t, taskFinalizer)
return ctrl.Result{}, true, r.Update(ctx, t)
}

// deletePod deletes the Task's Pod if it still exists, and reports whether it is
// fully gone yet so the caller can wait for termination before removing the finalizer.
func (r *TaskReconciler) deletePod(ctx context.Context, t *aliecsv1alpha1.Task, log logr.Logger) (podDeleted bool, err error) {
pod := &v1.Pod{}
err = r.Get(ctx, types.NamespacedName{Name: podNameFromTask(t.Name), Namespace: t.Namespace}, pod)
if errors.IsNotFound(err) {
log.Info("Pod deleted")
return true, nil
} else if err != nil {
return false, err
}

if pod.DeletionTimestamp.IsZero() {
log.Info("Deleting pod before removing finalizer")
if err := r.Delete(ctx, pod); err != nil && !errors.IsNotFound(err) {
return false, err
}
}
log.Info("Waiting for pod to terminate before removing finalizer")
return false, nil
}

func (*TaskReconciler) cleargRPC(t *aliecsv1alpha1.Task, log logr.Logger) {
if client, exists := clientsForContainers[t.Name]; exists {
log.Info("Cleaning up gRPC connection")
if err := client.Close(); err != nil {
log.Error(err, "Failed to close gRPC client during deletion")
}
delete(clientsForContainers, t.Name)
log.Info("gRPC cleaned")
}
}

func podNameFromTask(name string) string {
Expand Down
Loading