fix: stop upscale while excluded when original replicas set

[jonathan-mayer]

Motivation

Currently if a namespace is excluded but a workload in it has the downscaler/original-replicas annotation the downscaler will scale up the workload. This changes this, so that excluding the namespace or workload will make the downscaler ignore it fully.

I will wait for a response from @samuel-esp to see what he thinks about this change.

Changes

Tests done

TODO

• I've assigned myself to this PR

[github-actions]

File	Coverage
All files	97%	✅
test_autoscale_resource.py	99%	✅
test_cmd.py	100%	✅
test_events.py	100%	✅
test_grace_period.py	100%	✅
test_ignore_if_labels_dont_match.py	100%	✅
test_main.py	100%	✅
test_pods_force_uptime.py	100%	✅
test_resources.py	100%	✅
test_scaler.py	95%	✅
test_time.py	100%	✅

Minimum allowed coverage is 70%

Generated by 🐒 cobertura-action against f5b313d

[samuel-esp]

Hi @jonathan-mayer, for sure an improvement! Didn't notice this behavior before, thank you for addressing! Can you try add/refactor test to reflect the new logic?

[JTaeuber]

LGTM

[samuel-esp]

One use case that could have been lost:

• User includes namespace A, B, C as a target
• User understands that namespace A is not needed anymore, so it deletes it from the target

In this case resources scaled down in namespace A won't be scaled up again. I'll try to test this scenario on Saturday. If this is confirmed I will address the problem in this way

• Introduce a new argument --do-not-wake-up-if-not-in-target
• Refactor the logic with a conditional, but I'll keep the default as it was to avoid compatibility problems

@jonathan-mayer would it be ok for you?

[jonathan-mayer]

Seems like a good idea. Although, I would change the name of the new cli argument to something like --upscale-excluded-namespaces.

[samuel-esp]

@jonathan-mayer even better because it's shorter name, thank you! We'll make this argument default inside the chart. If the user wants the new behavior, it should set a boolean variable to false inside values.yaml (if false, this argument won't be included inside the downscaler deployment)