mercari / tortoise

Tortoise: Shell-Shockingly-Good Kubernetes Autoscaling
MIT License
317 stars 14 forks source link

Make the annotation naming consistent #322

Closed sanposhiho closed 4 months ago

sanposhiho commented 5 months ago

Problem

This issue is raised at https://github.com/mercari/tortoise/pull/308.

https://github.com/mercari/tortoise/blob/main/pkg/annotation/annotation.go

We have annotations starting from tortoises.autoscaling.mercari.com/ and from tortoise.autoscaling.mercari.com/. We should unify it with tortoise.autoscaling.mercari.com/* ideally.

Proposal

So, basically what we have to do is rename "tortoises.autoscaling.mercari.com/tortoise-name" to "tortoise.autoscaling.mercari.com/tortoise-name".

But, it cannot be done simply because if we just rename the annotation, it'd break existing tortoises working in the cluster.

We have to migrate the annotation gradually.

First step

Second step

It should be done enough time passed after the first step.

sadath-12 commented 5 months ago

Hi @sanposhiho as this is not production ready can we just change it ?

sadath-12 commented 5 months ago

/assign

sanposhiho commented 5 months ago

No. It's mentioned as "not production ready" for now though, actually in order to evaluate tortoise's recommendation logic in real world, our company already started to use it in the dev cluster widely and even some workloads in prod. Also, either way, regardless of our company, given it's open-source, we should try not to bring any breaking changes as much as possible.

So, I do believe we have to follow the gradual migration of the annotation, like the way I proposed at the top.

sadath-12 commented 5 months ago

thanks for clearing . will work on it 😊

sanposhiho commented 4 months ago

https://github.com/mercari/tortoise/pull/359 removed the tortoise name annotation itself. Closing this.