Provide support for stateful sets

[sc7565]

Wanted to know, if current setup also supports the statefulsets?

Use-Case

Autoscale - statefulsets

[iyacontrol]

@sc7565 , at now it not support statefulset.

@tomkerkhove Is it necessary to support sts? need discussion.

[iyacontrol]

pr is #498.
I think we should discuss in detail whether statefullset is supported, then go to review the code.

hi @ahmelsayed . what is your idea ?

[zroubalik]

It would be great to support StatefulSet, but I think it should be a part of a bigger discussion on supporting mulitple types and whether we should use one general CRD or multiple CRDs for particular types or another approach.

Solution in PR #498 is adding another complexity to the existing CRD.

[iyacontrol]

hi @zroubalik

At first, the kubernetes native hpa support deployment and statefulset. so i think keda should support statefulset.

At last, I tend to choose general CRD. There is no particularly big difference in the logic of the code for deployment and statefulset. PR #498 , to compatible with previous versions. if you use keda for deployment， yaml is the same as before.

[zroubalik]

@iyacontrol absolutely agree on supporting StatefulSet!

Don't take my comment wrong. To help understand me, we had the similar discussion with supporting Job before and I wanted to restart the discussion on this since we are gonna add another type :)

My concern is, that currently there are many combinations of properties in the yaml for different types which might be confusing for user. I'd prefer to have simple CRD for a particular type.

[iyacontrol]

hi @zroubalik , I read your reply carefully.

you are right. many combinations of properties in the yaml might be confusing for user. so
I think the key point is how to use ScaledObjectScaleType filed. if user supply ScaledObjectScaleType filed , deploymentName or statefulSetName can merge into one filed， for example name or appName and so on. Of course these are just some of my personal understandings.

Nice to discuss with you!

[tomkerkhove]

Frankly, I'm with @zroubalik on this and brought this up when we added jobs as well. While the code might not be different, from a consumer perspective they are different and so should the resources that we create.

Adding @jeffhollan @ahmelsayed @anirudhgarg to let them chime in on this as well as I know they're more on the ScaledObject side of the fence.

That said - Either way are fine for me, splitting just feels better from a consumer/ops standpoint.

[iyacontrol]

Hope more people discuss.

I think we can get some inspiration from kubernetes native hpa, why is there only one type of kubernetes native hpa? Instead of statefulset hpa and deployment hpa.

[tomkerkhove]

While I get your point, I think from a consumer perspective it's a lot easier if it just works with the things you need and you don't need to know when to use what.

But again, see your point - Just have mixed feelings with it

[sc7565]

do we think we can get traction on this, we feel statefulset scaling is very beneficial and we do have lot of use cases for same

[jeffhollan]

@sc7565 I don't see a reason we wouldn't want to support this. I think there is fair discussion on if the implementation would vary enough to justify splitting out the CRDs - but fair points on both sides. It looks like hpa already works for stateful sets and assuming custom metrics and stuff works the same, so I don't imagine it would be a ton of work to add this (I'm a PM tho so who knows 😅). A lot of considerations on statefulsets but imagine KEDA should provide the capability, we document the considerations clearly, and let folks use it if it makes sense. That's my 2c. May be worth opening another issue or renaming this one to "add support for stateful sets" - providing a bit of an expected experience (should the scaledObject and hpa be the same as deployments, just that it now scales a StatefulSet?) and we could flag as help-wanted

[balchua]

Sorry, chiming in a little bit late here.
I like the idea of supporting sts. I also agree that the CRD can get confusing.

Take for example the Job type.
There are attributes (or fields) which are not relevant, for example minReplicas and maxReplicas for Job. Another field that we repurpose is the queueLength. It would have been easier to understand if it is named maxJobs.

In reality we have two types of workloads, those that are ephemeral in nature (i.e. they run to completion) like Job and those which are "always on" for as long as there is constant flow of messages/events.

Deployments and Statefulsets belong to that category.

[tomkerkhove]

Fair point. But if we do split the CRDs, wouldn't it be best to have ScaledJob, ScaledDeployment & ScaledStatefulSet to make it super explicit & clear rather than ScaledJob & ScaledDeamon (or so)?

[balchua]

Naming is hard 😁

[iyacontrol]

the metaobject of deployment is

kind: Deployment
apiVersion: apps/v1

the metaobject of statefulset is

apiVersion: apps/v1
kind: StatefulSet

maybe ScaleJob and ScaleApp consistent with k8s？

[tomkerkhove]

Is a Deployment an App? I'd say no, that's what https://oam.dev/ covers.

Frankly; I think we should align with OAM's Core Workload Types or if we split it, why not per type?

[balchua]

+1 on oam naming convention.
Perhaps like this?
ScaledTask = Job and ScaledWorker = always on.

[gbird3]

Would love for this to get finalized soon. Tested out keda using deployments and it is working great but our main use case requires Statefulset support.

[zroubalik]

Fixed by #731