kedacore / keda

KEDA is a Kubernetes-based Event Driven Autoscaling component. It provides event driven scale for any container running in Kubernetes
https://keda.sh
Apache License 2.0
8.27k stars 1.05k forks source link

ScaledJob Paused Condition doesn't change to False when removing annotation #5251

Closed nappelson closed 1 month ago

nappelson commented 9 months ago

Report

When I set the ScaledJob paused annotation to True

autoscaling.keda.sh/paused: "true"
```, confirm that the job is paused, and then remove that annotation, the Paused condition from `kubectl get ScaledJob` still shows True.

Interestingly enough, the keda operator begins to scale the Job so it doesn't recognize this condition. 

### Expected Behavior

`kubectl get ScaledJob` should show `Paused = False`

### Actual Behavior

`kubectl get ScaledJob` shows `Paused = True` even though the operator is in fact scaling the job.

### Steps to Reproduce the Problem

1. create a ScaledJob with `autoscaling.keda.sh/paused: "true"`
2. confirm ScaledJob is created with `Paused: True` by calling `kubectl get ScaledJob` 
3. remove annotation and apply changes
4. verify that `kubectl get ScaledJob` still shows `Paused: False` even though the job is scaling

### Logs from KEDA operator

The keda operator does in fact show 

INFO ScaledJob is paused, stopping scaling loop.


after removing the annotation even though the ScaledJob resource condition shows that it is still paused.

### KEDA Version

2.12.0

### Kubernetes Version

1.27

### Platform

Google Cloud

### Scaler Details

_No response_

### Anything else?

I found this bug while testing code for https://github.com/kedacore/keda/issues/5215
nappelson commented 9 months ago

I think I have found the bug...

https://github.com/kedacore/keda/blob/54b05f3fb7ab6aed4aea5f6765eca4c465465619/controllers/keda/scaledjob_controller.go#L209

Essentially, requestScaleLoop is being called with the old scaledJob object. The conditions haven't been updated yet here - https://github.com/kedacore/keda/blob/54b05f3fb7ab6aed4aea5f6765eca4c465465619/controllers/keda/scaledjob_controller.go#L132

requestScaleLoop needs to be called after we call setStatusConditions.

I'm happy to fix this in https://github.com/kedacore/keda/issues/5215 or have two separate PRs. What would the maintainers prefer?

zroubalik commented 9 months ago

@nappelson thanks! Yes, please use 2 different PRs

nappelson commented 8 months ago

I think I have found the bug...

https://github.com/kedacore/keda/blob/54b05f3fb7ab6aed4aea5f6765eca4c465465619/controllers/keda/scaledjob_controller.go#L209

Essentially, requestScaleLoop is being called with the old scaledJob object. The conditions haven't been updated yet here -

https://github.com/kedacore/keda/blob/54b05f3fb7ab6aed4aea5f6765eca4c465465619/controllers/keda/scaledjob_controller.go#L132

requestScaleLoop needs to be called after we call setStatusConditions.

I'm happy to fix this in #5215 or have two separate PRs. What would the maintainers prefer?

Actually, this doesn't appear to be "the bug". The reason why is because ScaledObjects call requestScaleLoop before SetStatusConditions as well...

I'm wondering if this is due to how the respective scale_executors are implemented for ScaledObjects and ScaledJobs and if there is a bug in the executor for jobs.

zroubalik commented 8 months ago

Yeah, looking briefly at the code. We should probalby store the paused condition right away, it is a special case most likely. For the other conditions it makes sense to way for the result of requestScaleLoop() or another functions/validations.

Or do you see an easy way how we can handle this in the executor? I haven't seen the code for some time, so don't remember anything 😄 Will try to dig in more later on.

stale[bot] commented 6 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 6 months ago

This issue has been automatically closed due to inactivity.

stale[bot] commented 6 months ago

This issue has been automatically closed due to inactivity.

stale[bot] commented 4 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 months ago

This issue has been automatically closed due to inactivity.

stale[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 1 month ago

This issue has been automatically closed due to inactivity.