Skip to content

LoadBalancer in BackendTrafficPolicy#2063

Merged
arkodg merged 2 commits intoenvoyproxy:mainfrom
arkodg:lb-api
Oct 25, 2023
Merged

LoadBalancer in BackendTrafficPolicy#2063
arkodg merged 2 commits intoenvoyproxy:mainfrom
arkodg:lb-api

Conversation

@arkodg
Copy link
Copy Markdown
Contributor

@arkodg arkodg commented Oct 25, 2023

Fixes: #1105

Fixes: envoyproxy#1105

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg requested a review from a team as a code owner October 25, 2023 04:34
@arkodg arkodg marked this pull request as draft October 25, 2023 04:34
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 25, 2023

Codecov Report

Merging #2063 (1a578ca) into main (7fa96a7) will increase coverage by 0.04%.
Report is 1 commits behind head on main.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##             main    #2063      +/-   ##
==========================================
+ Coverage   65.03%   65.07%   +0.04%     
==========================================
  Files          99       99              
  Lines       14536    14578      +42     
==========================================
+ Hits         9453     9487      +34     
- Misses       4496     4504       +8     
  Partials      587      587              
Files Coverage Δ
internal/gatewayapi/backendtrafficpolicy.go 66.66% <81.81%> (+2.12%) ⬆️

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg marked this pull request as ready for review October 25, 2023 21:54
@arkodg arkodg requested review from a team, cnvergence, tanujd11 and zhaohuabing and removed request for a team October 25, 2023 22:00
// set to ConsistentHash
//
// +optional
ConsistentHash *ConsistentHash `json:"consistentHash,omitempty"`
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing Oct 25, 2023

Choose a reason for hiding this comment

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

  • What happens if the type is ConsistentHash and the ConsistentHash is nil?

  • If Source IP is the only hash source we would like to support now, maybe just remove the ConsistentHash property and add a comment, or we can add another common-used hash source, like headers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is what @zirain is referring to in #2063 (comment)
we can add CEL Validation to proactively check this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you open a ticket for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

raised #2066

@arkodg arkodg merged commit d760d6d into envoyproxy:main Oct 25, 2023
@arkodg arkodg deleted the lb-api branch October 25, 2023 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

proposal: LoadbalancingPolicy

3 participants