feat(infra): Add rateLimitHpa support in EnvoyGateway API by keithfz · Pull Request #4983 · envoyproxy/gateway

keithfz · 2024-12-30T21:15:15Z

What type of PR is this?

Adds HPA support for rate limit deployment under the EnvoyGateway API

What this PR does / why we need it:
Allows autoscaling of the rate limit deployment similar to what is done today for the envoy proxy deployment.

Which issue(s) this PR fixes:

Fixes #4930

Release Notes: Yes

codecov · 2024-12-30T21:33:04Z

Codecov Report

Attention: Patch coverage is 80.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 66.77%. Comparing base (43621b4) to head (3a74a19).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...tructure/kubernetes/ratelimit/resource_provider.go	78.57%	6 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4983      +/-   ##
==========================================
+ Coverage   66.75%   66.77%   +0.02%     
==========================================
  Files         209      209              
  Lines       32058    32101      +43     
==========================================
+ Hits        21399    21435      +36     
- Misses       9381     9385       +4     
- Partials     1278     1281       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

arkodg · 2025-01-03T23:53:43Z

internal/infrastructure/kubernetes/ratelimit/resource_provider.go

imo we shouldnt do this, its bad practice to set replicas and hpa, but we shouldnt override a user setting, and instead recommend a user to leave replicas unset when using hpa

oh nvm youre following

gateway/internal/infrastructure/kubernetes/proxy/resource_provider.go

Line 328 in dd1c66f

