drop Visitor pattern for getting info out of DAG (#4099)

Replaces the use of the visitor pattern
for getting data out of the DAG with
"accessor" functions that know about
the structure of the DAG and can fetch
information efficiently.

Closes #4072.

Signed-off-by: Steve Kriss <krisss@vmware.com>

changelogs/unreleased/4099-skriss-minor.md

@@ -0,0 +1,4 @@
+### Performance improvement for processing configuration
+
+The performance of Contour's configuration processing has been made more efficient, particularly for clusters with large numbers (i.e. >1k) of HTTPProxies and/or Ingresses.
+This means that there should be less of a delay between creating/updating an HTTPProxy/Ingress in Kubernetes, and having it reflected in Envoy's configuration.

internal/dag/accessors.go

@@ -18,6 +18,7 @@ import (
 	"strconv"
 
 	"github.com/projectcontour/contour/internal/annotation"
+	"github.com/projectcontour/contour/internal/xds"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -26,7 +27,7 @@ import (
 // EnsureService looks for a Kubernetes service in the cache matching the provided
 // namespace, name and port, and returns a DAG service for it. If a matching service
 // cannot be found in the cache, an error is returned.
-func (dag *DAG) EnsureService(meta types.NamespacedName, port intstr.IntOrString, cache *KubernetesCache, enableExternalNameSvc bool) (*Service, error) {
+func (d *DAG) EnsureService(meta types.NamespacedName, port intstr.IntOrString, cache *KubernetesCache, enableExternalNameSvc bool) (*Service, error) {
 	svc, svcPort, err := cache.LookupService(meta, port)
 	if err != nil {
 		return nil, err
@@ -108,116 +109,193 @@ func externalName(svc *v1.Service) string {
 	return svc.Spec.ExternalName
 }
 
-// GetSecureVirtualHosts returns all secure virtual hosts in the DAG.
-func (dag *DAG) GetSecureVirtualHosts() map[ListenerName]*SecureVirtualHost {
-	getter := svhostGetter(map[ListenerName]*SecureVirtualHost{})
-	dag.Visit(getter.visit)
-	return getter
-}
-
 // GetSecureVirtualHost returns the secure virtual host in the DAG that
 // matches the provided name, or nil if no matching secure virtual host
 // is found.
-func (dag *DAG) GetSecureVirtualHost(ln ListenerName) *SecureVirtualHost {
-	return dag.GetSecureVirtualHosts()[ln]
+func (d *DAG) GetSecureVirtualHost(hostname string) *SecureVirtualHost {
+	return d.SecureVirtualHosts[hostname]
 }
 
 // EnsureSecureVirtualHost adds a secure virtual host with the provided
 // name to the DAG if it does not already exist, and returns it.
-func (dag *DAG) EnsureSecureVirtualHost(ln ListenerName) *SecureVirtualHost {
-	if svh := dag.GetSecureVirtualHost(ln); svh != nil {
+func (d *DAG) EnsureSecureVirtualHost(hostname string) *SecureVirtualHost {
+	if svh := d.GetSecureVirtualHost(hostname); svh != nil {
 		return svh
 	}
 
 	svh := &SecureVirtualHost{
 		VirtualHost: VirtualHost{
-			Name:         ln.Name,
-			ListenerName: ln.ListenerName,
+			Name:         hostname,
+			ListenerName: "ingress_https",
 		},
 	}
-	dag.AddRoot(svh)
+	d.SecureVirtualHosts[hostname] = svh
 	return svh
 }
 
-// svhostGetter is a visitor that gets all secure virtual hosts
-// in the DAG.
-type svhostGetter map[ListenerName]*SecureVirtualHost
-
-func (s svhostGetter) visit(vertex Vertex) {
-	switch obj := vertex.(type) {
-	case *SecureVirtualHost:
-		s[ListenerName{Name: obj.Name, ListenerName: obj.VirtualHost.ListenerName}] = obj
-	default:
-		vertex.Visit(s.visit)
-	}
-}
-
-// GetVirtualHosts returns all virtual hosts in the DAG.
-func (dag *DAG) GetVirtualHosts() map[ListenerName]*VirtualHost {
-	getter := vhostGetter(map[ListenerName]*VirtualHost{})
-	dag.Visit(getter.visit)
-	return getter
-}
-
 // GetVirtualHost returns the virtual host in the DAG that matches the
 // provided name, or nil if no matching virtual host is found.
-func (dag *DAG) GetVirtualHost(ln ListenerName) *VirtualHost {
-	return dag.GetVirtualHosts()[ln]
+func (d *DAG) GetVirtualHost(hostname string) *VirtualHost {
+	return d.VirtualHosts[hostname]
 }
 
 // EnsureVirtualHost adds a virtual host with the provided name to the
 // DAG if it does not already exist, and returns it.
-func (dag *DAG) EnsureVirtualHost(ln ListenerName) *VirtualHost {
-	if vhost := dag.GetVirtualHost(ln); vhost != nil {
+func (d *DAG) EnsureVirtualHost(hostname string) *VirtualHost {
+	if vhost := d.GetVirtualHost(hostname); vhost != nil {
 		return vhost
 	}
 
 	vhost := &VirtualHost{
-		Name:         ln.Name,
-		ListenerName: ln.ListenerName,
+		Name:         hostname,
+		ListenerName: "ingress_http",
 	}
-	dag.AddRoot(vhost)
+	d.VirtualHosts[hostname] = vhost
 	return vhost
 }
 
-// vhostGetter is a visitor that gets all virtual hosts
-// in the DAG.
-type vhostGetter map[ListenerName]*VirtualHost
+func (d *DAG) GetClusters() []*Cluster {
+	var res []*Cluster
+
+	for _, listener := range d.Listeners {
+		for _, vhost := range listener.VirtualHosts {
+			for _, route := range vhost.Routes {
+				res = append(res, route.Clusters...)
+
+				if route.MirrorPolicy != nil && route.MirrorPolicy.Cluster != nil {
+					res = append(res, route.MirrorPolicy.Cluster)
+				}
+			}
+		}
+
+		for _, vhost := range listener.SecureVirtualHosts {
+			for _, route := range vhost.Routes {
+				res = append(res, route.Clusters...)
+
+				if route.MirrorPolicy != nil && route.MirrorPolicy.Cluster != nil {
+					res = append(res, route.MirrorPolicy.Cluster)
+				}
+			}
+
+			if vhost.TCPProxy != nil {
+				res = append(res, vhost.TCPProxy.Clusters...)
+			}
+		}
+	}
+
+	return res
+}
 
-func (v vhostGetter) visit(vertex Vertex) {
-	switch obj := vertex.(type) {
-	case *VirtualHost:
-		v[ListenerName{Name: obj.Name, ListenerName: obj.ListenerName}] = obj
-	default:
-		vertex.Visit(v.visit)
+func (d *DAG) GetServiceClusters() []*ServiceCluster {
+	var res []*ServiceCluster
+
+	for _, cluster := range d.GetClusters() {
+		// A Service has only one WeightedService entry. Fake up a
+		// ServiceCluster so that the visitor can pretend to not
+		// know this.
+		c := &ServiceCluster{
+			ClusterName: xds.ClusterLoadAssignmentName(
+				types.NamespacedName{
+					Name:      cluster.Upstream.Weighted.ServiceName,
+					Namespace: cluster.Upstream.Weighted.ServiceNamespace,
+				},
+				cluster.Upstream.Weighted.ServicePort.Name),
+			Services: []WeightedService{
+				cluster.Upstream.Weighted,
+			},
+		}
+
+		res = append(res, c)
 	}
+
+	for _, ec := range d.ExtensionClusters {
+		res = append(res, &ec.Upstream)
+	}
+
+	return res
 }
 
 // GetExtensionClusters returns all extension clusters in the DAG.
-func (dag *DAG) GetExtensionClusters() map[string]*ExtensionCluster {
-	getter := extensionClusterGetter(map[string]*ExtensionCluster{})
-	dag.Visit(getter.visit)
-	return getter
+func (d *DAG) GetExtensionClusters() map[string]*ExtensionCluster {
+	// TODO for DAG consumers, this should iterate
+	// over Listeners.SecureVirtualHosts and get
+	// AuthorizationServices.
+	res := map[string]*ExtensionCluster{}
+	for _, ec := range d.ExtensionClusters {
+		res[ec.Name] = ec
+	}
+	return res
+}
+
+func (d *DAG) GetSecrets() []*Secret {
+	var res []*Secret
+	for _, l := range d.Listeners {
+		for _, svh := range l.SecureVirtualHosts {
+			if svh.Secret != nil {
+				res = append(res, svh.Secret)
+			}
+			if svh.FallbackCertificate != nil {
+				res = append(res, svh.FallbackCertificate)
+			}
+		}
+	}
+
+	for _, c := range d.GetClusters() {
+		if c.ClientCertificate != nil {
+			res = append(res, c.ClientCertificate)
+		}
+	}
+
+	return res
 }
 
 // GetExtensionCluster returns the extension cluster in the DAG that
 // matches the provided name, or nil if no matching extension cluster
 // is found.
-func (dag *DAG) GetExtensionCluster(name string) *ExtensionCluster {
-	return dag.GetExtensionClusters()[name]
+func (d *DAG) GetExtensionCluster(name string) *ExtensionCluster {
+	for _, ec := range d.ExtensionClusters {
+		if ec.Name == name {
+			return ec
+		}
+	}
+
+	return nil
 }
 
-// extensionClusterGetter is a visitor that gets all extension clusters
-// in the DAG.
-type extensionClusterGetter map[string]*ExtensionCluster
+func (d *DAG) GetVirtualHostRoutes() map[*VirtualHost][]*Route {
+	res := map[*VirtualHost][]*Route{}
+
+	for _, l := range d.Listeners {
+		for _, vhost := range l.VirtualHosts {
+			var routes []*Route
+			for _, r := range vhost.Routes {
+				routes = append(routes, r)
+			}
+			if len(routes) > 0 {
+				res[vhost] = routes
+			}
+		}
+	}
 
-func (v extensionClusterGetter) visit(vertex Vertex) {
-	switch obj := vertex.(type) {
-	case *ExtensionCluster:
-		v[obj.Name] = obj
-	default:
-		vertex.Visit(v.visit)
+	return res
+}
+
+func (d *DAG) GetSecureVirtualHostRoutes() map[*SecureVirtualHost][]*Route {
+	res := map[*SecureVirtualHost][]*Route{}
+
+	for _, l := range d.Listeners {
+		for _, vhost := range l.SecureVirtualHosts {
+			var routes []*Route
+			for _, r := range vhost.Routes {
+				routes = append(routes, r)
+			}
+			if len(routes) > 0 {
+				res[vhost] = routes
+			}
+		}
 	}
+
+	return res
 }
 
 // validSecret returns true if the Secret contains certificate and private key material.

internal/dag/builder.go

@@ -59,12 +59,14 @@ func (b *Builder) Build() *DAG {
 		gatewayController = b.Source.gatewayclass.Spec.ControllerName
 	}
 
-	dag := DAG{
-		StatusCache: status.NewCache(gatewayNSName, gatewayController),
+	dag := &DAG{
+		VirtualHosts:       map[string]*VirtualHost{},
+		SecureVirtualHosts: map[string]*SecureVirtualHost{},
+		StatusCache:        status.NewCache(gatewayNSName, gatewayController),
 	}
 
 	for _, p := range b.Processors {
-		p.Run(&dag, &b.Source)
+		p.Run(dag, &b.Source)
 	}
-	return &dag
+	return dag
 }