From e7f6d0091edb6821b436ba36152775b0adbe48f5 Mon Sep 17 00:00:00 2001 From: Ardika Bagus Date: Thu, 7 Mar 2024 13:37:06 +0700 Subject: [PATCH 1/3] fix: remove default replicas function Signed-off-by: Ardika Bagus --- api/v1alpha1/kubernetes_helpers.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/api/v1alpha1/kubernetes_helpers.go b/api/v1alpha1/kubernetes_helpers.go index 72f6a380ad9..957fabe72b0 100644 --- a/api/v1alpha1/kubernetes_helpers.go +++ b/api/v1alpha1/kubernetes_helpers.go @@ -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{ @@ -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), @@ -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() } @@ -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() From 545eb65fa9841871ed46bfc9367e59115e100c8d Mon Sep 17 00:00:00 2001 From: Ardika Bagus Date: Thu, 7 Mar 2024 13:37:51 +0700 Subject: [PATCH 2/3] chore: omit replicas because nil equal to 1 by default Signed-off-by: Ardika Bagus --- api/v1alpha1/validation/envoygateway_validate_test.go | 5 +---- api/v1alpha1/validation/envoyproxy_validate_test.go | 2 -- internal/envoygateway/config/decoder_test.go | 1 - .../config/testdata/decoder/in/gateway-ratelimit.yaml | 1 - .../kubernetes/proxy/testdata/deployments/bootstrap.yaml | 1 - .../proxy/testdata/deployments/component-level.yaml | 1 - .../kubernetes/proxy/testdata/deployments/default.yaml | 1 - .../proxy/testdata/deployments/disable-prometheus.yaml | 1 - .../deployments/override-labels-and-annotations.yaml | 1 - .../proxy/testdata/deployments/patch-deployment.yaml | 1 - .../proxy/testdata/deployments/shutdown-manager.yaml | 1 - .../proxy/testdata/deployments/with-annotations.yaml | 1 - .../proxy/testdata/deployments/with-concurrency.yaml | 1 - .../proxy/testdata/deployments/with-extra-args.yaml | 1 - .../proxy/testdata/deployments/with-image-pull-secrets.yaml | 1 - .../proxy/testdata/deployments/with-node-selector.yaml | 1 - .../deployments/with-topology-spread-constraints.yaml | 1 - .../kubernetes/ratelimit/testdata/deployments/default.yaml | 1 - .../ratelimit/testdata/deployments/disable-prometheus.yaml | 1 - .../ratelimit/testdata/deployments/patch-deployment.yaml | 1 - .../ratelimit/testdata/deployments/with-node-selector.yaml | 1 - .../deployments/with-topology-spread-constraints.yaml | 1 - 22 files changed, 1 insertion(+), 26 deletions(-) diff --git a/api/v1alpha1/validation/envoygateway_validate_test.go b/api/v1alpha1/validation/envoygateway_validate_test.go index e03fdeb3016..612353ba88f 100644 --- a/api/v1alpha1/validation/envoygateway_validate_test.go +++ b/api/v1alpha1/validation/envoygateway_validate_test.go @@ -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, @@ -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) diff --git a/api/v1alpha1/validation/envoyproxy_validate_test.go b/api/v1alpha1/validation/envoyproxy_validate_test.go index 41d595a4d6d..93381bd527a 100644 --- a/api/v1alpha1/validation/envoyproxy_validate_test.go +++ b/api/v1alpha1/validation/envoyproxy_validate_test.go @@ -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) diff --git a/internal/envoygateway/config/decoder_test.go b/internal/envoygateway/config/decoder_test.go index 875f7bebcd5..eb2f24855db 100644 --- a/internal/envoygateway/config/decoder_test.go +++ b/internal/envoygateway/config/decoder_test.go @@ -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{ diff --git a/internal/envoygateway/config/testdata/decoder/in/gateway-ratelimit.yaml b/internal/envoygateway/config/testdata/decoder/in/gateway-ratelimit.yaml index 015d7e149ec..d767c07bab5 100644 --- a/internal/envoygateway/config/testdata/decoder/in/gateway-ratelimit.yaml +++ b/internal/envoygateway/config/testdata/decoder/in/gateway-ratelimit.yaml @@ -6,7 +6,6 @@ provider: type: Kubernetes kubernetes: rateLimitDeployment: - replicas: 1 strategy: type: RollingUpdate container: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml index 1e4064a3162..e40e9f75a6d 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml index 11b7ff5d5ff..eaaa4fe3070 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml index 866c1e4f393..0494805da44 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml index 6523569c80e..dd9f7589afe 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml index 253588c0665..f94e7d60edc 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml @@ -17,7 +17,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml index 31c18276aac..f21cea60099 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml index 43059817605..274f6824ada 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml index 9b1765fff3d..0fa8f3154d1 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml @@ -15,7 +15,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml index f4299e05180..30dbb84616a 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml index 41c17c28068..82ae60063a3 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml index fe7b608026c..d42d7c43ef6 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml index 6d544f57ddb..d3ca7b694cf 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml index b0b5afbc3f2..775076ec0d8 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml @@ -12,7 +12,6 @@ metadata: namespace: envoy-gateway-system spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml index 759a9c60199..34d9c86feb7 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml @@ -15,7 +15,6 @@ metadata: uid: test-owner-reference-uid-for-deployment spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml index 25da4c56e25..71ee276e730 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml @@ -15,7 +15,6 @@ metadata: uid: test-owner-reference-uid-for-deployment spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml index 3f075c640a7..1a87af97c57 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml @@ -15,7 +15,6 @@ metadata: uid: test-owner-reference-uid-for-deployment spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml index a83f8432aaa..a1729c6d430 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml @@ -15,7 +15,6 @@ metadata: uid: test-owner-reference-uid-for-deployment spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml index bddb780d39b..7367a5ace81 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml @@ -15,7 +15,6 @@ metadata: uid: test-owner-reference-uid-for-deployment spec: progressDeadlineSeconds: 600 - replicas: 1 revisionHistoryLimit: 10 selector: matchLabels: From 36cb5292afd867b6b4b4ff0e0e570891651f322e Mon Sep 17 00:00:00 2001 From: Ardika Bagus Date: Thu, 7 Mar 2024 13:50:39 +0700 Subject: [PATCH 3/3] chore: add a note when a user is being explicit on deployment replicas Signed-off-by: Ardika Bagus --- api/v1alpha1/shared_types.go | 2 ++ site/content/en/latest/api/extension_types.md | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/api/v1alpha1/shared_types.go b/api/v1alpha1/shared_types.go index 4014014e9df..a634995e1b5 100644 --- a/api/v1alpha1/shared_types.go +++ b/api/v1alpha1/shared_types.go @@ -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 +// 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" diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index a0b3f0047e6..c4b73799bef 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -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)