Pending Admission Check still holds resources

[leipanhz]

To admit a workload, Kueue scheduler checks quota reservation and all admission checks. Only when all conditions are met, the workload is admitted. In my experiment with a custom admission controller, when admission check is not passed (under pending status): although workload is not admitted, it is still holding CPU/GPU/Memory resources. This is not desired as we want the resource to be allocated for other jobs during admission pending stage.

Any comments on how to address this issue?

[leipanhz]

Separately, I experimented with sending AdmissionCheck to be "retry" when the check didn't pass. In such case, workloads created after it are unblocked, and able to run. However, the first workload was failed and evicted without getting retry.

The workload details are in the following:

Status:
Admission Checks:
Last Transition Time: 2024-10-29T22:17:19Z
Message: The workload needs to retry Admission Check later
Name: ac
State: Retry
Conditions:
Last Transition Time: 2024-10-29T22:17:19Z
Message: The workload has failed admission checks
Observed Generation: 1
Reason: Pending
Status: False
Type: QuotaReserved
Last Transition Time: 2024-10-29T22:17:19Z
Message: At least one admission check is false
Observed Generation: 1
Reason: AdmissionCheck
Status: True
Type: Evicted
Last Transition Time: 2024-10-29T22:17:19Z
Message: At least one admission check is false
Observed Generation: 1
Reason: AdmissionCheck
Status: True
Type: Requeued
Events:
Type Reason Age From Message

Normal QuotaReserved 24s kueue-admission Quota reserved in ClusterQueue cluster-queue, wait time since queued was 1s
Normal EvictedDueToAdmissionCheck 24s kueue-workload-controller At least one admission check is false
Normal Pending 9s (x3 over 24s) kueue-admission The workload has failed admission checks

[mimowo]

I see, the culprit seems to be that the Retry is still there, and so the Workload gets constantly evicted. @PBundyra has fixed this here: #3323. So IIUC your options are:

