syself / cluster-api-provider-hetzner

Cluster API Provider Hetzner 🚀 Kubernetes Infrastructure as Software 🔧 Terraform/Kubespray/kOps alternative for running Kubernetes on Hetzner
https://caph.syself.com
Apache License 2.0
539 stars 51 forks source link

:seedling: Add new CX server types #1330

Closed apricote closed 3 weeks ago

apricote commented 4 weeks ago

What this PR does / why we need it:

Adds the new server types cx22, cx32, cx42 and cx52 to the CRD validation. (There is no cx12). These were just launched and you can find out more at https://www.hetzner.com/news/new-cx-plans/

The previous generation (cx11 - cx51) was deprecated and will be removed on 06 September 2024. I saw that you reference them in some templates and you might want to replace them until then.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

janiskemper commented 3 weeks ago

@apricote general question: What about the fact that certain server types are only available in some locations? Do you think it is feasible that we make an actual validation so that people can be 100% sure that the type exists? For example with the US regions?

apricote commented 3 weeks ago

What about the fact that certain server types are only available in some locations? Do you think it is feasible that we make an actual validation so that people can be 100% sure that the type exists? For example with the US regions?

Thats certainly a possibility to do in the validating webhook. This makes it dependent on the Cloud API, not sure if you want that, though a 1 hour cache or similar would be good enough.

You would also need some kind of API Token to get the list of server types. As far as I can see, there is no direct link between the HcloudMachineTemplate & the tokens. There is also no guarantee, that every user/project has the same server types available.

janiskemper commented 3 weeks ago

I actually thought about hardcoding it in CAPH :sweat_smile: So kinda having what we already have in little more complex with taking into account the different regions.

What do you mean by this?

There is also no guarantee, that every user/project has the same server types

I'm just looking for a solution that makes it the most reliable that users don't specify server types that are not available

apricote commented 2 weeks ago

There is also no guarantee, that every user/project has the same server types

There might be users with pre-view access to new server types, or custom server types for their usecases. I think this is mostly used internally, for example before the CAX servers launched the employees already had access to develop & test for them.

This is already an issue with the current hardcoded enum in the CRD, so whatever you are going to change its not going to get worse :D

Hardcoding it somewhere is probably the easiest. If its in the CRD, editors/IDEs can get the json schema and provide the user with autocomplete, not sure if any editor works well with this.

janiskemper commented 2 weeks ago

So what would be your ideal solution? Not validating? If validating then only with an API call to fetch the "real" and up-to-date list? Having a more complex logic per location? Not validation but some good documentation so that users don't do mistakes? Currently, I'm not even sure if it is visible in a good way if a type that is not available is specified and the server cannot create it. Need to check