Upstream gRPC service spec not populated properly by FDS

[stevenpyzhang]

Describe the bug grpc service spec is empty in the Upstream even though FDS successfully discovers the gRPC service

kubectl get upstream -n gloo-system default-test-service-8000 -o yaml
...
    serviceSpec:
      grpc: {}
...

From logs {"level":"info","ts":1626110957.4183335,"logger":"fds.v1.event_loop.fds.function-discovery-updater","caller":"grpc/grpc.go:115","msg":"tcp://test-service.default.svc.cluster.local:8000 discovered as a gRPC service","version":"1.7.11"}

discovery.solo.io/function_discovery: enabled was added to the Kubernetes service object to enable FDS.

To Reproduce

1. glooctl install gateway
2. Add discovery.solo.io/function_discovery: enabled label to the k8s service object
3. Create VS based on https://docs.solo.io/gloo-edge/master/guides/traffic_management/destination_types/grpc/#adding-a-virtual-service
4. grpcurl -plaintext $(glooctl proxy address --port http) list returns the service function definitions (so reflection is enabled)
5. kubectl get upstream -n gloo-system default-test-service-8000 -o yaml shows an empty grpc service spec

Expected behavior spec.kube.serviceSpec.grpc field should be filled out with the protobuf descriptor and the gRPC functions

Additional Context Not sure exactly what's going on since nothing in the logs, but maybe the protobuf descriptor string is too large (not sure if this is even possible..)

[chrisgaun]

@sam-heilbron Can you have a look?

[chrisgaun]

@stevenpyzhang which version were you using? Enterprise or Open Source?

[stevenpyzhang]

This happens on both 1.8.1 and 1.7.11 I'm using open source

I think I've found a workaround for the problem this caused though. Previously, I had been trying to create my static upstream by taking the information FDS puts into the discovered upstream yaml and copying that into the static upstream definition, but whenever I got it working with the discovered upstream, the field would be empty as mentioned above ^.

I eventually realized that if I left the grpc field as {} for my static upstream, Gloo Edge would still be able to connect to the upstream properly.

[sam-heilbron]

Hi @stevenpyzhang , yeah it looks like we could benefit from some additional error logging in this section of the code. Can you send along what grpc service spec you would expect to appear?

[stevenpyzhang]

I was expecting something similar to the example grpc service spec https://docs.solo.io/gloo-edge/master/guides/traffic_management/destination_types/grpc/#verify-that-grpc-functions-were-discovered The part I struggled with most was generating the correct protobuf descriptor on my own.

I think what could also be helpful is to indicate that the user doesn't have to fill out the grpc service spec manually when creating a static upstream.

[github-actions[bot]]

This issue has been marked as stale because of no activity in the last 180 days. It will be closed in the next 180 days unless it is tagged "no stalebot" or other activity occurs.