search

knative / operator

Combined operator for Knative.
Apache License 2.0
187 stars 99 forks source link

Unable to disable workloads with HPAs #1688

Closed treyhyde closed 1 month ago

treyhyde commented 8 months ago

Describe the bug

You can disable unneeded / optional workloads such as mt-broker-controller by settings replicas to 0. This trick DOES NOT work for workloads (anymore) that have an HPA. Setting replicas to 0 here gets reset by various bits of logic and guards in func hpaTransform.. The HPA is created and the 0 value is discarded.

Able to disable: mt-broker-controller Not able to disable mt-broker-ingress mt-broker-filter

These workloads are not needed if you are using an alternative broker implementation such as the Kafka Broker.

Expected behavior

HPA is not created and replicas set to zero on the deployments

To Reproduce Create an entry in workloads such as

 - name: mt-broker-filter
    replicas: 0 # not used

Knative release version Last several major versions up to an including 1.13.0

Additional context I'm pretty sure we have been able to disable these workloads for a while but this has been broken for a bit.

houshengbo commented 6 months ago

@treyhyde What about 1.12.x? Before u upgraded to 1.13, does it exist?

treyhyde commented 6 months ago

@treyhyde What about 1.12.x? Before u upgraded to 1.13, does it exist?

The problem has existed for a couple of major versions. I'm pretty sure that this wasn't a problem at some point but I've been unable to ditch these workloads for a bit.

hamishforbes commented 6 months ago

Same issue here. I'd go further and request it be possible to disable the HPAs entirely.

We use KEDA and Prometheus metrics for autoscaling and do not have the metrics-server component installed in our clusters so the HPAs just sit there unable to fetch metrics anyway.

houshengbo commented 5 months ago

It is this line https://github.com/knative/operator/blob/main/pkg/reconciler/common/hpa.go#L64 causing this behavior. I looks like the comment was put there intentionally, so I guess this behavior was expected. However, I am reaching out the folks regarding the initial reasons.

ReToCode commented 5 months ago

I assume it is this way to make sure you never have less replicas than what we tested with (and what we set in replicas in the yaml). How about setting replicas to 0 and also minReplicas of the HPA to 0. IMHO that should do what you ask for?

houshengbo commented 5 months ago

I would work for me if we set both the number of replicas and the minReplicas of HPA to 0. I am trying to find out the initial reason we did not allow setting number of the replicas less than the minReplicas of the HPA. I hope we did not step over other issues in that regard.

github-actions[bot] commented 2 months ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

treyhyde commented 1 day ago

/reopen still valuable to disable idle workloads from a load / security perspective.