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

Update GRPC dependency #16591

Closed
wants to merge 5 commits into from
Closed

Conversation

rsafonseca
Copy link

Fixes testing packages compatibility issues with grpc from 1.55.0 onwards
Resolves #15877

rsafonseca and others added 3 commits September 13, 2023 10:02
target.Endpoint and some other fields are deprecated, URL field is
suggested to use instead
path is required to be stripped of "/" prefix for naming/resolver to
work porperly

Signed-off-by: Ramil Mirhasanov <[email protected]>
Signed-off-by: Rafael da Fonseca <[email protected]>
Signed-off-by: Rafael da Fonseca <[email protected]>
Signed-off-by: Rafael da Fonseca <[email protected]>
Signed-off-by: Rafael da Fonseca <[email protected]>
@ahrtr
Copy link
Member

ahrtr commented Sep 18, 2023

Note that 3.5 has huge difference from the main branch in terms of code structure, so usually it isn't recommended to cherry pick any commits from main.

@chaochn47 since you are the original author for bumping grpc in main branch, could you guide/work together the contributor to complete this PR?

@chaochn47
Copy link
Member

chaochn47 commented Sep 18, 2023

Hey @rsafonseca sorry for the late reply.

This PR updates gRPC directly from v1.41.0 to v1.58.0 by cherry-picking #16338. It does not look correct because it missed quite a lot test updates when we upgraded from v1.41.0 to v1.52.0.

For example:

The following test failure of current PR was caused by unix://localhost:port was never officially supported. As part of the gRPC-go version update, we have to change to unix:localhost:port just like #14125.

logger.go:130: 2023-09-14T07:48:09.275Z	ERROR	default	grpc gateway failed to dial	{"addr": "unix://localhost:41618", "error": "failed to build resolver: invalid (non-empty) authority: localhost:41618"}

Please back port all gRPC updates and associated tests since main and release-3.5 diverged. I just blamed the change history of root go.mod.

The history from v1.41.0 to v1.58.0 is

I would recommend you back port them one by one in ascending order and check if the local test passes or not by following the contributing guide. Let me know when the first PR is out.

I will help you complete this and eventually fix #15877. If you prefer DM, I have sent you the invitation to join the kubernetes slack workplace and I am in the #etcd channel.

@chaochn47
Copy link
Member

In order to use Outlier Detection, release-3.5 should be at least gRPC-go v1.50.0 onward. Working on it..

@moficodes
Copy link
Member

Discussed during etcd triage. I can take a look to confirm this changes are already satisfied with a different PR.

@wenjiaswe
Copy link
Contributor

wenjiaswe commented Dec 7, 2023

/assign @moficodes Thank you very much Mofi!

@rsafonseca
Copy link
Author

I can confirm that they are! This was handled in another thread, we can close it :)

@rsafonseca rsafonseca closed this Dec 12, 2023
@moficodes moficodes removed their assignment May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants