Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubernetes: add cluster autoscaler config #766

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dhij
Copy link

@dhij dhij commented Jan 7, 2025

This PR allows users to specify cluster autoscaler configuration in godo during kubernetes create and update. We accept a JSON string as the value for cluster autoscaler configuration.

@dhij dhij force-pushed the ihwang/add-ca-config branch from 7658f4c to 49b2e53 Compare January 7, 2025 16:40
SurgeUpgrade bool `json:"surge_upgrade,omitempty"`
RegistryEnabled bool `json:"registry_enabled,omitempty"`
ControlPlaneFirewall *KubernetesControlPlaneFirewall `json:"control_plane_firewall,omitempty"`
ClusterAutoscalerConfiguration string `json:"cluster_autoscaler_configuration,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is type string, but in the tests I see that is is a JSON object:

{"scale-down-utilization-threshold":"0.5","scale-down-unneeded-time":"1m30s"}

Is there any reason why doesn't use a more specific type? Generally, I'd expect something like:

type KubernetesAutoscalerConfiguration struct {
        ScaleDownUtilizationThreshold float64 `json:"scale_down_utilization_threshold"`
        ScaleDownUnneededTime         time.Duration `json:"scale_down_unneeded_time"`
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a JSON string and I actually had the same question on this but apparently this is what Timo had left on the ticket: Given we may want to support more parameters over time, it is probably advisable to accept a generic JSON object with CA flags/values as key/value pairs. That minimizes maintenance effort and also allows us to (de-)serialize the field more easily in the k8saas DB.

@andrewsomething andrewsomething self-assigned this Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants