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.

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

feat: support LB priority for non xRoute endpoints #4033

Merged
merged 22 commits into from
Aug 17, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 3 additions & 0 deletions api/v1alpha1/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ 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.⬤
alexwo marked this conversation as resolved.
Show resolved Hide resolved
Failover *bool `json:"failover,omitempty"`
}

// BackendCluster contains all the configuration required for configuring access
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/wasm_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ type Wasm struct {
// +kubebuilder:default=false
FailOpen *bool `json:"failOpen,omitempty"`

// Priority defines the location of the Wasm extension in the HTTP filter chain.
alexwo marked this conversation as resolved.
Show resolved Hide resolved
// Failover defines the location of the Wasm extension in the HTTP filter chain.
// If not specified, the Wasm extension will be inserted before the router filter.
// Priority *uint32 `json:"priority,omitempty"`
// Failover *uint32 `json:"priority,omitempty"`
}

// WasmCodeSource defines the source of the Wasm code.
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ spec:
description: BackendRef defines how an ObjectReference that
is specific to BackendRef.
properties:
failover:
description: |-
Failover This indicates whether the backend is designated as a failover.
Multiple failover backends can be configured for a single BackendService.⬤
type: boolean
group:
default: ""
description: |-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10398,6 +10398,11 @@ spec:
description: BackendRef defines how an ObjectReference
that is specific to BackendRef.
properties:
failover:
description: |-
Failover This indicates whether the backend is designated as a failover.
Multiple failover backends can be configured for a single BackendService.⬤
type: boolean
group:
default: ""
description: |-
Expand Down Expand Up @@ -11269,6 +11274,11 @@ spec:
description: BackendRef defines how an ObjectReference
that is specific to BackendRef.
properties:
failover:
description: |-
Failover This indicates whether the backend is designated as a failover.
Multiple failover backends can be configured for a single BackendService.⬤
type: boolean
group:
default: ""
description: |-
Expand Down Expand Up @@ -12218,6 +12228,11 @@ spec:
description: BackendRef defines how an ObjectReference
that is specific to BackendRef.
properties:
failover:
description: |-
Failover This indicates whether the backend is designated as a failover.
Multiple failover backends can be configured for a single BackendService.⬤
type: boolean
group:
default: ""
description: |-
Expand Down Expand Up @@ -13103,6 +13118,11 @@ spec:
description: BackendRef defines how an ObjectReference
that is specific to BackendRef.
properties:
failover:
description: |-
Failover This indicates whether the backend is designated as a failover.
Multiple failover backends can be configured for a single BackendService.⬤
type: boolean
group:
default: ""
description: |-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,11 @@ spec:
description: BackendRef defines how an ObjectReference that
is specific to BackendRef.
properties:
failover:
description: |-
Failover This indicates whether the backend is designated as a failover.
Multiple failover backends can be configured for a single BackendService.⬤
type: boolean
group:
default: ""
description: |-
Expand Down Expand Up @@ -1130,6 +1135,11 @@ spec:
description: BackendRef defines how an ObjectReference that
is specific to BackendRef.
properties:
failover:
description: |-
Failover This indicates whether the backend is designated as a failover.
Multiple failover backends can be configured for a single BackendService.⬤
type: boolean
group:
default: ""
description: |-
Expand Down
2 changes: 1 addition & 1 deletion internal/gatewayapi/envoyextensionpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ func (t *Translator) buildExtProc(
}

ds, err = t.processExtServiceDestination(
&extProc.BackendRefs[i].BackendObjectReference,
&extProc.BackendRefs[i],
policyNamespacedName,
egv1a1.KindEnvoyExtensionPolicy,
ir.GRPC,
Expand Down
15 changes: 10 additions & 5 deletions internal/gatewayapi/ext_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (

// TODO: zhaohuabing combine this function with the one in the route translator
func (t *Translator) processExtServiceDestination(
backendRef *gwapiv1.BackendObjectReference,
backendRef *egv1a1.BackendRef,
policyNamespacedName types.NamespacedName,
policyKind string,
protocol ir.AppProtocol,
Expand All @@ -37,12 +37,12 @@ func (t *Translator) processExtServiceDestination(

switch KindDerefOr(backendRef.Kind, KindService) {
case KindService:
ds = t.processServiceDestinationSetting(*backendRef, backendNamespace, protocol, resources, envoyProxy)
ds = t.processServiceDestinationSetting(backendRef.BackendObjectReference, backendNamespace, protocol, resources, envoyProxy)
case egv1a1.KindBackend:
if !t.BackendEnabled {
return nil, fmt.Errorf("resource %s of type Backend cannot be used since Backend is disabled in Envoy Gateway configuration", string(backendRef.Name))
}
ds = t.processBackendDestinationSetting(*backendRef, backendNamespace, resources)
ds = t.processBackendDestinationSetting(backendRef.BackendObjectReference, backendNamespace, resources)
ds.Protocol = protocol
}

Expand All @@ -58,7 +58,7 @@ func (t *Translator) processExtServiceDestination(
}

backendTLS = t.applyBackendTLSSetting(
*backendRef,
backendRef.BackendObjectReference,
backendNamespace,
// Gateway is not the appropriate parent reference here because the owner
// of the BackendRef is the policy, and there is no hierarchy
Expand All @@ -78,7 +78,12 @@ func (t *Translator) processExtServiceDestination(

// TODO: support weighted non-xRoute backends
ds.Weight = ptr.To(uint32(1))

if backendRef.Failover != nil {
// set only the secondary priority, the backend defaults to a primary priority if unset.
if ptr.Deref(backendRef.Failover, false) {
ds.Priority = ptr.To(uint32(1))
}
}
return ds, nil
}

Expand Down
5 changes: 4 additions & 1 deletion internal/gatewayapi/securitypolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,7 @@ func (t *Translator) buildExtAuth(policy *egv1a1.SecurityPolicy, resources *Reso
authority string
err error
traffic *ir.TrafficFeatures
priority *bool
)

switch {
Expand All @@ -825,6 +826,7 @@ func (t *Translator) buildExtAuth(policy *egv1a1.SecurityPolicy, resources *Reso
backendRef = http.BackendRef
if len(http.BackendRefs) != 0 {
backendRef = egv1a1.ToBackendObjectReference(http.BackendRefs[0])
priority = http.BackendRefs[0].Failover
}
protocol = ir.HTTP
if traffic, err = translateTrafficFeatures(http.BackendSettings); err != nil {
Expand All @@ -834,6 +836,7 @@ func (t *Translator) buildExtAuth(policy *egv1a1.SecurityPolicy, resources *Reso
backendRef = grpc.BackendRef
if len(grpc.BackendRefs) != 0 {
backendRef = egv1a1.ToBackendObjectReference(grpc.BackendRefs[0])
priority = grpc.BackendRefs[0].Failover
}
protocol = ir.GRPC
if traffic, err = translateTrafficFeatures(grpc.BackendSettings); err != nil {
Expand All @@ -852,7 +855,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},
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@arkodg arkodg Aug 15, 2024

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@alexwo alexwo Aug 16, 2024

Choose a reason for hiding this comment

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

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

pnn,
KindSecurityPolicy,
protocol,
Expand Down
Loading
Loading