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: optimize edsClusterConfig for the association between cluster resources and endpoint resources in envoy's configdump?include_eds interface #1414

Merged
merged 2 commits into from
May 11, 2023

Conversation

tmsnan
Copy link
Contributor

@tmsnan tmsnan commented May 10, 2023

In the config_dump?inclueds_eds interface of the envoy of the gateway, there is no association information between the endpoints resource and the cluster resource, which is inconvenient for us to troubleshoot, as shown in the following figure
4231c518e0472eea72e2e81658663b40
After optimization, the picture is as follows:
0f54296543a303df65e78bf33f79c080

@tmsnan tmsnan requested a review from a team as a code owner May 10, 2023 06:12
@arkodg
Copy link
Contributor

arkodg commented May 10, 2023

sounds good @tmsnan this PR looks fine to me
recommend running make test locally to fix the errors, most of the tests are YAML based and you will need to insert the new cluster field into the YAML testdata

@qicz qicz added kind/enhancement New feature or request area/translator Issues related to Gateway's translation service, e.g. translating Gateway APIs into the IR. area/egctl labels May 11, 2023
@qicz qicz added this to the 0.5.0-rc1 milestone May 11, 2023
Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
@tmsnan tmsnan force-pushed the dev-optimize-edsClusterConfig branch from dbcead1 to a7ec28e Compare May 11, 2023 07:42
@tmsnan
Copy link
Contributor Author

tmsnan commented May 11, 2023

done

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution !

@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #1414 (a7ec28e) into main (5f222eb) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1414      +/-   ##
==========================================
+ Coverage   62.26%   62.35%   +0.08%     
==========================================
  Files          79       79              
  Lines       11184    11185       +1     
==========================================
+ Hits         6964     6974      +10     
+ Misses       3764     3757       -7     
+ Partials      456      454       -2     
Impacted Files Coverage Δ
internal/xds/translator/cluster.go 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

Copy link
Member

@qicz qicz left a comment

Choose a reason for hiding this comment

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

lgtm

@arkodg
Copy link
Contributor

arkodg commented May 11, 2023

@zirain looks like RL E2E is flaky

@arkodg arkodg merged commit ad58bab into envoyproxy:main May 11, 2023
18 checks passed
tanujd11 pushed a commit to tanujd11/gateway that referenced this pull request May 20, 2023
…esources and endpoint resources in envoy's configdump?include_eds interface (envoyproxy#1414)

* optimize edsClusterConfig

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
@tmsnan tmsnan deleted the dev-optimize-edsClusterConfig branch September 6, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/egctl area/translator Issues related to Gateway's translation service, e.g. translating Gateway APIs into the IR. kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants