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

xds client: Updated v3 type for http connection manager #4137

Merged
merged 2 commits into from
Jan 7, 2021

Conversation

pradeepmvn
Copy link
Contributor

Fix for Issue #4136
I am unable to validate the failure running unit tests for client_lds_tests.go

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 6, 2021

CLA Signed

The committers are authorized under a signed CLA.

@menghanl
Copy link
Contributor

menghanl commented Jan 7, 2021

Can you try changing

mcm, _ := proto.Marshal(cm)
lis := &v3listenerpb.Listener{
Name: v3LDSTarget,
ApiListener: &v3listenerpb.ApiListener{
ApiListener: &anypb.Any{
TypeUrl: version.V3HTTPConnManagerURL,
Value: mcm,
},

To

cmAny, _ := ptypes.MarshalAny(cm)

...

{
    ApiListener: cmAny,
}

And see if this would catch the bug? Thanks!

@pradeepmvn
Copy link
Contributor Author

ok sure. Misunderstood it. let me test using ptypes on ConnectionManager.

@pradeepmvn
Copy link
Contributor Author

Rolling back CM type to type.googleapis.com/envoy.config.filter.network.http_connection_manager.v3.HttpConnectionManager

and updating the unit test cases to ptype marshal caught the issue.

go test -run Test/UnmarshalListener_ClientSide

--- FAIL: Test/UnmarshalListener_ClientSide/v3_listener_resource (0.00s)
client_lds_test.go:298: UnmarshalListener([[type.googleapis.com/envoy.config.listener.v3.Listener]:{name:"lds.target.good:3333" api_listener:{api_listener:{[type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager]:{rds:{config_source:{ads:{}} route_config_name:"v3RouteConfig"}}}}}]) = (map[], xds: unexpected resource type: "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager" in LDS response) want (map[lds.target.good:3333:{v3RouteConfig <nil>}], false)
tlogger.go:99: client_xds.go:62 Resource with name: lds.target.good:2222, type: *envoy_config_listener_v3.Listener, contains: name:"lds.target.good:2222" api_listener:{api_listener:{[type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager]:{rds:{config_source:{ads:{}} route_config_name:"v2RouteConfig"}}}}  (t=+5.371401ms)
tlogger.go:99: client_xds.go:62 Resource with name: lds.target.good:3333, type: *envoy_config_listener_v3.Listener, contains: name:"lds.target.good:3333" api_listener:{api_listener:{[type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager]:{rds:{config_source:{ads:{}} route_config_name:"v3RouteConfig"}}}}  (t=+5.406124ms)

updating to new type type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager resolved unit tests and passed them

go test -run Test/UnmarshalListener_ClientSide
PASS
ok      google.golang.org/grpc/xds/internal/client      0.285s

Copy link
Contributor

@menghanl menghanl left a comment

Choose a reason for hiding this comment

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

Thanks so much for the fix and the test! LGTM!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants