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

Allow multiple endpoints in Envoy clusters configured with hostnames #21655

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

t-davies
Copy link
Contributor

@t-davies t-davies commented Aug 24, 2024

Description

Currently when Consul generates configuration for Envoy, it behaves the same for both LOGICAL_DNS and STRICT_DNS discovery types and provides only a single endpoint to Envoy in both modes. This means that when multiple hostnames are configured in STRICT_DNS mode, requests are not load balanced over all available hosts.

This change configures Envoy with all available hostnames when the discovery type is set to STRICT_DNS. In LOGICAL_DNS mode, Envoy supports only a single endpoint and so the current behaviour of Consul remains unchanged.

Testing & Reproduction steps

Tested manually, as follows:

  1. Create Consul cluster

    consul agent -dev
    
  2. Create 2 instances of the same service ("example-service"), with different hostnames.

    curl --request PUT \
      --url http://localhost:8500/v1/catalog/register \
      --header 'Content-Type: application/json' \
      --data '{
        "Datacenter": "dc1",
        "Node": "ext-headers.jsontest.com",
        "ID": "adb6d185-300c-40f3-b81b-799cc57f889a",
        "Address": "headers.jsontest.com",
        "NodeMeta": {
          "external-node": "true",
          "external-probe": "false"
        },
        "Service": {
          "ID": "example-service-a063a65b",
          "Service": "example-service",
          "Tags": [],
          "Address": "headers.jsontest.com",
          "Port": 80
        },
        "Checks": [{
          "CheckID": "ext-fff3e68d",
          "Name": "ext-default",
          "Status": "passing",
          "ServiceID": "example-service-a063a65b",
          "Definition": {
            "TCP": "headers.jsontest.com:80",
            "Interval": "15s",
            "Timeout": "5s",
            "DeregisterCriticalServiceAfter": "60m"
           }
        }]
      }'
    
    curl --request PUT \
      --url http://localhost:8500/v1/catalog/register \
      --header 'Content-Type: application/json' \
      --data '{
        "Datacenter": "dc1",
        "Node": "ext-ip.jsontest.com",
        "ID": "b676fcbd-44c8-4bf8-ac05-a3b346e8b3fb",
        "Address": "ip.jsontest.com",
        "NodeMeta": {
          "external-node": "true",
          "external-probe": "false"
        },
        "Service": {
          "ID": "example-service-d94bc56d",
          "Service": "example-service",
          "Tags": [],
          "Address": "ip.jsontest.com",
          "Port": 80
        },
        "Checks": [{
          "CheckID": "ext-49daef29",
          "Name": "ext-default",
          "Status": "passing",
          "ServiceID": "example-service-d94bc56d",
          "Definition": {
            "TCP": "ip.jsontest.com:80",
            "Interval": "15s",
            "Timeout": "5s",
            "DeregisterCriticalServiceAfter": "60m"
           }
        }]
      }'
    
  3. Create terminating gateway, configured with "example-service" as a destination.

    curl --request PUT \
      --url http://localhost:8500/v1/config \
      --header 'Content-Type: application/json' \
      --data '{
     "Kind": "terminating-gateway",
     "Name": "ext-terminating-gateway",
     "Services": [
       {
         "Name": "example-service"
       }
     ]
    }'
    
    curl --request PUT \
      --url http://localhost:8500/v1/agent/service/register \
      --header 'Content-Type: application/json' \
      --data '{
        "Kind": "terminating-gateway",
        "Name": "ext-terminating-gateway",
        "Port": 22500
    }'
    
    consul connect envoy -gateway=terminating -service ext-terminating-gateway
    
  4. Fetch the Envoy config from the terminating gateway and observe that only a single hostname is present in lb_endpoints (since the default discovery type is LOGICAL_DNS).

    curl --request GET --url http://localhost:19000/config_dump
    
  5. Set envoy_dns_discovery_type to "STRICT_DNS" in proxy-defaults.

    curl --request PUT \
      --url http://localhost:8500/v1/config \
      --header 'Content-Type: application/json' \
      --data '{
     "Kind": "proxy-defaults",
     "Name": "global",
     "Config": {
       "protcol": "http",
       "envoy_dns_discovery_type": "STRICT_DNS"
     }
    }'
    
  6. Refetch config and observe that the Envoy config now contains both hostnames in lb_endpoints and will load balance incoming requests across all configured endpoints.

    curl --request GET --url http://localhost:19000/config_dump
    

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@github-actions github-actions bot added the theme/envoy/xds Related to Envoy support label Aug 24, 2024
break

if len(endpoints) == envoyMaxEndpoints {
break
Copy link
Contributor Author

@t-davies t-davies Aug 25, 2024

Choose a reason for hiding this comment

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

Pretty sure we do still need to break early here to maintain the same behaviour that we had before with regards to how fallbacks are determined, but happy to be corrected so we can remove this slightly clunky check.

@t-davies t-davies force-pushed the f-envoy-static-dns-multi-hostnames branch from 5c93213 to b533de4 Compare August 25, 2024 13:44
@t-davies t-davies marked this pull request as ready for review August 25, 2024 13:45
@t-davies t-davies force-pushed the f-envoy-static-dns-multi-hostnames branch 5 times, most recently from feea22c to 63b6ecd Compare September 2, 2024 13:46
@t-davies
Copy link
Contributor Author

t-davies commented Sep 4, 2024

@blake - anyone that can take a look please?

@t-davies t-davies force-pushed the f-envoy-static-dns-multi-hostnames branch 2 times, most recently from 064b6d8 to 8a0e573 Compare September 12, 2024 08:04
@t-davies t-davies force-pushed the f-envoy-static-dns-multi-hostnames branch from 8a0e573 to 592a0ed Compare September 19, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/envoy/xds Related to Envoy support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant