Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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!

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

Successfully merging this pull request may close these issues.

3 participants