[v2] Adding Standard Resource metrics to KEDA

[ckuduvalli]

Signed-off-by: Chaitanya Kuduvalli Ramachandra chaitanya.r@ETC02Y77ZVJG5H.local

Provide a description of what has been changed
KEDA currently supports only external metrics. We wish to support standard HPA metrics like CPU and memory usage through KEDA. This pr is to enable the same.

Checklist

• A PR is opened to update the documentation on https://github.com/kedacore/keda-docs
• Commits are signed with Developer Certificate of Origin (DCO)
• Tests have been added

Fixes #852

[ckuduvalli]

KEDA Docs pr:
kedacore/keda-docs#188

[tomkerkhove]

With this change, our new spec looks like this:

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: {scaled-object-name}
spec:
  scaleTargetRef:
    deploymentName: {deployment-name} # must be in the same namespace as the ScaledObject
    containerName: {container-name}  #Optional. Default: deployment.spec.template.spec.containers[0]
  pollingInterval: 30  # Optional. Default: 30 seconds
  cooldownPeriod:  300 # Optional. Default: 300 seconds
  minReplicaCount: 0   # Optional. Default: 0
  maxReplicaCount: 100 # Optional. Default: 100
  hpaConfig:           # Optional. If not set, KEDA won't scale based on resource utilization
    resourceMetrics:
      name: cpu/memory # Name of the resource to be targeted
      target:
        type: value/ utilization/ averagevalue
        value: 60 # Optional
        averageValue: 40 # Optional
        averageUtilization: 50 # Optional
    behavior:
  triggers:
  # {list of triggers to activate the deployment}

However, scaling on resource of the deployment feels like another trigger for me; not something we slap on the new scaled object spec or is this just me?

Reason why this feels odd to me is that ScaledObject relies on triggers, for example Service Bus queue, where we will almost never want to scale on CPU in the same scaled object but rather a new trigger in the list or another scaled object, no?

[ckuduvalli]

@tomkerkhove I had a discussion with @zroubalik before making this change and this is how we came up with a solution that we will support it as a standard resource metric than as a scaler. This is because HPA also has different types of metrics: Resource metrics and External metrics. Scalers support external metrics. So, we thought of using the resource metrics of HPA directly. Hence made this change. But looking forward to a discussion on this.

[tomkerkhove]

One of the strengths about KEDA is that people who are not a Kubernetes expert don't need to know how Kubernetes handles scaling.

I can see why we want to open up HPA configuration to those who have the expertise, but for scaling on resource metrics I would say that that's different and has to align with other scalers to be more consistent.

An example of a current one is this:

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: order-processor-scaler
  labels:
    app: order-processor
    deploymentName: order-processor
spec:
  scaleTargetRef:
    deploymentName: order-processor
  # minReplicaCount: 0 Change to define how many minimum replicas you want
  maxReplicaCount: 10
  triggers:
  - type: azure-servicebus
    metadata:
      queueName: orders
      queueLength: '5'
    authenticationRef:
      name: trigger-auth-service-bus-orders

But if I were to only scale on CPU then it would look like this:

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: order-processor-scaler
  labels:
    app: order-processor
    deploymentName: order-processor
spec:
  scaleTargetRef:
    deploymentName: order-processor
  # minReplicaCount: 0 Change to define how many minimum replicas you want
  maxReplicaCount: 10
  hpaConfig:           # Optional. If not set, KEDA won't scale based on resource utilization
    resourceMetrics:
      name: cpu # Name of the resource to be targeted
      target:
        type: averagevalue
        averageValue: 40 # Optional
  triggers:
  - type: azure-servicebus
    metadata:
      queueName: orders
      queueLength: '5'
    authenticationRef:
      name: trigger-auth-service-bus-orders

What if it looked like this:

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: order-processor-scaler
  labels:
    app: order-processor
    deploymentName: order-processor
spec:
  scaleTargetRef:
    deploymentName: order-processor
  # minReplicaCount: 0 Change to define how many minimum replicas you want
  maxReplicaCount: 10
  advanced:
    horizontalPodAutoscalerConfig:           # Optional. If not set, KEDA won't scale based on resource 
      behavior:
  triggers:
  - type: resource-metrics
    metadata:
      metric: cpu
      value: 40
    authenticationRef:
      name: trigger-auth-service-bus-orders

