feat: support LB priority for non xRoute endpoints #4033

alexwo · 2024-08-12T14:21:45Z

What does this PR do?

It enables users to configure failover backends.

How is this achieved?

The proposal involves adding a Failover boolean property to backend refs, what allows to indicate whether the backend is designated as a failover.

This approach is illustrated in this PR for external extProc.

As per issue #3055

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

codecov · 2024-08-12T14:28:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.92%. Comparing base (683b5b5) to head (9277ec6).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4033      +/-   ##
==========================================
+ Coverage   67.87%   67.92%   +0.05%     
==========================================
  Files         187      187              
  Lines       23023    23030       +7     
==========================================
+ Hits        15626    15644      +18     
+ Misses       6277     6271       -6     
+ Partials     1120     1115       -5

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

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

alexwo · 2024-08-12T20:06:51Z

/retest

alexwo · 2024-08-12T20:24:17Z

/retest

alexwo · 2024-08-13T06:03:41Z

/retest

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

guydc

Can we also add an XDS translation test?

api/v1alpha1/wasm_types.go

api/v1alpha1/shared_types.go

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

alexwo · 2024-08-15T09:42:01Z

/retest

alexwo · 2024-08-15T09:42:20Z

Can we also add an XDS translation test?

added

alexwo · 2024-08-15T11:03:20Z

/retest

alexwo · 2024-08-15T11:03:38Z

Can we also add an XDS translation test?

added

guydc · 2024-08-15T12:02:52Z

overall LGTM.

cc @arkodg

In terms of naming, I think that failover is a common industry term used by Akamai, Cloudflare, GCP, AWS and Azure in this context. Nginx and HAProxy use "backup".

I think that "backup" can be a bit vague in this context:

It's associated with data backup/restore operations
It doesn't express the fact that the "backup" backend will automatically handle traffic in case of primary backend failure. Users may anticipate that they need to "enable" a backup server as a primary in case of failure.

We need to follow up with an E2E and user docs that explain how priorities interact with Active/Passive healthchecks and discovery.

alexwo · 2024-08-15T12:40:21Z

/retest

api/v1alpha1/shared_types.go

Co-authored-by: zirain <zirain2009@gmail.com> Signed-off-by: Alex Volchok <alex.volchok@sap.com>

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

alexwo · 2024-08-15T16:47:05Z

/retest

arkodg · 2024-08-15T21:26:50Z

api/v1alpha1/shared_types.go

