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.

Sign up for GitHub

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

fix: omit default replicas on Kubernetes Deployment #2816

Merged
merged 3 commits into from
Mar 8, 2024
Merged

fix: omit default replicas on Kubernetes Deployment #2816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7f6d00
fix: remove default replicas function
ardikabs Mar 7, 2024
545eb65
chore: omit replicas because nil equal to 1 by default
ardikabs Mar 7, 2024
36cb529
chore: add a note when a user is being explicit on deployment replicas
ardikabs Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions api/v1alpha1/kubernetes_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ import (
"k8s.io/utils/ptr"
)

// DefaultKubernetesDeploymentReplicas returns the default replica settings.
func DefaultKubernetesDeploymentReplicas() *int32 {
repl := int32(DefaultDeploymentReplicas)
return &repl
}

// DefaultKubernetesDeploymentStrategy returns the default deployment strategy settings.
func DefaultKubernetesDeploymentStrategy() *appv1.DeploymentStrategy {
return &appv1.DeploymentStrategy{
Expand All @@ -38,7 +32,6 @@ func DefaultKubernetesContainerImage(image string) *string {
// DefaultKubernetesDeployment returns a new KubernetesDeploymentSpec with default settings.
func DefaultKubernetesDeployment(image string) *KubernetesDeploymentSpec {
return &KubernetesDeploymentSpec{
Replicas: DefaultKubernetesDeploymentReplicas(),
Strategy: DefaultKubernetesDeploymentStrategy(),
Pod: DefaultKubernetesPod(),
Container: DefaultKubernetesContainer(image),
Expand Down Expand Up @@ -96,10 +89,6 @@ func GetKubernetesServiceExternalTrafficPolicy(serviceExternalTrafficPolicy Serv

// defaultKubernetesDeploymentSpec fill a default KubernetesDeploymentSpec if unspecified.
func (deployment *KubernetesDeploymentSpec) defaultKubernetesDeploymentSpec(image string) {
if deployment.Replicas == nil {
deployment.Replicas = DefaultKubernetesDeploymentReplicas()
}

if deployment.Strategy == nil {
deployment.Strategy = DefaultKubernetesDeploymentStrategy()
}
Expand All @@ -121,6 +110,7 @@ func (deployment *KubernetesDeploymentSpec) defaultKubernetesDeploymentSpec(imag
}
}

// setDefault fill a default HorizontalPodAutoscalerSpec if unspecified
func (hpa *KubernetesHorizontalPodAutoscalerSpec) setDefault() {
if len(hpa.Metrics) == 0 {
hpa.Metrics = DefaultEnvoyProxyHpaMetrics()
Expand Down
2 changes: 2 additions & 0 deletions api/v1alpha1/shared_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,8 @@ const (
)

// KubernetesHorizontalPodAutoscalerSpec defines Kubernetes Horizontal Pod Autoscaler settings of Envoy Proxy Deployment.
// When HPA is enabled, it is recommended that the value in `KubernetesDeploymentSpec.replicas` be removed, otherwise
Copy link
Contributor Author

Choose a reason for hiding this comment

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

check here @Xunzhuo

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I mean if eg should take care the priority in code logics. cc @arkodg @zirain

Copy link
Member

Choose a reason for hiding this comment

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

+1, HPA will override replicas

Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer if we kept it simple for now, so our API matches K8s behavior

// Envoy Gateway will revert back to this value every time reconciliation occurs.
// See k8s.io.autoscaling.v2.HorizontalPodAutoScalerSpec.
//
// +kubebuilder:validation:XValidation:message="maxReplicas cannot be less than minReplicas",rule="!has(self.minReplicas) || self.maxReplicas >= self.minReplicas"
Expand Down
5 changes: 1 addition & 4 deletions api/v1alpha1/validation/envoygateway_validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,8 +668,7 @@ func TestEnvoyGatewayProvider(t *testing.T) {

envoyGatewayProvider.Kubernetes = &v1alpha1.EnvoyGatewayKubernetesProvider{
RateLimitDeployment: &v1alpha1.KubernetesDeploymentSpec{
Replicas: nil,
Pod: nil,
Pod: nil,
Container: &v1alpha1.KubernetesContainerSpec{
Resources: nil,
SecurityContext: nil,
Expand All @@ -684,8 +683,6 @@ func TestEnvoyGatewayProvider(t *testing.T) {

assert.NotNil(t, envoyGatewayProvider.Kubernetes.RateLimitDeployment)
assert.Equal(t, envoyGatewayProvider.Kubernetes.RateLimitDeployment, v1alpha1.DefaultKubernetesDeployment(v1alpha1.DefaultRateLimitImage))
assert.NotNil(t, envoyGatewayProvider.Kubernetes.RateLimitDeployment.Replicas)
assert.Equal(t, envoyGatewayProvider.Kubernetes.RateLimitDeployment.Replicas, v1alpha1.DefaultKubernetesDeploymentReplicas())
assert.NotNil(t, envoyGatewayProvider.Kubernetes.RateLimitDeployment.Pod)
assert.Equal(t, envoyGatewayProvider.Kubernetes.RateLimitDeployment.Pod, v1alpha1.DefaultKubernetesPod())
assert.NotNil(t, envoyGatewayProvider.Kubernetes.RateLimitDeployment.Container)
Expand Down
2 changes: 0 additions & 2 deletions api/v1alpha1/validation/envoyproxy_validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,8 +599,6 @@ func TestEnvoyProxyProvider(t *testing.T) {

assert.NotNil(t, envoyProxyProvider.Kubernetes.EnvoyDeployment)
assert.Equal(t, envoyProxyProvider.Kubernetes.EnvoyDeployment, egv1a1.DefaultKubernetesDeployment(egv1a1.DefaultEnvoyProxyImage))
assert.NotNil(t, envoyProxyProvider.Kubernetes.EnvoyDeployment.Replicas)
assert.Equal(t, envoyProxyProvider.Kubernetes.EnvoyDeployment.Replicas, egv1a1.DefaultKubernetesDeploymentReplicas())
assert.NotNil(t, envoyProxyProvider.Kubernetes.EnvoyDeployment.Pod)
assert.Equal(t, envoyProxyProvider.Kubernetes.EnvoyDeployment.Pod, egv1a1.DefaultKubernetesPod())
assert.NotNil(t, envoyProxyProvider.Kubernetes.EnvoyDeployment.Container)
Expand Down
1 change: 0 additions & 1 deletion internal/envoygateway/config/decoder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ func TestDecode(t *testing.T) {
Type: v1alpha1.ProviderTypeKubernetes,
Kubernetes: &v1alpha1.EnvoyGatewayKubernetesProvider{
RateLimitDeployment: &v1alpha1.KubernetesDeploymentSpec{
Replicas: v1alpha1.DefaultKubernetesDeploymentReplicas(),
Strategy: v1alpha1.DefaultKubernetesDeploymentStrategy(),
Container: &v1alpha1.KubernetesContainerSpec{
Env: []corev1.EnvVar{
Expand Down
1 change: 0 additions & 1 deletion internal/envoygateway/config/testdata/decoder/in/gateway-ratelimit.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ provider:
type: Kubernetes
kubernetes:
rateLimitDeployment:
replicas: 1
strategy:
type: RollingUpdate
container:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion ...infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion ...nfrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ metadata:
namespace: envoy-gateway-system
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ metadata:
uid: test-owner-reference-uid-for-deployment
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ metadata:
uid: test-owner-reference-uid-for-deployment
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ metadata:
uid: test-owner-reference-uid-for-deployment
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ metadata:
uid: test-owner-reference-uid-for-deployment
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
1 change: 0 additions & 1 deletion ...structure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ metadata:
uid: test-owner-reference-uid-for-deployment
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
Expand Down
2 changes: 1 addition & 1 deletion site/content/en/latest/api/extension_types.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1434,7 +1434,7 @@ _Appears in:_



KubernetesHorizontalPodAutoscalerSpec defines Kubernetes Horizontal Pod Autoscaler settings of Envoy Proxy Deployment. See k8s.io.autoscaling.v2.HorizontalPodAutoScalerSpec.
KubernetesHorizontalPodAutoscalerSpec defines Kubernetes Horizontal Pod Autoscaler settings of Envoy Proxy Deployment. When HPA is enabled, it is recommended that the value in `KubernetesDeploymentSpec.replicas` be removed, otherwise Envoy Gateway will revert back to this value every time reconciliation occurs. See k8s.io.autoscaling.v2.HorizontalPodAutoScalerSpec.

_Appears in:_
- [EnvoyProxyKubernetesProvider](#envoyproxykubernetesprovider)
Expand Down
Loading