This allows people to:

1. Autoscale based on resource-metrics and can simply be based on HPA default approach, but users don't have to worry about that
2. People can still influence the behavior of the HPA, but indicates that this is more of an advanced scenario and remove the abbreviation (HPA) for folks who are new to Kubernetes scaling.

PS: I'm not saying you didn't do a good job but just want to have this aligned because it deviates from the rest and we dare to assume everybody is a Kubernetes expert which is far from true.

Thoughts from you folks as well on this @zroubalik @ahmelsayed @jeffhollan?

[zroubalik]

I have seen some requests for this functionality among users and I like this idea, eventhough it is a corner case and won't be (likely) used that much.
Currently there's no way how can user modify the HPA that is being generated by KEDA, therefore there's no way how to setup scaling both on external trigger and cpu/memory resources.
And since we have already added behavior, I don't se a problem in adding other optional fields if we add a proper documentation for these features. Users not so familiar with kubernetes could easily ignore this setting.

@tomkerkhove I am not saying that your proposed solution is bad, I can see your point behind putting the setting under the triggers sections, but from my POV the intention here is to provide a way for experienced users to modify the created HPA, not to provide KEDA scaling based on cpu/memory resources. So by putting this under HPA related config (next to behavior) we are trying to distiguish between scaling directly managed by KEDA (ie. triggers) and config (scaling) managed solely by HPA (CPU/memory) -> KEDA is not being involved in this kind of scaling at all.

This solution has small technical side-benefits, we don't have to care about validation of that
config as it is already covered by HPA and this is a direct part of HPA Spec -> no need for us to do any modification, if there are any changes in the original spec. And if there are other HPA related settings coming in the future, we can easily add them here and everything will be in one place.

If we introduce a separate scaler for this (as you have proposed), we will have to validate the fields, keep track of that Spec (I admit that it will likely not change in the future) and implement more things around scalers as this is a special kind of scaler which requires a special handling. These changes shouldn't be too complex and hard to implement, but I just wanted to bring this under the consideration as well.

BTW I'd prefer horizontalPodAutoscalerConfig over hpaConfig for the field name.

[ckuduvalli]

@zroubalik Have changed the field name to horizontalPodAutoscalerConfig

[tomkerkhove]

@zroubalik I can see what you're saying but that means you are forcing people to understand how Kubernetes uses HPA and they should know they need an HPA to scale based on CPU/Memory. Not everybody has this knowledge, far from actually.

For me this is not a good approach as KEDA should be the autoscaling layer folks have to know, all the rest is technical detail. It would be more inline with scaling other workloads.

If I have an API & queue worker then I'd need to have this:
Queue Worker:

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: order-processor-scaler
  labels:
    app: order-processor
    deploymentName: order-processor
spec:
  scaleTargetRef:
    deploymentName: order-processor
  # minReplicaCount: 0 Change to define how many minimum replicas you want
  maxReplicaCount: 10
  triggers:
  - type: azure-servicebus
    metadata:
      queueName: orders
      queueLength: '5'
    authenticationRef:
      name: trigger-auth-service-bus-orders

API:

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: order-api-scaler
  labels:
    app: order-api
    deploymentName: order-api
spec:
  scaleTargetRef:
    deploymentName: order-api
  # minReplicaCount: 0 Change to define how many minimum replicas you want
  maxReplicaCount: 10
  hpaConfig:           # Optional. If not set, KEDA won't scale based on resource utilization
    resourceMetrics:
      name: cpu # Name of the resource to be targeted
      target:
        type: averagevalue
        averageValue: 40 # Optional
  triggers:

This feels odd, no? One might say you can just use HPA directly but then you don't have a unified layer anymore.

[tomkerkhove]

For me; KEDA is really about making autoscaling deadsimple and use same approach, regardless of the source.

[zroubalik]

@tomkerkhove yeah, you have got some points. Let's discuss this on the call next week.

[zroubalik]

@ahmelsayed what do you think about this?

[ahmelsayed]

From my prospective, one of the main reasons for using HPA for 1->N scaling has always been being able to make use of all the default resources HPA supports for scaling there, along side the custom metrics provided by KEDA. Providing the right interface for that however was always a bit of an unknown. I didn't know what's the best way to allow that that will be both flexible enough to be useful, but doesn't leak a lot of underlying concepts and complicated the ScaledObject interface.