@@ -473,6 +473,10 @@ type BackendRef struct {
 	// BackendObjectReference references a Kubernetes object that represents the backend.
 	// Only Service kind is supported for now.
 	gwapiv1.BackendObjectReference `json:",inline"`
+	// Failover This indicates whether the backend is designated as a failover.
+	// Multiple failover backends can be configured for a single BackendService.


Suggested change

// Multiple failover backends can be configured for a single BackendService.

// Multiple failover backends can be configured.

arkodg · 2024-08-15T22:02:56Z

api/v1alpha1/shared_types.go

@@ -473,6 +473,10 @@ type BackendRef struct {
 	// BackendObjectReference references a Kubernetes object that represents the backend.
 	// Only Service kind is supported for now.
 	gwapiv1.BackendObjectReference `json:",inline"`
+	// Failover This indicates whether the backend is designated as a failover.
+	// Multiple failover backends can be configured for a single BackendService.
+	// +optional


"Its strongly recommended to configure active or passive healthchecks to ensure failover can be detected when the active backends are unhealthy and can be readjusted once the primary backends are healthy again. The overprovisioning factor is set to 1.4 i.e. the failover backends will receive traffic only once the health of the active backends drops below 72%"

arkodg · 2024-08-15T22:09:37Z

internal/gatewayapi/securitypolicy.go

@@ -851,7 +854,7 @@ func (t *Translator) buildExtAuth(policy *egv1a1.SecurityPolicy, resources *Reso
 	authority = backendRefAuthority(resources, backendRef, policy)
 	pnn := utils.NamespacedName(policy)
 	if ds, err = t.processExtServiceDestination(
-		backendRef,
+		&egv1a1.BackendRef{BackendObjectReference: *backendRef, Failover: priority},


can we modify this to Failover: ptr.Deref(grpc.BackendRefs[0].Failover , false)

this line is typecasting from bool from int32 ? can we make it more explicit, no strong preference on using bool or int in IR

we don't have to type cast (its bool type both in ir/ api), priority var should be renamed failover in this context

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

alexwo · 2024-08-16T05:31:42Z

/retest

arkodg

LGTM thanks !

zirain · 2024-08-17T01:25:32Z

/retest

zirain · 2024-08-17T05:38:03Z

/retest

Adds a `failover` field to Backend API so we can support Active/Passive Failove backends within xRoutes similar to envoyproxy#4033 Relates to envoyproxy#3055 Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* [api] Add Failover field to Backend Adds a `failover` field to Backend API so we can support Active/Passive Failove backends within xRoutes similar to #4033 Relates to #3055 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix doc string Signed-off-by: Arko Dasgupta <arko@tetrate.io> * notImplementedHide Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io>

add priority support for backend refs

d595d82

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

alexwo requested a review from a team as a code owner August 12, 2024 14:21

alexwo added 2 commits August 12, 2024 21:27

fixing lint adding generate

841afdc

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

updating

31f1aa5

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

readd generates

5e906b4

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

alexwo force-pushed the prio_support branch from 497483a to 5e906b4 Compare August 14, 2024 06:12

alexwo mentioned this pull request Aug 14, 2024

Support LB priority for endpoints #3055

Open

Merge branch 'main' into prio_support

8d486dc

guydc reviewed Aug 15, 2024

View reviewed changes

api/v1alpha1/wasm_types.go Outdated Show resolved Hide resolved

api/v1alpha1/shared_types.go Outdated Show resolved Hide resolved

alexwo added 6 commits August 15, 2024 09:41

format and style fixes

9f94d03

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

Merge branch 'main' into prio_support

eea8f73

updating gen

9410100

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

updating gen

13fe06d

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

adding translation tests

bbc8ab7

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

adding translation tests

0b78365

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

alexwo force-pushed the prio_support branch from 15205f0 to 0b78365 Compare August 15, 2024 08:47

adding nl

7ffba68

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

guydc previously approved these changes Aug 15, 2024

View reviewed changes

zirain reviewed Aug 15, 2024

View reviewed changes

api/v1alpha1/shared_types.go Outdated Show resolved Hide resolved

alexwo dismissed guydc’s stale review via edfb775 August 15, 2024 14:29

alexwo and others added 4 commits August 15, 2024 16:29

Update api/v1alpha1/shared_types.go

edfb775

Co-authored-by: zirain <zirain2009@gmail.com> Signed-off-by: Alex Volchok <alex.volchok@sap.com>

Merge branch 'main' into prio_support

e9819b3

updating

ba85b4c

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

updating

c493830

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

arkodg changed the title ~~[feat] support LB priority for endpoints~~ [feat] support LB priority for non xRoute endpoints Aug 15, 2024

alexwo added 2 commits August 15, 2024 20:56

Merge branch 'main' into prio_support

dc57f0d

Merge branch 'main' into prio_support

8a9b487

arkodg reviewed Aug 15, 2024

View reviewed changes

alexwo added 2 commits August 16, 2024 06:39

updating

42ab608

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

Merge branch 'prio_support' of github.com:alexwo/gateway

d1974c2

Signed-off-by: Alexander Volchok <alex.volchok@sap.com>

Merge branch 'main' into prio_support

d786391

arkodg approved these changes Aug 16, 2024

View reviewed changes

arkodg requested review from a team August 16, 2024 19:48

zirain approved these changes Aug 17, 2024

View reviewed changes

Merge branch 'main' into prio_support

9277ec6

zirain changed the title ~~[feat] support LB priority for non xRoute endpoints~~ feat: support LB priority for non xRoute endpoints Aug 17, 2024

zirain merged commit 7f2c151 into envoyproxy:main Aug 17, 2024
26 of 27 checks passed

alexwo deleted the prio_support branch August 17, 2024 09:15

arkodg mentioned this pull request Aug 22, 2024

[api] Add Failover field to Backend #4099

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support LB priority for non xRoute endpoints #4033

feat: support LB priority for non xRoute endpoints #4033

alexwo commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 12, 2024 •

edited

Loading

alexwo commented Aug 12, 2024

alexwo commented Aug 12, 2024

alexwo commented Aug 13, 2024

guydc left a comment

alexwo commented Aug 15, 2024

alexwo commented Aug 15, 2024

alexwo commented Aug 15, 2024

alexwo commented Aug 15, 2024

guydc commented Aug 15, 2024 •

edited

Loading

alexwo commented Aug 15, 2024

alexwo commented Aug 15, 2024

arkodg Aug 15, 2024

alexwo Aug 16, 2024

arkodg Aug 15, 2024

arkodg Aug 15, 2024

arkodg Aug 15, 2024 •

edited

Loading

alexwo Aug 16, 2024 •

edited

Loading

alexwo commented Aug 16, 2024

arkodg left a comment

zirain commented Aug 17, 2024

zirain commented Aug 17, 2024

	// Multiple failover backends can be configured for a single BackendService.
	// Multiple failover backends can be configured.

feat: support LB priority for non xRoute endpoints #4033

feat: support LB priority for non xRoute endpoints #4033

Conversation

alexwo commented Aug 12, 2024 • edited Loading

What does this PR do?

How is this achieved?

codecov bot commented Aug 12, 2024 • edited Loading

Codecov Report

alexwo commented Aug 12, 2024

alexwo commented Aug 12, 2024

alexwo commented Aug 13, 2024

guydc left a comment

Choose a reason for hiding this comment

alexwo commented Aug 15, 2024

alexwo commented Aug 15, 2024

alexwo commented Aug 15, 2024

alexwo commented Aug 15, 2024

guydc commented Aug 15, 2024 • edited Loading

alexwo commented Aug 15, 2024

alexwo commented Aug 15, 2024

arkodg Aug 15, 2024

Choose a reason for hiding this comment

alexwo Aug 16, 2024

Choose a reason for hiding this comment

arkodg Aug 15, 2024

Choose a reason for hiding this comment

arkodg Aug 15, 2024

Choose a reason for hiding this comment

arkodg Aug 15, 2024 • edited Loading

Choose a reason for hiding this comment

alexwo Aug 16, 2024 • edited Loading

Choose a reason for hiding this comment

alexwo commented Aug 16, 2024

arkodg left a comment

Choose a reason for hiding this comment

zirain commented Aug 17, 2024

zirain commented Aug 17, 2024

alexwo commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 12, 2024 •

edited

Loading

guydc commented Aug 15, 2024 •

edited

Loading

arkodg Aug 15, 2024 •

edited

Loading

alexwo Aug 16, 2024 •

edited

Loading