if provider.GetEnvoyProxyKubeProvider().EnvoyHpa != nil {

i wonder why that is needed
cc @ardikabs

yeah, honestly I kind of agree, but I just went for consistency between this and the EnvoyProxy API. If we choose not to override a user setting here, we should probably have similar behavior in the EnvoyProxy HPA case as well.

yeah, lets wait for ardikabs' response, else keep it as is for now

It primarily prevents unwanted values from being set during HPA configuration, with the Envoy Gateway proactively ensuring such scenarios are avoided. This condition is already documented in the EG API Reference, highlighting when Replicas and HPA are being set.

gateway/api/v1alpha1/envoyproxy_types.go

Lines 312 to 316 in 10a31f1

// EnvoyHpa defines the Horizontal Pod Autoscaler settings for Envoy Proxy Deployment.

// Once the HPA is being set, Replicas field from EnvoyDeployment will be ignored.

//

// +optional

EnvoyHpa *KubernetesHorizontalPodAutoscalerSpec `json:"envoyHpa,omitempty"`

.

Historically, this behavior was implemented because EG previously set default replicas. As a result, even if users left the field empty (as recommended when using HPA), a default value of 1 was still applied.

But after this PR #2816, I think we can remove it, wdyt @arkodg?

ah thanks for adding clarity here @ardikabs, this extra step can be cleaned up in a follow up commit

internal/infrastructure/kubernetes/ratelimit/resource_provider.go

arkodg · 2025-01-04T00:11:46Z

thanks @keithfz, this PR looks great ! added some minor non blocking comments

arkodg

LGTM thanks !

zirain · 2025-01-06T20:39:25Z

api/v1alpha1/envoygateway_types.go

this' fine now, if we bring more fields in the future, pefer to something like:

ratelimit: deployment: ... hpa: ...

would we want to consider the same for other APIs in the future as well? e.g. EnvoyProxy:

gateway/api/v1alpha1/envoyproxy_types.go

Lines 291 to 328 in 7952936

type EnvoyProxyKubernetesProvider struct {

// EnvoyDeployment defines the desired state of the Envoy deployment resource.

// If unspecified, default settings for the managed Envoy deployment resource

// are applied.

//

// +optional

EnvoyDeployment *KubernetesDeploymentSpec `json:"envoyDeployment,omitempty"`

// EnvoyDaemonSet defines the desired state of the Envoy daemonset resource.

// Disabled by default, a deployment resource is used instead to provision the Envoy Proxy fleet

//

// +optional

EnvoyDaemonSet *KubernetesDaemonSetSpec `json:"envoyDaemonSet,omitempty"`

// EnvoyService defines the desired state of the Envoy service resource.

// If unspecified, default settings for the managed Envoy service resource

// are applied.

//

// +optional

EnvoyService *KubernetesServiceSpec `json:"envoyService,omitempty"`

// EnvoyHpa defines the Horizontal Pod Autoscaler settings for Envoy Proxy Deployment.

// Once the HPA is being set, Replicas field from EnvoyDeployment will be ignored.

//

// +optional

EnvoyHpa *KubernetesHorizontalPodAutoscalerSpec `json:"envoyHpa,omitempty"`

// UseListenerPortAsContainerPort disables the port shifting feature in the Envoy Proxy.

// When set to false (default value), if the service port is a privileged port (1-1023), add a constant to the value converting it into an ephemeral port.

// This allows the container to bind to the port without needing a CAP_NET_BIND_SERVICE capability.

//

// +optional

UseListenerPortAsContainerPort *bool `json:"useListenerPortAsContainerPort,omitempty"`

// EnvoyPDB allows to control the pod disruption budget of an Envoy Proxy.

// +optional

EnvoyPDB *KubernetesPodDisruptionBudgetSpec `json:"envoyPDB,omitempty"`

}

Both are making a good point, the extra hierarchy hides the component specific details, but the current style is envoyHpa which is probably why the same style was adopted for rateLimitDeployment

Signed-off-by: keithfz <[email protected]>

This is no longer needed after envoyproxy#2816 was merged Relates to envoyproxy#4983 (comment) Signed-off-by: Arko Dasgupta <[email protected]>

dont reset deployment replicas when hpa is set This is no longer needed after #2816 was merged Relates to #4983 (comment) Signed-off-by: Arko Dasgupta <[email protected]>

keithfz requested a review from a team as a code owner December 30, 2024 21:15

arkodg reviewed Jan 3, 2025

View reviewed changes

internal/infrastructure/kubernetes/ratelimit/resource_provider.go Outdated Show resolved Hide resolved

arkodg approved these changes Jan 6, 2025

View reviewed changes

arkodg requested review from a team January 6, 2025 18:31

zirain reviewed Jan 6, 2025

View reviewed changes

zirain approved these changes Jan 6, 2025

View reviewed changes

keithfz added 4 commits January 7, 2025 04:39

add hpa support for rate limit in EnvoyGateway API

02776a5

Signed-off-by: keithfz <[email protected]>

cleanup

35f8195

Signed-off-by: keithfz <[email protected]>

add testgen

008d9c3

Signed-off-by: keithfz <[email protected]>

lint

3a74a19

Signed-off-by: keithfz <[email protected]>

zirain force-pushed the ratelimit-hpa branch from 348353c to 3a74a19 Compare January 6, 2025 20:39

arkodg merged commit d4a0756 into envoyproxy:main Jan 6, 2025

arkodg mentioned this pull request Jan 7, 2025

dont reset replicas when hpa is set #5021

Merged

zirain pushed a commit to arkodg/gateway that referenced this pull request Jan 9, 2025

dont reset deployment replicas when hpa is set

dfb7231

This is no longer needed after envoyproxy#2816 was merged Relates to envoyproxy#4983 (comment) Signed-off-by: Arko Dasgupta <[email protected]>

arkodg added a commit that referenced this pull request Jan 9, 2025

dont reset replicas when hpa is set (#5021)

6466ceb

dont reset deployment replicas when hpa is set This is no longer needed after #2816 was merged Relates to #4983 (comment) Signed-off-by: Arko Dasgupta <[email protected]>

arkodg mentioned this pull request Oct 23, 2025

Support PDB for Ratelimit service #7314

Merged

cabrinha mentioned this pull request Feb 3, 2026

Helm chart CRDs not updated to support rateLimitHpa/rateLimitDeployment #8166

Open

	// EnvoyHpa defines the Horizontal Pod Autoscaler settings for Envoy Proxy Deployment.
	// Once the HPA is being set, Replicas field from EnvoyDeployment will be ignored.
	//
	// +optional
	EnvoyHpa *KubernetesHorizontalPodAutoscalerSpec `json:"envoyHpa,omitempty"`

	type EnvoyProxyKubernetesProvider struct {
	// EnvoyDeployment defines the desired state of the Envoy deployment resource.
	// If unspecified, default settings for the managed Envoy deployment resource
	// are applied.
	//
	// +optional
	EnvoyDeployment *KubernetesDeploymentSpec `json:"envoyDeployment,omitempty"`

	// EnvoyDaemonSet defines the desired state of the Envoy daemonset resource.
	// Disabled by default, a deployment resource is used instead to provision the Envoy Proxy fleet
	//
	// +optional
	EnvoyDaemonSet *KubernetesDaemonSetSpec `json:"envoyDaemonSet,omitempty"`

	// EnvoyService defines the desired state of the Envoy service resource.
	// If unspecified, default settings for the managed Envoy service resource
	// are applied.
	//
	// +optional
	EnvoyService *KubernetesServiceSpec `json:"envoyService,omitempty"`

	// EnvoyHpa defines the Horizontal Pod Autoscaler settings for Envoy Proxy Deployment.
	// Once the HPA is being set, Replicas field from EnvoyDeployment will be ignored.
	//
	// +optional
	EnvoyHpa *KubernetesHorizontalPodAutoscalerSpec `json:"envoyHpa,omitempty"`

	// UseListenerPortAsContainerPort disables the port shifting feature in the Envoy Proxy.
	// When set to false (default value), if the service port is a privileged port (1-1023), add a constant to the value converting it into an ephemeral port.
	// This allows the container to bind to the port without needing a CAP_NET_BIND_SERVICE capability.
	//
	// +optional
	UseListenerPortAsContainerPort *bool `json:"useListenerPortAsContainerPort,omitempty"`

	// EnvoyPDB allows to control the pod disruption budget of an Envoy Proxy.
	// +optional
	EnvoyPDB *KubernetesPodDisruptionBudgetSpec `json:"envoyPDB,omitempty"`
	}

Conversation

keithfz commented Dec 30, 2024

Uh oh!

codecov bot commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ardikabs Jan 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arkodg commented Jan 4, 2025

Uh oh!

arkodg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arkodg Jan 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Dec 30, 2024 •

edited

Loading

ardikabs Jan 4, 2025 •

edited

Loading

arkodg Jan 6, 2025 •

edited

Loading