feat: adding pod disruption budget support for envoy proxies #3583

alexwo · 2024-06-10T16:37:44Z

What this PR does / why we need it:
Allows to ensure that envoyproxy maintains a certain level of availability and resilience during maintenance operations.

Fixes # #3546

Signed-off-by: Alexander Volchok <[email protected]>

arkodg · 2024-06-10T16:43:32Z

api/v1alpha1/envoyproxy_types.go

@@ -277,6 +277,8 @@ type EnvoyProxyKubernetesProvider struct {
 	//
 	// +optional
 	UseListenerPortAsContainerPort *bool `json:"useListenerPortAsContainerPort,omitempty"`
+	// PodDisruptionBudget allows to control the pod disruption budget of ab Envoy Proxy.
+	PodDisruptionBudget *KubernetesPodDisruptionBudgetSpec `json:"podDisruptionBudget,omitempty"`


should this be EnvoyPdb instead ?

internal/infrastructure/kubernetes/infra_resource.go

arkodg · 2024-06-10T16:47:51Z

api/v1alpha1/envoyproxy_helpers.go

-		EnvoyService:    DefaultKubernetesService(),
+		EnvoyDeployment:     DefaultKubernetesDeployment(DefaultEnvoyProxyImage),
+		EnvoyService:        DefaultKubernetesService(),
+		PodDisruptionBudget: DefaultKubernetesPDB(),


this would enable it by default, but looks like we'd want this to be opt in like hpa

Indeed, I have just removed the default.

Signed-off-by: Alexander Volchok <[email protected]>

codecov · 2024-06-10T17:07:49Z

Codecov Report

Attention: Patch coverage is 68.42105% with 24 lines in your changes missing coverage. Please review.

Project coverage is 68.17%. Comparing base (0abdda7) to head (56f2798).

Files	Patch %	Lines
...ternal/infrastructure/kubernetes/infra_resource.go	72.72%	10 Missing and 2 partials ⚠️
...frastructure/kubernetes/proxy/resource_provider.go	76.92%	3 Missing and 3 partials ⚠️
internal/infrastructure/kubernetes/infra.go	0.00%	2 Missing and 2 partials ⚠️
...tructure/kubernetes/ratelimit/resource_provider.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3583   +/-   ##
=======================================
  Coverage   68.16%   68.17%           
=======================================
  Files         168      168           
  Lines       20408    20484   +76     
=======================================
+ Hits        13912    13964   +52     
- Misses       5494     5511   +17     
- Partials     1002     1009    +7

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

Signed-off-by: Alexander Volchok <[email protected]>

alexwo · 2024-06-11T06:43:51Z

/retest

Signed-off-by: Alexander Volchok <[email protected]>

alexwo · 2024-06-11T07:55:49Z

/retest

alexwo · 2024-06-11T07:56:13Z

/retest

alexwo · 2024-06-11T08:34:45Z

/retest

guydc

some minor comments

guydc · 2024-06-11T15:27:09Z

api/v1alpha1/envoyproxy_types.go

@@ -277,6 +277,10 @@ type EnvoyProxyKubernetesProvider struct {
 	//
 	// +optional
 	UseListenerPortAsContainerPort *bool `json:"useListenerPortAsContainerPort,omitempty"`
+
+	// EnvoyPDB allows to control the pod disruption budget of ab Envoy Proxy.


nit: ab => an

guydc · 2024-06-11T15:27:47Z

charts/gateway-addons-helm/README.md

@@ -0,0 +1,96 @@
+# gateway-addons-helm


Should this be generated as part of this PR?

There was a bug that caused this to be generated, however, now its fixed. here->bae64e3

guydc · 2024-06-11T15:39:43Z

api/v1alpha1/shared_types.go

+	// such as node drains or updates. This setting ensures that your envoy proxy maintains a certain level of availability
+	// and resilience during maintenance operations.
+	// +optional
+	MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty" protobuf:"bytes,1,opt,name=minAvailable"`


Do we need IntOrString here? The original K8s schema uses this type, but in other places in the EG API we expose use an Int and the convert to IntOrString if required.

Also, do we need protobuf:"bytes,1,opt,name=minAvailable" here?

guydc · 2024-06-11T15:41:06Z

internal/infrastructure/kubernetes/proxy/resource_provider.go

+				MatchLabels: labels,
+			},
+		},
+		Status: v1.PodDisruptionBudgetStatus{},


Is the empty status needed here?

Signed-off-by: Alexander Volchok <[email protected]>

alexwo · 2024-06-11T15:59:25Z

some minor comments

Thanks Guy, updated.

Signed-off-by: Alexander Volchok <[email protected]>

guydc

LGTM, Thanks!

arkodg · 2024-06-11T19:03:26Z

hey @alexwo DCO is failing, can you squash, sign and repush ?

Signed-off-by: Alexander Volchok <[email protected]>

alexwo · 2024-06-12T04:16:25Z

/retest

Signed-off-by: Alexander Volchok <[email protected]>

alexwo · 2024-06-12T04:46:51Z

/retest

shawnh2

LGTM! Just one nit

shawnh2 · 2024-06-12T07:52:34Z

internal/infrastructure/kubernetes/proxy/resource_provider.go

@@ -13,6 +13,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/policy/v1"


