[OSS] Improve Gateway Test Coverage of Catalog Health (#18011)

* fix(cli): remove failing check from 'connect envoy' registration for api gateway

* test(integration): add tests to check catalog statsus of gateways on startup

* remove extra sleep comment

* Update test/integration/consul-container/libs/assert/service.go

* changelog
This commit is contained in:
Dan Stough 2023-07-05 11:30:48 -04:00 committed by GitHub
parent 8af4ad178c
commit b94095d92e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 227 additions and 66 deletions

4
.changelog/18011.txt Normal file
View File

@ -0,0 +1,4 @@
```release-note:bug
connect: Removes the default health check from the `consul connect envoy` command when starting an API Gateway.
This health check would always fail.
```

View File

@ -440,6 +440,18 @@ func (c *cmd) run(args []string) int {
meta = map[string]string{structs.MetaWANFederationKey: "1"}
}
// API gateways do not have a default listener or ready endpoint,
// so adding any check to the registration will fail
var check *api.AgentServiceCheck
if c.gatewayKind != api.ServiceKindAPIGateway {
check = &api.AgentServiceCheck{
Name: fmt.Sprintf("%s listening", c.gatewayKind),
TCP: ipaddr.FormatAddressPort(tcpCheckAddr, lanAddr.Port),
Interval: "10s",
DeregisterCriticalServiceAfter: c.deregAfterCritical,
}
}
svc := api.AgentServiceRegistration{
Kind: c.gatewayKind,
Name: c.gatewaySvcName,
@ -449,12 +461,7 @@ func (c *cmd) run(args []string) int {
Meta: meta,
TaggedAddresses: taggedAddrs,
Proxy: proxyConf,
Check: &api.AgentServiceCheck{
Name: fmt.Sprintf("%s listening", c.gatewayKind),
TCP: ipaddr.FormatAddressPort(tcpCheckAddr, lanAddr.Port),
Interval: "10s",
DeregisterCriticalServiceAfter: c.deregAfterCritical,
},
Check: check,
}
if err := c.client.Agent().ServiceRegister(&svc); err != nil {

View File

@ -13,12 +13,11 @@ import (
"time"
"github.com/hashicorp/go-cleanhttp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/stretchr/testify/assert"
libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
)
@ -54,7 +53,7 @@ func CatalogServiceHasInstanceCount(t *testing.T, c *api.Client, svc string, cou
})
}
// CatalogServiceExists verifies the node name exists in the Consul catalog
// CatalogNodeExists verifies the node name exists in the Consul catalog
func CatalogNodeExists(t *testing.T, c *api.Client, nodeName string) {
retry.Run(t, func(r *retry.R) {
node, _, err := c.Catalog().Node(nodeName, nil)
@ -67,26 +66,55 @@ func CatalogNodeExists(t *testing.T, c *api.Client, nodeName string) {
})
}
func HTTPServiceEchoes(t *testing.T, ip string, port int, path string) {
doHTTPServiceEchoes(t, ip, port, path, nil)
// CatalogServiceIsHealthy verifies the service name exists and all instances pass healthchecks
func CatalogServiceIsHealthy(t *testing.T, c *api.Client, svc string, opts *api.QueryOptions) {
CatalogServiceExists(t, c, svc, opts)
retry.Run(t, func(r *retry.R) {
services, _, err := c.Health().Service(svc, "", false, opts)
if err != nil {
r.Fatal("error reading service health data")
}
if len(services) == 0 {
r.Fatal("did not find catalog entry for ", svc)
}
for _, svc := range services {
for _, check := range svc.Checks {
if check.Status != api.HealthPassing {
r.Fatal("at least one check is not PASSING for service", svc.Service.Service)
}
}
}
})
}
func HTTPServiceEchoes(t *testing.T, ip string, port int, path string) {
doHTTPServiceEchoes(t, ip, port, path, nil, nil)
}
func HTTPServiceEchoesWithHeaders(t *testing.T, ip string, port int, path string, headers map[string]string) {
doHTTPServiceEchoes(t, ip, port, path, headers, nil)
}
func HTTPServiceEchoesWithClient(t *testing.T, client *http.Client, addr string, path string) {
doHTTPServiceEchoesWithClient(t, client, addr, path, nil)
doHTTPServiceEchoesWithClient(t, client, addr, path, nil, nil)
}
func HTTPServiceEchoesResHeader(t *testing.T, ip string, port int, path string, expectedResHeader map[string]string) {
doHTTPServiceEchoes(t, ip, port, path, expectedResHeader)
doHTTPServiceEchoes(t, ip, port, path, nil, expectedResHeader)
}
func HTTPServiceEchoesResHeaderWithClient(t *testing.T, client *http.Client, addr string, path string, expectedResHeader map[string]string) {
doHTTPServiceEchoesWithClient(t, client, addr, path, expectedResHeader)
doHTTPServiceEchoesWithClient(t, client, addr, path, nil, expectedResHeader)
}
// HTTPServiceEchoes verifies that a post to the given ip/port combination returns the data
// in the response body. Optional path can be provided to differentiate requests.
func doHTTPServiceEchoes(t *testing.T, ip string, port int, path string, expectedResHeader map[string]string) {
func doHTTPServiceEchoes(t *testing.T, ip string, port int, path string, requestHeaders map[string]string, expectedResHeader map[string]string) {
client := cleanhttp.DefaultClient()
addr := fmt.Sprintf("%s:%d", ip, port)
doHTTPServiceEchoesWithClient(t, client, addr, path, expectedResHeader)
doHTTPServiceEchoesWithClient(t, client, addr, path, requestHeaders, expectedResHeader)
}
func doHTTPServiceEchoesWithClient(
@ -94,6 +122,7 @@ func doHTTPServiceEchoesWithClient(
client *http.Client,
addr string,
path string,
requestHeaders map[string]string,
expectedResHeader map[string]string,
) {
const phrase = "hello"
@ -110,8 +139,20 @@ func doHTTPServiceEchoesWithClient(
retry.RunWith(failer(), t, func(r *retry.R) {
t.Logf("making call to %s", url)
reader := strings.NewReader(phrase)
res, err := client.Post(url, "text/plain", reader)
req, err := http.NewRequest("POST", url, reader)
require.NoError(t, err, "could not construct request")
for k, v := range requestHeaders {
req.Header.Add(k, v)
if k == "Host" {
req.Host = v
}
}
res, err := client.Do(req)
if err != nil {
r.Fatal("could not make call to service ", url)
}

View File

@ -187,7 +187,11 @@ type ClusterConfig struct {
BuildOpts *libcluster.BuildOptions
Cmd string
LogConsumer *TestLogConsumer
Ports []int
// Exposed Ports are available on the cluster's pause container for the purposes
// of adding external communication to the cluster. An example would be a listener
// on a gateway.
ExposedPorts []int
}
// NewCluster creates a cluster with peering enabled. It also creates
@ -234,8 +238,8 @@ func NewCluster(
serverConf.Cmd = append(serverConf.Cmd, config.Cmd)
}
if config.Ports != nil {
cluster, err = libcluster.New(t, []libcluster.Config{*serverConf}, config.Ports...)
if config.ExposedPorts != nil {
cluster, err = libcluster.New(t, []libcluster.Config{*serverConf}, config.ExposedPorts...)
} else {
cluster, err = libcluster.NewN(t, *serverConf, config.NumServers)
}

View File

@ -7,11 +7,8 @@ import (
"fmt"
"testing"
"github.com/stretchr/testify/require"
libassert "github.com/hashicorp/consul/test/integration/consul-container/libs/assert"
libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
"github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
)
@ -40,7 +37,7 @@ func TestBasicConnectService(t *testing.T) {
},
})
clientService := createServices(t, cluster)
_, clientService := topology.CreateServices(t, cluster)
_, port := clientService.GetAddr()
_, adminPort := clientService.GetAdminAddr()
@ -51,30 +48,3 @@ func TestBasicConnectService(t *testing.T) {
libassert.HTTPServiceEchoes(t, "localhost", port, "")
libassert.AssertFortioName(t, fmt.Sprintf("http://localhost:%d", port), "static-server", "")
}
func createServices(t *testing.T, cluster *libcluster.Cluster) libservice.Service {
node := cluster.Agents[0]
client := node.GetClient()
// Create a service and proxy instance
serviceOpts := &libservice.ServiceOpts{
Name: libservice.StaticServerServiceName,
ID: "static-server",
HTTPPort: 8080,
GRPCPort: 8079,
}
// Create a service and proxy instance
_, _, err := libservice.CreateAndRegisterStaticServerAndSidecar(node, serviceOpts)
require.NoError(t, err)
libassert.CatalogServiceExists(t, client, "static-server-sidecar-proxy", nil)
libassert.CatalogServiceExists(t, client, libservice.StaticServerServiceName, nil)
// Create a client proxy instance with the server as an upstream
clientConnectProxy, err := libservice.CreateAndRegisterStaticClientSidecar(node, "", false, false)
require.NoError(t, err)
libassert.CatalogServiceExists(t, client, "static-client-sidecar-proxy", nil)
return clientConnectProxy
}

View File

@ -12,11 +12,10 @@ import (
"testing"
"time"
"github.com/hashicorp/go-cleanhttp"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/go-cleanhttp"
libassert "github.com/hashicorp/consul/test/integration/consul-container/libs/assert"
libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
@ -47,7 +46,7 @@ func TestAPIGatewayCreate(t *testing.T) {
InjectGossipEncryption: true,
AllowHTTPAnyway: true,
},
Ports: []int{
ExposedPorts: []int{
listenerPortOne,
serviceHTTPPort,
serviceGRPCPort,
@ -59,6 +58,21 @@ func TestAPIGatewayCreate(t *testing.T) {
namespace := getOrCreateNamespace(t, client)
// Create a gateway
// We intentionally do this before creating the config entries
gatewayService, err := libservice.NewGatewayService(context.Background(), libservice.GatewayConfig{
Kind: "api",
Namespace: namespace,
Name: gatewayName,
}, cluster.Agents[0], listenerPortOne)
require.NoError(t, err)
// We check this is healthy here because in the case of bringing up a new kube cluster,
// it is not possible to create the config entry in advance.
// The health checks must pass so the pod can start up.
// For API gateways, this should always pass, because there is no default listener for health in Envoy
libassert.CatalogServiceIsHealthy(t, client, gatewayName, &api.QueryOptions{Namespace: namespace})
// add api gateway config
apiGateway := &api.APIGatewayConfigEntry{
Kind: api.APIGateway,
@ -75,7 +89,7 @@ func TestAPIGatewayCreate(t *testing.T) {
require.NoError(t, cluster.ConfigEntryWrite(apiGateway))
_, _, err := libservice.CreateAndRegisterStaticServerAndSidecar(cluster.Agents[0], &libservice.ServiceOpts{
_, _, err = libservice.CreateAndRegisterStaticServerAndSidecar(cluster.Agents[0], &libservice.ServiceOpts{
ID: serviceName,
Name: serviceName,
Namespace: namespace,
@ -105,14 +119,6 @@ func TestAPIGatewayCreate(t *testing.T) {
require.NoError(t, cluster.ConfigEntryWrite(tcpRoute))
// Create a gateway
gatewayService, err := libservice.NewGatewayService(context.Background(), libservice.GatewayConfig{
Kind: "api",
Namespace: namespace,
Name: gatewayName,
}, cluster.Agents[0], listenerPortOne)
require.NoError(t, err)
// make sure the gateway/route come online
// make sure config entries have been properly created
checkGatewayConfigEntry(t, client, gatewayName, &api.QueryOptions{Namespace: namespace})

View File

@ -70,7 +70,7 @@ func TestHTTPRouteFlattening(t *testing.T) {
InjectGossipEncryption: true,
AllowHTTPAnyway: true,
},
Ports: []int{
ExposedPorts: []int{
listenerPort,
serviceOneHTTPPort,
serviceOneGRPCPort,
@ -298,7 +298,7 @@ func TestHTTPRoutePathRewrite(t *testing.T) {
InjectGossipEncryption: true,
AllowHTTPAnyway: true,
},
Ports: []int{
ExposedPorts: []int{
listenerPort,
fooHTTPPort,
fooGRPCPort,
@ -525,7 +525,7 @@ func TestHTTPRouteParentRefChange(t *testing.T) {
InjectGossipEncryption: true,
AllowHTTPAnyway: true,
},
Ports: []int{
ExposedPorts: []int{
listenerOnePort,
listenerTwoPort,
serviceHTTPPort,

View File

@ -0,0 +1,129 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0
package gateways
import (
"context"
"fmt"
"testing"
"time"
"github.com/docker/go-connections/nat"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/api"
libassert "github.com/hashicorp/consul/test/integration/consul-container/libs/assert"
libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
"github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
)
// TestIngressGateway Summary
// This test makes sure a cluster service can be reached via and ingress gateway.
//
// Steps:
// - Create a cluster (1 server and 1 client).
// - Create the example static-server and sidecar containers, then register them both with Consul
// - Create an ingress gateway and register it with Consul on the client agent
// - Create a config entry that binds static-server to a new listener on the ingress gateway
// - Verify that static-service is accessible through the ingress gateway port
func TestIngressGateway(t *testing.T) {
t.Parallel()
// Ingress gateways must have a listener other than 8443, which is used for health checks.
// 9999 is already exposed from consul agents
gatewayListenerPort := 9999
cluster, _, _ := topology.NewCluster(t, &topology.ClusterConfig{
NumServers: 1,
NumClients: 1,
ApplyDefaultProxySettings: true,
BuildOpts: &libcluster.BuildOptions{
Datacenter: "dc1",
InjectAutoEncryption: true,
InjectGossipEncryption: true,
// TODO(rb): fix the test to not need the service/envoy stack to use :8500
AllowHTTPAnyway: true,
},
})
apiClient := cluster.APIClient(0)
clientNode := cluster.Clients()[0]
// Set up the "static-server" backend
serverService, _ := topology.CreateServices(t, cluster)
// Create the ingress gateway service
// We expose this on the client node, which already has port 9999 exposed as part of it's pause "pod"
gwCfg := libservice.GatewayConfig{
Name: api.IngressGateway,
Kind: "ingress",
}
ingressService, err := libservice.NewGatewayService(context.Background(), gwCfg, clientNode)
require.NoError(t, err)
// this is deliberate
// internally, ingress gw have a 15s timeout before the /ready endpoint is available,
// then we need to wait for the health check to re-execute and propagate.
time.Sleep(45 * time.Second)
// We check this is healthy here because in the case of bringing up a new kube cluster,
// it is not possible to create the config entry in advance.
// The health checks must pass so the pod can start up.
libassert.CatalogServiceIsHealthy(t, apiClient, api.IngressGateway, nil)
// Register a service to the ingress gateway
// **NOTE**: We intentionally wait until after the gateway starts to create the config entry.
// This was a regression that can cause errors when starting up consul-k8s before you have the resource defined.
ingressGwConfig := &api.IngressGatewayConfigEntry{
Kind: api.IngressGateway,
Name: api.IngressGateway,
Listeners: []api.IngressListener{
{
Port: gatewayListenerPort,
Protocol: "http",
Services: []api.IngressService{
{
Name: libservice.StaticServerServiceName,
},
},
},
},
}
require.NoError(t, cluster.ConfigEntryWrite(ingressGwConfig))
// Wait for the request to persist
checkIngressConfigEntry(t, apiClient, api.IngressGateway, nil)
_, adminPort := ingressService.GetAdminAddr()
libassert.AssertUpstreamEndpointStatus(t, adminPort, "static-server.default", "HEALTHY", 1)
//libassert.GetEnvoyListenerTCPFilters(t, adminPort) // This won't succeed because the dynamic listener is delayed
libassert.AssertContainerState(t, ingressService, "running")
libassert.AssertContainerState(t, serverService, "running")
mappedPort, err := clientNode.GetPod().MappedPort(context.Background(), nat.Port(fmt.Sprintf("%d/tcp", gatewayListenerPort)))
require.NoError(t, err)
// by default, ingress routes are set per <service>.ingress.*
headers := map[string]string{"Host": fmt.Sprintf("%s.ingress.com", libservice.StaticServerServiceName)}
libassert.HTTPServiceEchoesWithHeaders(t, "localhost", mappedPort.Int(), "", headers)
}
func checkIngressConfigEntry(t *testing.T, client *api.Client, gatewayName string, opts *api.QueryOptions) {
t.Helper()
require.Eventually(t, func() bool {
entry, _, err := client.ConfigEntries().Get(api.IngressGateway, gatewayName, opts)
if err != nil {
t.Log("error constructing request", err)
return false
}
if entry == nil {
t.Log("returned entry is nil")
return false
}
return true
}, time.Second*10, time.Second*1)
}