I think there is value in having a generic way for users to patch the HPA generated by KEDA if needed (even if just as an advanced scenario), while still retaining the option to add knobs on the ScaledObject later on for that as well.

When I thought about it before, I was thinking of a way to allow users to provide a merge patch for HPA per ScaledObject that KEDA could apply on the generated HPA as a final step as this would allow you to see what the object looks like, then define any transform/patch for it that you like.

In that case the example Tom shared above

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: order-processor-scaler
spec:
  scaleTargetRef:
    deploymentName: order-processor
  triggers:
  - type: azure-servicebus
    metadata:
      queueName: orders
      queueLength: '5'
    authenticationRef:
      name: trigger-auth-service-bus-orders
+  patches:
+    hpa: {}

or have it defined as a patch in another object with the same name, something like

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledPatch
metadata:
  name: order-processor-scaler
spec:
  hpaPatch: {}

However, it seemed a bit of an overkill at the time and I wasn't sure if we might find a better way in the future, so I punted on it.

The interface proposed by this PR seems to more or less match what I was thinking about above (albeit it only takes *autoscalingv2beta2.ResourceMetricSource and I think it should probably take a []*autoscalingv2beta2.ResourceMetricSource instead).

I understand @tomkerkhove concern about additional complexity, but I think if the option listed under an advanced section as a patch to the generated HPA and not something that's always in your face when learning about KEDA would meet that requirement half-way.

[wesyao]

Hey, I'm glad I found this PR because this is exactly what I was trying to do. Hopefully there is a consensus soon. Is there a roadmap for when 2.0.0 will be stable enough for release?

[tomkerkhove]

We're actually trying to fix 2 things as far as I see:

• Give people control to manipulate the HPA created by KEDA
• Provide KEDA users to scale based on standard metrics.

I'm fully ok with opening up the HPA related changes as @ahmelsayed suggested or the current proposal but I would highly recommend to move the CPU/memory scaler to triggers since that's aligned with another approach.

Let's discuss during the standup tomorrow maybe?

[zroubalik]

Let's discuss during the standup tomorrow maybe?

+1

[zroubalik]

@ckuduvalli we disscussed this on the call and for now we will go with direction proposed in this PR. We can add a separate resource based scalers later, if there's a need from community.

Could you please update you PR:

• to put the whole config under advanced as proposed by @tomkerkhove here: [v2] Adding Standard Resource metrics to KEDA #874 (comment)
• to use []*autoscalingv2beta2.ResourceMetricSource as proposed by @ahmelsayed here [v2] Adding Standard Resource metrics to KEDA #874 (comment)

@tomkerkhove @ahmelsayed am I correct? 😄

[tomkerkhove]

Certainly!

This is what we've discussed during standup yesterday:

• Standard Resource metrics is more of an advanced scenario where people want to manipulate the HPA that is being created
  • Tom has the feeling that we should align it with other triggers to provide a consistent approach for users and carve out the mutation of the HPA
  • This brings a lot of additional complexity so it's not sure if it's worth it
  • We will start with the advanced scenario and see if people start asking for a CPU/Memory trigger

[ckuduvalli]

sorry I couldn't attend the standup yesterday. @zroubalik @tomkerkhove will update the pr as per the comments

[tomkerkhove]

No worries!

[ckuduvalli]

@tomkerkhove @ahmelsayed @zroubalik Have updated this pr as well as the docs pr. Please check
Docs pr: kedacore/keda-docs#188

[zroubalik]

LGTM

[zroubalik]

@ahmelsayed ok to merge?

[ahmelsayed]

Advanced and HorizontalPodAutoscalerConfig could be nil, right? should the condition be updated to && scaledObject.Spec.Advanced != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig != nil?

Also in other places

[ckuduvalli]

valid point. Have made the change

[ahmelsayed]

minor nit: I think it's slightly better to preallocate the slice to the correct size since that's known in this case.

Suggested change

	var metrics []autoscalingv2beta2.MetricSpec
	metrics := make ([]autoscalingv2beta2.MetricSpec, 0, len(resourceMetrics))

[ckuduvalli]

done

[ahmelsayed]

LGTM