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.

Sign up for GitHub

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

Jump to bottom

Fix some delta xds proxy logics #47813

Merged
merged 1 commit into from
Nov 13, 2023
Merged

Fix some delta xds proxy logics #47813

merged 1 commit into from
Nov 13, 2023

Conversation

hanxiaop
Copy link
Member

Please provide a description of this PR:
This is to sync logics with xds proxy. Found the inconsistencies when I was debugging.

To help us figure out who should review this PR, please put an X in all the areas that this PR affects.

  • Ambient
  • Configuration Infrastructure
  • Docs
  • Dual Stack
  • Installation
  • Networking
  • Performance and Scalability
  • Policies and Telemetry
  • Security
  • Test and Release
  • User Experience
  • Developer Infrastructure

Please check any characteristics that apply to this pull request.

  • Does not have any user-facing changes. This may include CLI changes, API changes, behavior changes, performance improvements, etc.

@hanxiaop hanxiaop requested review from a team as code owners November 10, 2023 06:36
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 10, 2023
@hanxiaop hanxiaop changed the title Fix some delta xds proxy logic Fix some delta xds proxy logics Nov 10, 2023
@hanxiaop hanxiaop added the release-notes-none Indicates a PR that does not require release notes. label Nov 10, 2023
@hanxiaop
Copy link
Member Author

/test release-notes

@hanxiaop hanxiaop requested a review from hzxuzhonghu November 10, 2023 06:41
hzxuzhonghu
hzxuzhonghu reviewed Nov 10, 2023
View reviewed changes
Copy link
Member

@hzxuzhonghu hzxuzhonghu left a comment

Choose a reason for hiding this comment

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

Is this kind of code refactor and no other change? I will take a closer look then

@hanxiaop
Copy link
Member Author

Is this kind of code refactor and no other change? I will take a closer look then

@hzxuzhonghu Not a refactor, but it only involves changes that are already in the xds proxy logic, many of which I noticed were made by you, and have not been added in delta xds.

hzxuzhonghu
hzxuzhonghu approved these changes Nov 13, 2023
View reviewed changes
Copy link
Member

@hzxuzhonghu hzxuzhonghu left a comment

Choose a reason for hiding this comment

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

lgtm

@hanxiaop
Copy link
Member Author

/test integ-security-istiodremote

2 similar comments
@hanxiaop
Copy link
Member Author

/test integ-security-istiodremote

@hanxiaop
Copy link
Member Author

/test integ-security-istiodremote

@hanxiaop
Copy link
Member Author

hanxiaop commented Nov 13, 2023 •
edited
Loading

/cc @zirain I think this https://prow.istio.io/view/gs/istio-prow/pr-logs/pull/istio_istio/47813/integ-security-istiodremote_istio/1723943975414403072 is a test flake introduced by #47641

@hanxiaop
Copy link
Member Author

try again /test integ-security-istiodremote

@hzxuzhonghu
Copy link
Member

/test integ-security-istiodremote

@istio-testing istio-testing merged commit 02f34d8 into istio:master Nov 13, 2023
@hanxiaop hanxiaop mentioned this pull request Feb 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hzxuzhonghu hzxuzhonghu hzxuzhonghu approved these changes

Assignees
No one assigned
Labels
area/networking release-notes-none Indicates a PR that does not require release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hanxiaop @hzxuzhonghu @istio-testing