luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

xds: remove HTTPCheckFetcher dependency (#13366) 

This is the OSS portion of enterprise PR 1994

Rather than directly interrogating the agent-local state for HTTP
checks using the `HTTPCheckFetcher` interface, we now rely on the
config snapshot containing the checks.

This reduces the number of changes required to support server xDS
sessions.

It's not clear why the fetching approach was introduced in
931d167ebb2300839b218d08871f22323c60175d.
This commit is contained in:
Dan Upton 2022-06-06 15:15:33 +01:00 committed by GitHub
parent 4c781d1e15
commit 5cd31933d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 39 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
agent/agent.go
View File

@ -824,7 +824,6 @@ func (a *Agent) listenAndServeGRPC() error {
return a.delegate.ResolveTokenAndDefaultMeta(id, nil, nil)
},
a,
a,
)
a.xdsServer.Register(a.publicGRPCServer)

30
agent/proxycfg/testing_connect_proxy.go
View File

@ -1,12 +1,15 @@
package proxycfg
import (
"time"
"github.com/mitchellh/go-testing-interface"
"github.com/stretchr/testify/assert"
"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/agent/consul/discoverychain"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/types"
)
// TestConfigSnapshot returns a fully populated snapshot
@ -222,6 +225,33 @@ func TestConfigSnapshotExposeConfig(t testing.T, nsFn func(ns *structs.NodeServi
}, nsFn, nil, baseEvents)
}
func TestConfigSnapshotExposeChecks(t testing.T) *ConfigSnapshot {
return TestConfigSnapshot(t,
func(ns *structs.NodeService) {
ns.Address = "1.2.3.4"
ns.Port = 8080
ns.Proxy.Upstreams = nil
ns.Proxy.Expose = structs.ExposeConfig{
Checks: true,
}
},
[]UpdateEvent{
{
CorrelationID: svcChecksWatchIDPrefix + structs.ServiceIDString("web", nil),
Result: []structs.CheckType{{
CheckID: types.CheckID("http"),
Name: "http",
HTTP: "http://127.0.0.1:8181/debug",
ProxyHTTP: "http://:21500/debug",
Method: "GET",
Interval: 10 * time.Second,
Timeout: 1 * time.Second,
}},
},
},
)
}
func TestConfigSnapshotGRPCExposeHTTP1(t testing.T) *ConfigSnapshot {
roots, leaf := TestCerts(t)

2
agent/xds/clusters.go
View File

@ -146,7 +146,7 @@ func (s *ResourceGenerator) clustersFromSnapshotConnectProxy(cfgSnap *proxycfg.C
// Add service health checks to the list of paths to create clusters for if needed
if cfgSnap.Proxy.Expose.Checks {
psid := structs.NewServiceID(cfgSnap.Proxy.DestinationServiceID, &cfgSnap.ProxyID.EnterpriseMeta)
for _, check := range s.CheckFetcher.ServiceHTTPBasedChecks(psid) {
for _, check := range cfgSnap.ConnectProxy.WatchedServiceChecks[psid] {
p, err := parseCheckPath(check)
if err != nil {
s.Logger.Warn("failed to create cluster for", "check", check.CheckID, "error", err)

2
agent/xds/clusters_test.go
View File

@ -638,7 +638,7 @@ func TestClustersFromSnapshot(t *testing.T) {
setupTLSRootsAndLeaf(t, snap)
// Need server just for logger dependency
g := newResourceGenerator(testutil.Logger(t), nil, nil, false)
g := newResourceGenerator(testutil.Logger(t), nil, false)
g.ProxyFeatures = sf
clusters, err := g.clustersFromSnapshot(snap)

1
agent/xds/delta.go
View File

@ -106,7 +106,6 @@ func (s *Server) processDelta(stream ADSDeltaStream, reqCh <-chan *envoy_discove
generator := newResourceGenerator(
s.Logger.Named(logging.XDS).With("xdsVersion", "v3"),
s.CheckFetcher,
s.CfgFetcher,
true,
)

2
agent/xds/endpoints_test.go
View File

@ -504,7 +504,7 @@ func TestEndpointsFromSnapshot(t *testing.T) {
setupTLSRootsAndLeaf(t, snap)
// Need server just for logger dependency
g := newResourceGenerator(testutil.Logger(t), nil, nil, false)
g := newResourceGenerator(testutil.Logger(t), nil, false)
g.ProxyFeatures = sf
endpoints, err := g.endpointsFromSnapshot(snap)

2
agent/xds/listeners.go
View File

@ -398,7 +398,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg.
// Add service health checks to the list of paths to create listeners for if needed
if cfgSnap.Proxy.Expose.Checks {
psid := structs.NewServiceID(cfgSnap.Proxy.DestinationServiceID, &cfgSnap.ProxyID.EnterpriseMeta)
for _, check := range s.CheckFetcher.ServiceHTTPBasedChecks(psid) {
for _, check := range cfgSnap.ConnectProxy.WatchedServiceChecks[psid] {
p, err := parseCheckPath(check)
if err != nil {
s.Logger.Warn("failed to create listener for", "check", check.CheckID, "error", err)

36
agent/xds/listeners_test.go
View File

@ -6,7 +6,6 @@ import (
"sort"
"testing"
"text/template"
"time"
envoy_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
testinf "github.com/mitchellh/go-testing-interface"
@ -451,33 +450,12 @@ func TestListenersFromSnapshot(t *testing.T) {
// NOTE: if IPv6 is not supported in the kernel per
// kernelSupportsIPv6() then this test will fail because the golden
// files were generated assuming ipv6 support was present
name: "expose-checks",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotExposeConfig(t, func(ns *structs.NodeService) {
ns.Proxy.Expose = structs.ExposeConfig{
Checks: true,
}
})
},
name: "expose-checks",
create: proxycfg.TestConfigSnapshotExposeChecks,
generatorSetup: func(s *ResourceGenerator) {
s.CfgFetcher = configFetcherFunc(func() string {
return "192.0.2.1"
})
s.CheckFetcher = httpCheckFetcherFunc(func(sid structs.ServiceID) []structs.CheckType {
if sid != structs.NewServiceID("web", nil) {
return nil
}
return []structs.CheckType{{
CheckID: types.CheckID("http"),
Name: "http",
HTTP: "http://127.0.0.1:8181/debug",
ProxyHTTP: "http://:21500/debug",
Method: "GET",
Interval: 10 * time.Second,
Timeout: 1 * time.Second,
}}
})
},
},
{
@ -838,7 +816,7 @@ func TestListenersFromSnapshot(t *testing.T) {
}
// Need server just for logger dependency
g := newResourceGenerator(testutil.Logger(t), nil, nil, false)
g := newResourceGenerator(testutil.Logger(t), nil, false)
g.ProxyFeatures = sf
if tt.generatorSetup != nil {
tt.generatorSetup(g)
@ -988,14 +966,6 @@ func customHTTPListenerJSON(t testinf.T, opts customHTTPListenerJSONOptions) str
return buf.String()
}
type httpCheckFetcherFunc func(serviceID structs.ServiceID) []structs.CheckType
var _ HTTPCheckFetcher = (httpCheckFetcherFunc)(nil)
func (f httpCheckFetcherFunc) ServiceHTTPBasedChecks(serviceID structs.ServiceID) []structs.CheckType {
return f(serviceID)
}
type configFetcherFunc func() string
var _ ConfigFetcher = (configFetcherFunc)(nil)

3
agent/xds/resources.go
View File

@ -14,7 +14,6 @@ import (
// resources for a single client.
type ResourceGenerator struct {
Logger hclog.Logger
CheckFetcher HTTPCheckFetcher
CfgFetcher ConfigFetcher
IncrementalXDS bool
@ -23,13 +22,11 @@ type ResourceGenerator struct {
func newResourceGenerator(
logger hclog.Logger,
checkFetcher HTTPCheckFetcher,
cfgFetcher ConfigFetcher,
incrementalXDS bool,
) *ResourceGenerator {
return &ResourceGenerator{
Logger: logger,
CheckFetcher: checkFetcher,
CfgFetcher: cfgFetcher,
IncrementalXDS: incrementalXDS,
}

2
agent/xds/routes_test.go
View File

@ -197,7 +197,7 @@ func TestRoutesFromSnapshot(t *testing.T) {
// golden files for every test case and so not be any use!
setupTLSRootsAndLeaf(t, snap)
g := newResourceGenerator(testutil.Logger(t), nil, nil, false)
g := newResourceGenerator(testutil.Logger(t), nil, false)
g.ProxyFeatures = sf
routes, err := g.routesFromSnapshot(snap)

9
agent/xds/server.go
View File

@ -85,12 +85,6 @@ const (
// coupling this to the agent.
type ACLResolverFunc func(id string) (acl.Authorizer, error)
// HTTPCheckFetcher is the interface the agent needs to expose
// for the xDS server to fetch a service's HTTP check definitions
type HTTPCheckFetcher interface {
ServiceHTTPBasedChecks(serviceID structs.ServiceID) []structs.CheckType
}
// ConfigFetcher is the interface the agent needs to expose
// for the xDS server to fetch agent config, currently only one field is fetched
type ConfigFetcher interface {
@ -113,7 +107,6 @@ type Server struct {
Logger hclog.Logger
CfgSrc ProxyConfigSource
ResolveToken ACLResolverFunc
CheckFetcher HTTPCheckFetcher
CfgFetcher ConfigFetcher
// AuthCheckFrequency is how often we should re-check the credentials used
@ -165,7 +158,6 @@ func NewServer(
serverlessPluginEnabled bool,
cfgMgr ProxyConfigSource,
resolveToken ACLResolverFunc,
checkFetcher HTTPCheckFetcher,
cfgFetcher ConfigFetcher,
) *Server {
return &Server{
@ -173,7 +165,6 @@ func NewServer(
Logger: logger,
CfgSrc: cfgMgr,
ResolveToken: resolveToken,
CheckFetcher: checkFetcher,
CfgFetcher: cfgFetcher,
AuthCheckFrequency: DefaultAuthCheckFrequency,
activeStreams: &activeStreamCounters{},

2
agent/xds/serverless_plugin_oss_test.go
View File

@ -73,7 +73,7 @@ func TestServerlessPluginFromSnapshot(t *testing.T) {
// golden files for every test case and so not be any use!
setupTLSRootsAndLeaf(t, snap)
g := newResourceGenerator(testutil.Logger(t), nil, nil, false)
g := newResourceGenerator(testutil.Logger(t), nil, false)
g.ProxyFeatures = sf
res, err := g.allResourcesFromSnapshot(snap)

1
agent/xds/xds_protocol_helpers_test.go
View File

@ -160,7 +160,6 @@ func newTestServerDeltaScenario(
serverlessPluginEnabled,
mgr,
resolveToken,
nil, /*checkFetcher HTTPCheckFetcher*/
nil, /*cfgFetcher ConfigFetcher*/
)
if authCheckFrequency > 0 {