Support ScaledObject taking over existing HPAs with the same name while they are not managed by other ScaledObject

[pragmaticivan]

Provide a description of what has been changed

Checklist

• When introducing a new scaler, I agree with the scaling governance policy
• I have verified that my change is according to the deprecations & breaking changes policy
• Tests have been added
• Changelog has been updated and is aligned with our changelog requirements
• A PR is opened to update the documentation on (repo) (if applicable)
• Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #4457

[tomkerkhove]

@pragmaticivan Are you still working on this or waiting for us to review?

[pragmaticivan]

@pragmaticivan Are you still working on this or waiting for us to review?

Hi, sorry still working just used the draft so I can figure out how CI is working on this PR

[tomkerkhove]

No worries!

[zroubalik]

Folks, I am still not 100% sure, we should go into this direction - ie. changing ownership of existing resources. I might be wrong of course 🤷‍♂️

[pragmaticivan]

@zroubalik All good, no attachment to the PR. Maybe we can figure out another creative way to migrate from traditional HPA to ScaledObject progressively.

[zroubalik]

@zroubalik All good, no attachment to the PR. Maybe we can figure out another creative way to migrate from traditional HPA to ScaledObject progressively.

Yeah definitely, one of my concerns is, what would happen if the HPA is managed by some other tool? We shouldn't change the ownership, as it could have consequencies.

[pragmaticivan]

@zroubalik I get it, but at this point, it becomes intentional. It requires the hardcoded HPA name + a flag requesting that to be done. If the person pushing that code reaches prod, its likely that they already have the permissions to do that manually and are just trying to migrate an HPA to Keda.

Right now, the option is to delete the existing HPA or disable the hook (which is way more dangerous in a multi-tenant cluster). The deletion causes a little bit of downtime because, for some reason, the default HPA behavior for removal is to go down an up to what the workload has set for the number of replicas (which is also dangerous).

[zroubalik]

@zroubalik I get it, but at this point, it becomes intentional. It requires the hardcoded HPA name + a flag requesting that to be done. If the person pushing that code reaches prod, its likely that they already have the permissions to do that manually and are just trying to migrate an HPA to Keda.

Right now, the option is to delete the existing HPA or disable the hook (which is way more dangerous in a multi-tenant cluster). The deletion causes a little bit of downtime because, for some reason, the default HPA behavior for removal is to go down an up to what the workload has set for the number of replicas (which is also dangerous).

Yeah, these are valid points. I just want to be careful, we might be able to mitigate my concerns.

[tomkerkhove]

@zroubalik We discussed this in our previous standup; maybe the notes help get an idea as well - https://docs.google.com/document/d/1zdwD6j86GxcCe5S5ay9suCO77WPrEDnKSfuaI24EwM4/edit?usp=sharing

[ghost]

Hi, any progress on this PR?
Actually we need this feature so that our users will have Zero downtime during migration!

[pragmaticivan]

@bingikarthik I'm getting back to wrapping that up this week, although the code as it is should probably be enough to make the feature work as expected. Just learning my way around the codebase to make sure that's the case but I was able to add some tests.

[ghost]

Thanks @pragmaticivan for the update!

[rwkarg]

From a user perspective this seems relatively safe and un-surprising. If I ignore the new feature (don't add the annotation) then I get the existing behavior. In order to have it do something new, I would be required to explicitly add the annotation and specify the same name as an existing HPA.

One thing I'm not clear on from the PR is where I would specify the HPA name I'm looking to take over (I'm certain this will be covered in the forthcoming documentation).

[pragmaticivan]

metadata:
  annotations:
    keda.sh/transfer-hpa-ownership: "true"
spec:
   advanced:
      horizontalPodAutoscalerConfig:
      name: {name-of-hpa-resource}

@rwkarg yes, doc coming up once I gain confidence that the current code is ok.

For instance, it might be the case the label should be autoscaling.keda.sh/transfer-hpa-ownership instead.

[JorTurFer]

This looking really well :)
I have left some comments inline

[JorTurFer]

/run-e2e scaled_object_validation
Update: You can check the progress here

[zroubalik]

We are about to release a new version in a few hours. Would you like to include this into the release? If so, we should hurry up :)

@pragmaticivan ^^

[zroubalik]

LGTM, thanks!

[zroubalik]

/run-e2e scaled_object_validation
Update: You can check the progress here

[pragmaticivan]

PR for the doc: kedacore/keda-docs#1166