-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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 go and remove v2 workarounds #30546
Conversation
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
/test release-notes_istio |
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@@ -2,10 +2,10 @@ | |||
"xds_servers": [ | |||
{ | |||
"server_uri": "localhost:14057", | |||
"channel_creds": [{"type": "insecure"}] | |||
"channel_creds": [{"type": "insecure"}], | |||
"server_features" : ["xds_v3"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed because the new grpc-go expects server_features to be child of xds_servers grpc/grpc-go#4087
/test integ-telemetry-mc-k8s-tests_istio |
@@ -29,6 +29,7 @@ run: | |||
skip-files: | |||
- ".*\\.pb\\.go" | |||
- ".*\\.gen\\.go" | |||
- pilot/pkg/networking/grpcgen/grpcgen_test.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to skip import order check. Otherwise we will have to set those env vars in the build file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, thats annoying. Ok, SGTM. I have a bug with GRPC to make this reasonable to configure
@@ -26,12 +25,16 @@ import ( | |||
"google.golang.org/grpc/resolver" | |||
"google.golang.org/grpc/serviceconfig" | |||
|
|||
// To setup the env vars needed for grpc-go. Should be loaded before grpc/xds is loaded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is golang initialization order based on import order or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is based on import ordering
* wip Signed-off-by: Rama Chavali <rama.rao@salesforce.com> * fix bootstrap Signed-off-by: Rama Chavali <rama.rao@salesforce.com> * remove unnecessary go.sum change Signed-off-by: Rama Chavali <rama.rao@salesforce.com> * add go.sum change Signed-off-by: Rama Chavali <rama.rao@salesforce.com> * make gen Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
This PR updates the grpc-go to latest released version and removes the workarounds for v2.
[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ X] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure
Pull Request Attributes
Please check any characteristics that apply to this pull request.
[X ] Does not have any changes that may affect Istio users.