• use the main branch
• wait for 0.9 release (Release v0.9.0 #3302)
• reset to Pending the admission check yourself in the controller after eviction

[leipanhz]

@mimowo @PBundyra

1. regarding: "reset to Pending the admission check yourself in the controller after eviction" Would this work only with main or release v0.9?
2. when will v0.9 be ready?
   Thank you

[tenzen-y]

1. This indicates that you need to fix this bug yourself in your code. So, this solution can be implemented with v0.8.
2. The current candidate is 4th Nov.

[leipanhz]

1. This indicates that you need to fix this bug yourself in your code. So, this solution can be implemented with v0.8.
2. The current candidate is 4th Nov.

@tenzen-y Can you please elaborate how to fix this bug myself without upgrading Kueue version? I did try to reset the status from retry to pending in my controller, but that doesn't work. The workload was evicted and requeued, admission check was set to pending, resource was reserved, but job wasn't admitted.

Admission Checks:
Last Transition Time: 2024-10-31T18:56:25Z
Message: The workload is pending on Admission Check
Name: ac
State: Pending
Conditions:
Last Transition Time: 2024-10-31T18:56:25Z
Message: Quota reserved in ClusterQueue cluster-queue
Observed Generation: 1
Reason: QuotaReserved
Status: True
Type: QuotaReserved
Last Transition Time: 2024-10-31T18:56:25Z
Message: Previously: At least one admission check is false
Observed Generation: 1
Reason: QuotaReserved
Status: False
Type: Evicted
Last Transition Time: 2024-10-31T18:55:53Z
Message: At least one admission check is false
Observed Generation: 1
Reason: AdmissionCheck
Status: True
Type: Requeued

Events:
Type Reason Age From Message
Normal QuotaReserved 77s kueue-admission Quota reserved in ClusterQueue cluster-queue, wait time since queued was 0s
Normal EvictedDueToAdmissionCheck 77s kueue-workload-controller At least one admission check is false
Normal Pending 59s (x3 over 77s) kueue-admission The workload has failed admission checks
Normal QuotaReserved 45s kueue-admission Quota reserved in ClusterQueue cluster-queue, wait time since queued was 32s

[tenzen-y]

TBH, I do not think that you should fix the bug by yourself since we cherry-pick the bug fixes to the release-0.8 branch yesterday.

#3395

So, could you try to use the latest release-0.8 branch?

[leipanhz]

TBH, I do not think that you should fix the bug by yourself since we cherry-pick the bug fixes to the release-0.8 branch yesterday.

#3395
So, could you try to use the latest release-0.8 branch?

My ask is based on the previous suggestion to fix bug myself. So, I tried with the main branch that includes the fix, unfortunately the same behavior was observed: After the admission check changes from retry to pending, workload is not admitted.

[leipanhz]

In addition regarding the bug and fix: if the fix is to reset admission check to pending in the controller after eviction, does it address my initial question? Quote here:
when admission check is not passed (under pending status): although workload is not admitted, it is still holding CPU/GPU/Memory resources.

We expect resources (GPU/Memory, etc.) are not allocated until all admission checks pass. How can this addressed?

[leipanhz]

In addition regarding the bug and fix: if the fix is to reset admission check to pending in the controller after eviction, does it address my initial question? Quote here: when admission check is not passed (under pending status): although workload is not admitted, it is still holding CPU/GPU/Memory resources.

We expect resources (GPU/Memory, etc.) are not allocated until all admission checks pass. How can this addressed?

Rechecking this: it seems when admission check is under "retry", resource will not be reserved? If this is the case, it is good. The question left is how to retry a workload after it's evicted and requeued.

[leipanhz]

So, I tried with the main branch that includes the fix, unfortunately the same behavior was observed: After the admission check changes from retry to pending, workload is not admitted.

I can try again with the latest release v0.9 when it's out.

[leipanhz]

@tenzen-y @mimowo I tried with the latest release v0.8.2 that includes the workload change, however, same issues are still observed:

1. Once workload is set to retry, the admission check is failed and won't be admitted
2. However, workload shows the quota has been reserved even though admission check is failed

Status:
Admission Checks:
Last Transition Time: 2024-11-06T16:58:06Z
Message: The workload needs to retry Admission Check later
Name: ac
State: Retry
Conditions:
Last Transition Time: 2024-11-06T16:58:02Z
Message: The workload has failed admission checks
Observed Generation: 1
Reason: Pending
Status: False
Type: QuotaReserved
Events:
Type Reason Age From Message

Normal Pending 9m14s kueue-admission couldn't assign flavors to pod set master: insufficient unused quota for cpu in flavor default-flavor, 200m more needed, insufficient unused quota for memory in flavor default-flavor, 176Mi more needed
Normal Pending 8m50s (x4 over 9m10s) kueue-admission The workload has failed admission checks

[mimowo]

@leipanhz it is a bit hard to tell what is going on because this is a custom AC.

Once workload is set to retry, the admission check is failed and won't be admitted

IIUC your explanation we don't observe this behavior in the built-in ACs like ProvReq - for example in this integration test we check can be set to Ready based on the ProvReq status.

I suspect maybe your custom AC controller sets the check to Retry on the second attempt, but hard to tell.

[leipanhz]

@mimowo

The retry is set based on the following:

		if (logic to check admission) {
			log.V(2).Info("Set status to be retry")
			workload.SetAdmissionCheckState(&wlPatch.Status.AdmissionChecks, acStateRetry)
			err := r.Status().Patch(ctx, wlPatch, client.Apply, client.FieldOwner(PrefetchACCName), client.ForceOwnership)
			if err != nil {
				log.V(2).Error(err, "failed to update admission check status to retry")
			}
			return ctrl.Result{RequeueAfter: waitingPeriod}, err
		} ...

It is possible the controller will set retry more than once for a single workload, would that be in issue?

[leipanhz]

@mimowo In addition, could you please clarify the following:
However, workload shows the quota has been reserved even though admission check is failed or pending
When admission check isn't ready, we don't want the quota to be reserved, otherwise it beats the purpose of having kueue scheduler if the resource is on hold during waiting period.

Admission Checks:
Last Transition Time: 2024-11-06T17:57:00Z
Message: The workload is pending on Admission Check
Name: ac
State: Pending
Conditions:
Last Transition Time: 2024-11-06T17:57:00Z
Message: Quota reserved in ClusterQueue cluster-queue
Observed Generation: 1
Reason: QuotaReserved
Status: True
Type: QuotaReserved

[mimowo]

It is possible the controller will set retry more than once for a single workload, would that be in issue?

I suspect this is the case here - that your AC goes into a loop of:
QuotaReserved=True, Pending -> QuotaReserved=True, Retry -> QuotaReserved=False,Pending -> QuotaReserved=True,Pending

so that on the consecutive runs is still sets Retry (but this is a guess). It would be best if you inspect the logs or use debugger.

When admission check isn't ready, we don't want the quota to be reserved

I see, but this is currently WAI, not a bug. It would require a new feature extension to allow that.
For now we have two phases:

1. quota check -> results in QuotaReserved=True
2. extra admission checks -> result in Admitted=True

So, it is important to make sure phase 2 is not too long so that (as you say) they resources don't stay unused for too long.

[leipanhz]

this is currently WAI, not a bug. It would require a new feature extension to allow that.

@mimowo I'd like to request a feature not to reserve resources until all conditions are met (admission checks, resource reservation). This is the main motivation we are integrating with Kueue for resource efficiency. It's likely other applications have such needs too.

In the meantime, do you know if there is any workaround can be applied? Thank you

[mimowo]

In the meantime, do you know if there is any workaround can be applied?

Can you explain higher level what is your use case, and what the external AC is for? The proposed workaround may depend on that.

[leipanhz]

In the meantime, do you know if there is any workaround can be applied?

Can you explain higher level what is your use case, and what the external AC is for? The proposed workaround may depend on that.

The idea is the following:

For some of our jobs, in the beginning there is a task (pre-task) that is depending on some external services to be ready, which could take long time. Instead of holding cluster resources, a better way is to extract the pre-task and run it separately in the AC controller. Only when the pre-task is complete, then we allow the workload to move on with resource reservation and workload admission.

[mimowo]

I see, could you use two jobs / workloads for that? One job represents the pre-task, and another the main task. You need to watch the pre-task, once finished you create the main job?

[leipanhz]

Do you mean user submits two jobs? This is not considered currently :( as the goal of having kueue admission check is to automate the whole process so there are minimum changes on the user end.

[mimowo]

I see, good point. Yes, I meant two Jobs. Still, the pre-task job could be created by your in-house code (for example in a webhook). Then, the webhook would need to also temporarily remove the "queue-name" label so that it is not scheduled by Kueue. Once the pre-task is finished you could add-back the "queue-name" label (probably another controller) which would trigger scheduling by Kueue. This is many steps, I know, but you asked for a workaround :).

[leipanhz]

I see, good point. Yes, I meant two Jobs. Still, the pre-task job could be created by your in-house code (for example in a webhook). Then, the webhook would need to also temporarily remove the "queue-name" label so that it is not scheduled by Kueue. Once the pre-task is finished you could add-back the "queue-name" label (probably another controller) which would trigger scheduling by Kueue. This is many steps, I know, but you asked for a workaround :).

Not sure if we want to take this approach, will think about it.

Reading your previous comment and have a question:

It is possible the controller will set retry more than once for a single workload, would that be in issue?

Does that imply the ac can only retry once and will evict the workload for the second retry? Is it possible to set max retry times to a configurable number to allow retry multiple times?

[mimowo]

Does that imply the ac can only retry once and will evict the workload for the second retry? Is it possible to set max retry times to a configurable number to allow retry multiple times?

We don't have a retry config for all ACs yet, this is yet another feature request, or you could do it yourself in your AC controller.
We recently introduced it for ProvisioningRequest AC: #3375, so you could probably follow the pattern, but this is non-trivial, I know.

[PBundyra]

Hi @leipanhz

Does that imply the ac can only retry once and will evict the workload for the second retry? Is it possible to set max retry times to a configurable number to allow retry multiple times?

There is no limit on how many retries a workload can do. We implemented a cap on admissioncheck controller side to on setting Retry condition, but there is nothing preventing the workload controller to evict a Workload over and over

I believe what mimowo meant in this comment:

It is possible the controller will set retry more than once for a single workload, would that be in issue?

is that your admission check controller retries the check over and over in the infinite loop.
So in the code snippet you provided

		if (logic to check admission) {
			log.V(2).Info("Set status to be retry")
			workload.SetAdmissionCheckState(&wlPatch.Status.AdmissionChecks, acStateRetry)
			err := r.Status().Patch(ctx, wlPatch, client.Apply, client.FieldOwner(PrefetchACCName), client.ForceOwnership)
			if err != nil {
				log.V(2).Error(err, "failed to update admission check status to retry")
			}
			return ctrl.Result{RequeueAfter: waitingPeriod}, err
		} ...

