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 BackendTrafficPolicy #2053

Merged
merged 6 commits into from
Oct 24, 2023
Merged

Conversation

arkodg
Copy link
Contributor

@arkodg arkodg commented Oct 23, 2023

Takes PR #1961 forward

Takes PR envoyproxy#1961 forward

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #2053 (3939dab) into main (5a91d2d) will increase coverage by 0.04%.
The diff coverage is 55.18%.

@@            Coverage Diff             @@
##             main    #2053      +/-   ##
==========================================
+ Coverage   65.18%   65.22%   +0.04%     
==========================================
  Files          95       97       +2     
  Lines       13947    14199     +252     
==========================================
+ Hits         9091     9261     +170     
- Misses       4286     4363      +77     
- Partials      570      575       +5     
Files Coverage Δ
internal/gatewayapi/clienttrafficpolicy.go 74.33% <100.00%> (ø)
internal/gatewayapi/translator.go 98.42% <100.00%> (+0.29%) ⬆️
internal/message/types.go 0.00% <ø> (ø)
internal/status/backendtrafficpolicy.go 0.00% <0.00%> (ø)
internal/gatewayapi/runner/runner.go 22.03% <0.00%> (-0.98%) ⬇️
internal/status/status.go 0.00% <0.00%> (ø)
internal/gatewayapi/zz_generated.deepcopy.go 0.00% <0.00%> (ø)
internal/gatewayapi/resource.go 66.07% <0.00%> (-1.21%) ⬇️
internal/provider/kubernetes/controller.go 54.31% <25.58%> (+0.21%) ⬆️
internal/gatewayapi/backendtrafficpolicy.go 68.67% <68.67%> (ø)

... and 2 files with indirect coverage changes

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Alice-Lilith
Alice-Lilith previously approved these changes Oct 23, 2023
Copy link
Member

@Alice-Lilith Alice-Lilith left a comment

Choose a reason for hiding this comment

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

Thanks for taking this forward 🙏

@arkodg arkodg requested review from a team, kflynn and Xunzhuo and removed request for a team October 23, 2023 23:57
Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Alice-Lilith
Alice-Lilith previously approved these changes Oct 24, 2023
zirain
zirain previously approved these changes Oct 24, 2023
Copy link
Member

@zirain zirain left a comment

Choose a reason for hiding this comment

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

LGTM with small nits.

- There can be only be ONE policy resource attached to a specific `Listener` (section) within a `Gateway`
- If the policy targets a resource but cannot attach to it, this information should be reflected
in the Policy Status field using the `Conflicted=True` condition.
- If multiple polices target the same resource, the oldest resource (based on creation timestamp) will
Copy link
Member

Choose a reason for hiding this comment

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

just want to make sure all EG's CRD follow same rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes all policies follow similar rule

Co-authored-by: zirain <zirain2009@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
@arkodg arkodg dismissed stale reviews from zirain and Alice-Lilith via fb3c7ce October 24, 2023 01:20
Co-authored-by: zirain <zirain2009@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
@arkodg arkodg requested review from zirain and Alice-Lilith October 24, 2023 01:22
@zirain zirain merged commit a858547 into envoyproxy:main Oct 24, 2023
@arkodg arkodg deleted the backend-traffic-policy branch October 24, 2023 06:17
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.

3 participants