better go with policyv1 here. same for all other places.

Signed-off-by: Alexander Volchok <[email protected]>

adding pod disruption budget support for envoy proxies

8fe497f

Signed-off-by: Alexander Volchok <[email protected]>

alexwo requested a review from a team as a code owner June 10, 2024 16:37

updating

e1cde55

Signed-off-by: Alexander Volchok <[email protected]>

arkodg reviewed Jun 10, 2024

View reviewed changes

internal/infrastructure/kubernetes/infra_resource.go Show resolved Hide resolved

arkodg reviewed Jun 10, 2024

View reviewed changes

arkodg mentioned this pull request Jun 10, 2024

add support to PodDisruptionBudget for envoyproxy / eg control plane #3546

Closed

2 tasks

alexwo added 2 commits June 10, 2024 19:00

remove DefaultKubernetesPDB so that we can opt in

662d2cb

Signed-off-by: Alexander Volchok <[email protected]>

rename to envoy pdb

33e3633

Signed-off-by: Alexander Volchok <[email protected]>

alexwo added 3 commits June 10, 2024 20:34

updating

3c68fb4

Signed-off-by: Alexander Volchok <[email protected]>

Merge branch 'main' into pdb

35a70bf

updating

554d72a

Signed-off-by: Alexander Volchok <[email protected]>

alexwo force-pushed the pdb branch from 34f7e5e to 554d72a Compare June 11, 2024 06:11

alexwo added 2 commits June 11, 2024 09:01

Merge branch 'main' into pdb

029030e

updating docs

bdf9a99

Signed-off-by: Alexander Volchok <[email protected]>

alexwo force-pushed the pdb branch from fd9eef5 to bdf9a99 Compare June 11, 2024 07:10

guydc reviewed Jun 11, 2024

View reviewed changes

alexwo added 3 commits June 11, 2024 17:49

updating

5c631ea

Signed-off-by: Alexander Volchok <[email protected]>

updating

7fb3ad6

Signed-off-by: Alexander Volchok <[email protected]>

updating

0f04260

Signed-off-by: Alexander Volchok <[email protected]>

updating

3e41c87

Signed-off-by: Alexander Volchok <[email protected]>

guydc previously approved these changes Jun 11, 2024

View reviewed changes

updating

477d36b

Signed-off-by: Alexander Volchok <[email protected]>

alexwo force-pushed the pdb branch from ad33dad to 477d36b Compare June 11, 2024 23:07

alexwo added 2 commits June 12, 2024 01:08

Merge branch 'main' into pdb

754f2be

Merge branch 'main' into pdb

98ee081

updatig

237b4b8

Signed-off-by: Alexander Volchok <[email protected]>

alexwo dismissed guydc’s stale review via 237b4b8 June 12, 2024 04:21

shawnh2 approved these changes Jun 12, 2024

View reviewed changes

updating

56f2798

Signed-off-by: Alexander Volchok <[email protected]>

guydc approved these changes Jun 12, 2024

View reviewed changes

guydc merged commit 7fdc738 into envoyproxy:main Jun 12, 2024
26 checks passed

alexwo deleted the pdb branch June 13, 2024 07:03

Skaronator mentioned this pull request Dec 3, 2024

Add support for PDB maxUnavailable in EnvoyProxy CRD #4840

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adding pod disruption budget support for envoy proxies #3583

feat: adding pod disruption budget support for envoy proxies #3583

alexwo commented Jun 10, 2024 •

edited

Loading

arkodg Jun 10, 2024

arkodg Jun 10, 2024

alexwo Jun 10, 2024 •

edited

Loading

codecov bot commented Jun 10, 2024 •

edited

Loading

alexwo commented Jun 11, 2024

alexwo commented Jun 11, 2024

alexwo commented Jun 11, 2024

alexwo commented Jun 11, 2024

guydc left a comment

guydc Jun 11, 2024

guydc Jun 11, 2024

alexwo Jun 11, 2024 •

edited

Loading

guydc Jun 11, 2024

guydc Jun 11, 2024

alexwo Jun 11, 2024

alexwo commented Jun 11, 2024 •

edited

Loading

guydc left a comment

arkodg commented Jun 11, 2024

alexwo commented Jun 12, 2024

alexwo commented Jun 12, 2024

shawnh2 left a comment

shawnh2 Jun 12, 2024

feat: adding pod disruption budget support for envoy proxies #3583

feat: adding pod disruption budget support for envoy proxies #3583

Conversation

alexwo commented Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexwo Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Jun 10, 2024 • edited Loading

Codecov Report

alexwo commented Jun 11, 2024

alexwo commented Jun 11, 2024

alexwo commented Jun 11, 2024

alexwo commented Jun 11, 2024

guydc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexwo Jun 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexwo commented Jun 11, 2024 • edited Loading

guydc left a comment

Choose a reason for hiding this comment

arkodg commented Jun 11, 2024

alexwo commented Jun 12, 2024

alexwo commented Jun 12, 2024

shawnh2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexwo commented Jun 10, 2024 •

edited

Loading

alexwo Jun 10, 2024 •

edited

Loading

codecov bot commented Jun 10, 2024 •

edited

Loading

alexwo Jun 11, 2024 •

edited

Loading

alexwo commented Jun 11, 2024 •

edited

Loading