First if remains true right after retry, so during next reconcile loop it will also try to retry the workload. Might this be the case?

[mimowo]

There is no limit on how many retries a workload can do. We implemented a cap on admissioncheck controller side to on setting Retry condition, but there is nothing preventing the workload controller to evict a Workload over and over

Ok, so once the cap is exceeded you should set the AC state to Rejected. This will prevent the workload controller to re-attempt. This deactivates the AC.

First if remains true right after retry, so during next reconcile loop it will also try to retry the workload. Might this be the case?

This is what I suspect, you may confirm this with logs checking I believe.

I think the logic should be like:

canAdmit, canRetry := canAdmit()
if canAdmit {
  // logic to set the AC to Ready
} else if canRetry {
  log.V(2).Info("Set status to be retry")
  workload.SetAdmissionCheckState(&wlPatch.Status.AdmissionChecks, acStateRetry)
  err := r.Status().Patch(ctx, wlPatch, client.Apply, client.FieldOwner(PrefetchACCName), client.ForceOwnership)
  if err != nil {
    log.V(2).Error(err, "failed to update admission check status to retry")
  }
  return ctrl.Result{RequeueAfter: waitingPeriod}, err
} else { 
  // cannot admit and cannot retry
  // logic to set the AC to Rejected
}

[PBundyra]

Also an important consideration is whether the pre-task you mention run any pods? Does it keep running it even after retry? If so, then this may impact holding reserved quota even after AdmissionCheck was set to Retry and the Workload was evicted

[leipanhz]

Hi @mimowo @PBundyra

We recently introduced it for ProvisioningRequest AC: #3375, so you could probably follow the pattern

Thank you for the pointer, I will take a look at it.

There is no limit on how many retries a workload can do. We implemented a cap on admissioncheck controller side to on setting Retry condition,

Can you provide the link of code that implements this?

is that your admission check controller retries the check over and over in the infinite loop.

No, after certain time, the admission check will pass. Simply changing Retry to Pending, the admission check will be Ready eventually (but Pending state reserves quota even though AC doesn't pass, we can't allow that).
In our case, we need to periodically checking admission, if it doesn't pass, then keep retrying, based on what @mimowo, if workload is retried for more than once, it will fail permanently. Is it the case?

Also an important consideration is whether the pre-task you mention run any pods? Does it keep running it even after retry?

No, pre-task queries status of another service and doesn't run pod.

I think the logic should be like:...

