[Misc] Add autoscaling validation of minReplicas and maxReplicas

[jiangxiaobin96]

Pull Request Description

Add validation of minReplicas and maxReplicas in HPA autoscaling

[gemini-code-assist]

Summary of Changes

Hello @jiangxiaobin96, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces enhanced validation and default handling for Horizontal Pod Autoscaler (HPA) configurations. It ensures that the maxReplicas value is always greater than or equal to minReplicas and provides a default minReplicas of 1 if not explicitly set or set to an invalid value. Additionally, it adds a check to ensure that a metric source is always provided for autoscaling.

Highlights

• HPA Replica Validation: Implemented validation within the makeHPA function to prevent the creation of Horizontal Pod Autoscalers where maxReplicas is less than minReplicas, returning an error in such cases.
• Default minReplicas Handling: Modified the makeHPA function to automatically set minReplicas to 1 if it is nil or less than or equal to 0, ensuring a valid minimum replica count.
• Metric Source Presence Check: Added a new check in GetPaMetricSources to explicitly return an error if no MetricsSource is defined, preventing misconfigurations.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request adds important validation for minReplicas and maxReplicas in the HPA strategy, preventing invalid configurations. The changes are well-implemented. I've added a couple of suggestions for further improvement: one to refactor a function for better clarity and error reporting, and another to consider using a validating admission webhook for a better user experience by providing immediate feedback on invalid configurations.

[gemini-code-assist]

The logic here is correct, but it could be simplified for better readability and to provide a more informative error message. Using a switch statement makes the different cases very clear and is more idiomatic in Go for handling multiple conditions on a single variable.

	switch len(pa.Spec.MetricsSources) {
	case 0:
		return MetricSource{}, fmt.Errorf("cannot find any MetricsSource")
	case 1:
		return pa.Spec.MetricsSources[0], nil
	default:
		return MetricSource{}, fmt.Errorf("for now we only support one MetricsSource, but found %d", len(pa.Spec.MetricsSources))
	}

[gemini-code-assist]

While validating this in the controller is a good step, it provides delayed feedback to the user. The reconciliation will fail, and the user will have to check the status or events of the PodAutoscaler resource to understand the problem.

A better user experience would be to implement a validating admission webhook for the PodAutoscaler CRD. This would reject invalid configurations immediately upon creation or update (kubectl apply), providing instant feedback to the user at the client-side.

[googs1025]

I'm a bit curious that MinReplicas seems to be optional rather than required in HPA.

[jiangxiaobin96]

I get it! That's why hpa.Spec.MinReplicas = minReplicas is set after hpa := &autoscalingv2.HorizontalPodAutoscaler

[googs1025]

I think this check is sufficient at this stage. If there are more complex validations or other validations in the future, we can use validate webhook to achieve

[jiangxiaobin96]

cc @Jeffwan