HDS support for gRPC Health Checks

[sean-jeffrey]

Title: HDS support for gRPC Health Checks

Description: I'm trying to use HDS to configure a gRPC health check, but its failling and I see this error in the logs:

...  cluster must support HTTP/2 for gRPC healthchecking

Digging into the code I see that the health checker code (source/common/upstream/health_checker_impl.cc) has a validation check to ensure that gRPC health checks can only be configured for clusters that have the HTTP2 feature present. It seems that clusters generated via HDS do not ever set the HTTP2 feature and so it it doesn't seem possible to configure a gRPC health check with HDS.

What I propose is that support for HttpProtocolOptions are added to the ClusterHealthCheck message proto so that Clusters built with HDS can have the HTTP2 feature set and thus gRPC health checks may be configured via HDS.

[sean-jeffrey]

I'm happy to submit a PR if the suggestion is reasonable :)

[adisuissa]

Can you explain the problem in a bit more detail and provide the config that is problematic? I think the tests in https://github.com/envoyproxy/envoy/blob/main/test/integration/hds_integration_test.cc are covering the right cases, no?

[sean-jeffrey]

Correct me if I'm mistaken but I don't see an any tests in https://github.com/envoyproxy/envoy/blob/main/test/integration/hds_integration_test.cc that use a gRPC health check specifier, only different forms of HTTP and TCP.

To restate the problem: I want to use the health discovery service to configure a gRPC health check https://github.com/envoyproxy/envoy/blob/21fba6209fdeee43c86a917b6a26823fd718d941/api/envoy/config/core/v3/health_check.proto#L205 but the config fails due to the validation at https://github.com/envoyproxy/envoy/blob/main/source/common/upstream/health_checker_impl.cc#L70 which requires the upstream cluster to be explicitly configured with the HTTP/2 feature. As far as I can tell the health discovery service does not offer a means for setting the HTTP/2 feature on an upstream cluster and so I'm at an impasse.

If that doesn't clear it up I can provide a config.

[adisuissa]

I see. I might have mistaken that with these test https://github.com/envoyproxy/envoy/blob/main/test/integration/health_check_integration_test.cc. Can you provide a config?

[sean-jeffrey]

Here is a sample config:

{
  "cluster_health_checks": [
    {
      "cluster_name": "my-cluster",
      "health_checks": [
        {
          "timeout": {
            "seconds": 1
          },
          "interval": {
            "seconds": 1
          },
          "unhealthy_threshold": {
            "value": 1
          },
          "healthy_threshold": {
            "value": 1
          },
          "HealthChecker": {
            "GrpcHealthCheck": {}
          },
          "no_traffic_interval": {
            "seconds": 1
          }
        }
      ],
      "locality_endpoints": [
        {
          "endpoints": [
            {
              "address": {
                "Address": {
                  "SocketAddress": {
                    "address": "10.0.0.1",
                    "PortSpecifier": {
                      "PortValue": 3000
                    }
                  }
                }
              }
            }
          ]
        }
      ]
    }
  ],
  "interval": {
    "seconds": 2
  }
}