This is what's implemented in my code, with canadmit = if (logic to check admission), can retry = else., see my previous post

        if (logic to check admission) {
		log.V(2).Info("Set status to be retry")
		workload.SetAdmissionCheckState(&wlPatch.Status.AdmissionChecks, acStateRetry)
		err := r.Status().Patch(ctx, wlPatch, client.Apply, client.FieldOwner(PrefetchACCName), client.ForceOwnership)
		if err != nil {
			log.V(2).Error(err, "failed to update admission check status to retry")
		}
		return ctrl.Result{RequeueAfter: waitingPeriod}, err
	} else {
			log.V(2).Info("Set status to be ready")
			workload.SetAdmissionCheckState(&wlPatch.Status.AdmissionChecks, acStateReady)
			...
       }

Please correct if I am wrong

[leipanhz]

Hi @mimowo @PBundyra: I tested AdmissionCheck Retry status with the following scenario with 3 jobs: Job1, 2, 3 are submitted sequentially; cluster can only handle one job at a time.

• Job1: no Admission Check; job is admitted right away. It takes 10s to finish.
• Job2: has Admission Check; needs to wait 30 seconds before AC can pass (During waiting time, AC status is set to Retry)
• Job3: no Admission Check; job is suspended after submission due to insufficient quota/resources. It takes 10s to finish

Desired result:
Once Job 1 is finished, Job 3 will be admitted. After Job3 is finished, Job 2 will be admitted.

Observed:

• Job 3 is admitted upon finish of Job 1, as expected.

• As soon as Job 3 is admitted, Job2's status becomes "failed admission check". It was never re-admitted. This is done in scheduler.go. AC controller couldn't receive any status on this job any more.

  Admission Checks:
  Last Transition Time: 2024-11-13T00:40:42Z
  Message: The workload needs to retry Prefetch Admission Check later
  Name: prefetch-ac
  State: Retry
  Conditions:
  Last Transition Time: 2024-11-13T00:40:42Z
  Message: The workload has failed admission checks
  Observed Generation: 1
  Reason: Pending
  Status: False
  Type: QuotaReserved
  Events:
  Type Reason Age From Message

  Normal Pending 22m kueue-admission couldn't assign flavors to pod set master: insufficient unused quota for cpu in flavor default-flavor, 200m more needed, insufficient unused quota for memory in flavor default-flavor, 176Mi more needed
  Normal Pending 22m (x4 over 22m) kueue-admission The workload has failed admission checks

In the controller, I set up requeue after 5 seconds if ACstatus is retry, but requeue didn't happen. Is this expected behavior on Retry? If not, how can we set the status back to ready when all conditins are met?

[mimowo]

AC controller couldn't receive any status on this job any more.

This is surprising that it is stuck in Retry after the recent fix which should set it back to Pending. What is the version you are using? 0.8.3+ and 0.9.0+ contain the fix.

[leipanhz]

I was using version to v0.8.2, which was released on 11/4. Now I updated the version again to v0.9.

There are some promising results: when it's quick to pass admission check, i.e., a few seconds. The retry works as expected 👍

When admission check takes longer such as 40 seconds, even I specify requeue it after 1 minute, that didn't work. It seems after certain time the ac will fail and ac reconciler won't receive any further notifications. Is there a retry time limit I can configure so Kueue will wait for much longer time for admission check to pass? Thanks

[mimowo]

Kueue should be waiting indefinitely for ACs in principle. For example ProvReq AC takes longer than 1min typically, so I suspect something else might still be going on. More details needed.

[leipanhz]

Quick summary:

@mimowo and I spent long hours to debug this issue. It turns out that the culprit is if a workload is evicted, setting admission check to Retry somehow triggers a bug. It causes this workload to be stuck and never able to reserve quota. As a result, the job is stuck in suspended mode forever.

While root cause is still to be investigated, a workaround is to put guard in the reconciler such as this:

kueue/pkg/controller/admissionchecks/provisioning/controller.go

Lines 122 to 124 in c655645

	if !workload.HasQuotaReservation(wl) || workload.IsFinished(wl) || workload.IsEvicted(wl) {
	return reconcile.Result{}, nil
	}

Thanks @mimowo for your knowledge and time on debugging this issue!