Skip to content
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.

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

Move logic to evict on Workload deactivation to the Workload controller #1841

Open
alculquicondor opened this issue Mar 14, 2024 · 3 comments
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@alculquicondor
Copy link
Contributor

What would you like to be cleaned:

The logic is currently in the job reconciler:

if !ptr.Deref(wl.Spec.Active, true) {
workload.SetEvictedCondition(wl, kueue.WorkloadEvictedByDeactivation, "The workload is deactivated")
err := workload.ApplyAdmissionStatus(ctx, r.client, wl, true)
if err != nil {
return ctrl.Result{}, fmt.Errorf("setting eviction: %w", err)
}
return ctrl.Result{}, nil
}

Why is this needed:

  1. The logic is fully independent of the job
  2. If an integration decides not to use the job reconciler framework, they would have to re-implement this logic.
@alculquicondor alculquicondor added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Mar 14, 2024
@tenzen-y
Copy link
Member

/assign

@tenzen-y
Copy link
Member

First of all, I will try to investigate the reason why we added this logic to the jobs controller.

@tenzen-y
Copy link
Member

kueue/pkg/controller/jobframework/reconciler.go

I sought #1252, but I could not find any reasonable reason to put this logic on the job framework-controller.

So, I'll try to move it to the workload-controller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

No branches or pull requests

2 participants