Add a new load-balancer-status flag for setting ingress details

Signed-off-by: Haitao Li <hli@atlassian.com>

apis/projectcontour/v1alpha1/contourconfig.go

@@ -253,11 +253,15 @@ type EnvoyConfig struct {
 	// +optional
 	Service *NamespacedName `json:"service,omitempty"`
 
-	// Ingress holds Envoy service parameters for setting Ingress status.
+	// LoadBalancer specifies how Contour should set the ingress status address.
+	// If provided, the value can be in one of the formats:
+	//   - hostname:<address,...>: Contour will use the provided comma separated list of addresses directly.
+	//   - service:<namespace>/<name>: Contour will use the address of the designated service.
+	//   - ingress:<namespace>/<name>: Contour will use the address of the designated ingress.
 	//
-	// Contour's default is { namespace: "projectcontour", name: "envoy" }.
+	// Contour's default is an empty string.
 	// +optional
-	Ingress *NamespacedName `json:"ingress,omitempty"`
+	LoadBalancer string `json:"loadBalancer,omitempty"`
 
 	// Defines the HTTP Listener for Envoy.
 	//

cmd/contour/serve.go

@@ -20,6 +20,7 @@ import (
 	"net/http"
 	"os"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/alecthomas/kingpin/v2"
@@ -145,8 +146,6 @@ func registerServe(app *kingpin.Application) (*kingpin.CmdClause, *serveContext)
 	serve.Flag("envoy-service-https-port", "Kubernetes Service port for HTTPS requests.").PlaceHolder("<port>").IntVar(&ctx.httpsPort)
 	serve.Flag("envoy-service-name", "Name of the Envoy service to inspect for Ingress status details.").PlaceHolder("<name>").StringVar(&ctx.Config.EnvoyServiceName)
 	serve.Flag("envoy-service-namespace", "Envoy Service Namespace.").PlaceHolder("<namespace>").StringVar(&ctx.Config.EnvoyServiceNamespace)
-	serve.Flag("envoy-ingress-name", "Name of the Envoy ingress to inspect for Ingress status details.").PlaceHolder("<name>").StringVar(&ctx.Config.EnvoyIngressName)
-	serve.Flag("envoy-ingress-namespace", "Envoy Ingress Namespace.").PlaceHolder("<namespace>").StringVar(&ctx.Config.EnvoyIngressNamespace)
 
 	serve.Flag("health-address", "Address the health HTTP endpoint will bind to.").PlaceHolder("<ipaddr>").StringVar(&ctx.healthAddr)
 	serve.Flag("health-port", "Port the health HTTP endpoint will bind to.").PlaceHolder("<port>").IntVar(&ctx.healthPort)
@@ -169,6 +168,8 @@ func registerServe(app *kingpin.Application) (*kingpin.CmdClause, *serveContext)
 	serve.Flag("leader-election-resource-namespace", "The namespace of the resource (Lease) leader election will lease.").Default(config.GetenvOr("CONTOUR_NAMESPACE", "projectcontour")).StringVar(&ctx.LeaderElection.Namespace)
 	serve.Flag("leader-election-retry-period", "The interval which Contour will attempt to acquire leadership lease.").Default("2s").DurationVar(&ctx.LeaderElection.RetryPeriod)
 
+	serve.Flag("load-balancer-status", "Address to set or the source to inspect for ingress status.").PlaceHolder("<kind:namespace/name|address>").StringVar(&ctx.Config.LoadBalancerStatus)
+
 	serve.Flag("root-namespaces", "Restrict contour to searching these namespaces for root ingress routes.").PlaceHolder("<ns,ns>").StringVar(&ctx.rootNamespaces)
 
 	serve.Flag("stats-address", "Envoy /stats interface address.").PlaceHolder("<ipaddr>").StringVar(&ctx.statsAddr)
@@ -673,87 +674,162 @@ func (s *Server) doServe() error {
 	}
 
 	// Set up ingress load balancer status writer.
+	if err := s.setupIngressLoadBalancerStatusWriter(contourConfiguration, ingressClassNames, gatewayControllerName, gatewayRef, sh.Writer()); err != nil {
+		return err
+	}
+
+	xdsServer := &xdsServer{
+		log:             s.log,
+		registry:        s.registry,
+		config:          *contourConfiguration.XDSServer,
+		snapshotHandler: snapshotHandler,
+		resources:       resources,
+		initialDagBuilt: contourHandler.HasBuiltInitialDag,
+	}
+	if err := s.mgr.Add(xdsServer); err != nil {
+		return err
+	}
+
+	notifier := &leadership.Notifier{
+		ToNotify: append([]leadership.NeedLeaderElectionNotification{
+			contourHandler,
+			observer,
+		}, needsNotification...),
+	}
+	if err := s.mgr.Add(notifier); err != nil {
+		return err
+	}
+
+	// GO!
+	return s.mgr.Start(signals.SetupSignalHandler())
+}
+
+func (s *Server) setupIngressLoadBalancerStatusWriter(
+	contourConfiguration contour_api_v1alpha1.ContourConfigurationSpec,
+	ingressClassNames []string,
+	gatewayControllerName string,
+	gatewayRef *types.NamespacedName,
+	statusUpdater k8s.StatusUpdater) error {
 	lbsw := &loadBalancerStatusWriter{
 		log:                   s.log.WithField("context", "loadBalancerStatusWriter"),
 		cache:                 s.mgr.GetCache(),
 		lbStatus:              make(chan corev1.LoadBalancerStatus, 1),
 		ingressClassNames:     ingressClassNames,
 		gatewayControllerName: gatewayControllerName,
 		gatewayRef:            gatewayRef,
-		statusUpdater:         sh.Writer(),
+		statusUpdater:         statusUpdater,
 	}
 	if err := s.mgr.Add(lbsw); err != nil {
 		return err
 	}
 
-	// Register an informer to watch envoy's service if we haven't been given static details.
+	elbs := &envoyLoadBalancerStatus{}
 	if lbAddress := contourConfiguration.Ingress.StatusAddress; len(lbAddress) > 0 {
-		s.log.WithField("loadbalancer-address", lbAddress).Info("Using supplied information for Ingress status")
-		lbsw.lbStatus <- parseStatusFlag(lbAddress)
+		elbs.Kind = "hostname"
+		elbs.FQDNs = lbAddress
+	} else if contourConfiguration.Envoy.LoadBalancer != "" {
+		status, err := parseEnvoyLoadBalancerStatus(contourConfiguration.Envoy.LoadBalancer)
+		if err != nil {
+			return err
+		}
+		elbs = status
 	} else {
+		elbs.Kind = "service"
+		elbs.Namespace = contourConfiguration.Envoy.Service.Namespace
+		elbs.Name = contourConfiguration.Envoy.Service.Name
+	}
+	switch strings.ToLower(elbs.Kind) {
+	case "hostname":
+		s.log.WithField("loadbalancer-fqdns", lbAddress).Info("Using supplied hostname for Ingress status")
+		lbsw.lbStatus <- parseStatusFlag(elbs.FQDNs)
+	case "service":
+		// Register an informer to watch supplied service
 		serviceHandler := &k8s.ServiceStatusLoadBalancerWatcher{
-			ServiceName: contourConfiguration.Envoy.Service.Name,
+			ServiceName: elbs.Name,
 			LBStatus:    lbsw.lbStatus,
 			Log:         s.log.WithField("context", "serviceStatusLoadBalancerWatcher"),
 		}
 
 		var handler cache.ResourceEventHandler = serviceHandler
-		if contourConfiguration.Envoy.Service.Namespace != "" {
-			handler = k8s.NewNamespaceFilter([]string{contourConfiguration.Envoy.Service.Namespace}, handler)
+		if elbs.Namespace != "" {
+			handler = k8s.NewNamespaceFilter([]string{elbs.Namespace}, handler)
 		}
 
 		if err := s.informOnResource(&corev1.Service{}, handler); err != nil {
-			s.log.WithError(err).WithField("resource", "services").Fatal("failed to create informer")
+			s.log.WithError(err).WithField("resource", "services").Fatal("failed to create services informer")
 		}
-
+		s.log.Infof("Watching %s for Ingress status", elbs)
+	case "ingress":
+		// Register an informer to watch supplied ingress
 		ingressHandler := &k8s.IngressStatusLoadBalancerWatcher{
-			ServiceName: contourConfiguration.Envoy.Service.Name,
+			IngressName: elbs.Name,
 			LBStatus:    lbsw.lbStatus,
 			Log:         s.log.WithField("context", "ingressStatusLoadBalancerWatcher"),
 		}
 
-		var ingressEventHandler cache.ResourceEventHandler = ingressHandler
-		if contourConfiguration.Envoy.Ingress.Namespace != "" {
-			handler = k8s.NewNamespaceFilter([]string{contourConfiguration.Envoy.Ingress.Namespace}, handler)
+		var handler cache.ResourceEventHandler = ingressHandler
+		if elbs.Namespace != "" {
+			handler = k8s.NewNamespaceFilter([]string{elbs.Namespace}, handler)
 		}
 
-		if err := informOnResource(&networking_v1.Ingress{}, ingressEventHandler, s.mgr.GetCache()); err != nil {
+		if err := s.informOnResource(&networking_v1.Ingress{}, handler); err != nil {
 			s.log.WithError(err).WithField("resource", "ingresses").Fatal("failed to create ingresses informer")
 		}
+		s.log.Infof("Watching %s for Ingress status", elbs)
+	default:
+		return fmt.Errorf("unsupported ingress kind: %s", elbs.Kind)
+	}
 
-		s.log.WithField("envoy-service-name", contourConfiguration.Envoy.Service.Name).
-			WithField("envoy-service-namespace", contourConfiguration.Envoy.Service.Namespace).
-			Info("Watching Service for Ingress status")
+	return nil
+}
 
-		s.log.WithField("envoy-ingress-name", contourConfiguration.Envoy.Ingress.Name).
-			WithField("envoy-ingress-namespace", contourConfiguration.Envoy.Ingress.Namespace).
-			Info("Watching Ingress for Ingress status")
-	}
+type envoyLoadBalancerStatus struct {
+	Kind  string
+	FQDNs string
+	config.NamespacedName
+}
 
-	xdsServer := &xdsServer{
-		log:             s.log,
-		registry:        s.registry,
-		config:          *contourConfiguration.XDSServer,
-		snapshotHandler: snapshotHandler,
-		resources:       resources,
-		initialDagBuilt: contourHandler.HasBuiltInitialDag,
+func (elbs *envoyLoadBalancerStatus) String() string {
+	if elbs.Kind == "hostname" {
+		return fmt.Sprintf("%s:%s", elbs.Kind, elbs.FQDNs)
 	}
-	if err := s.mgr.Add(xdsServer); err != nil {
-		return err
+	return fmt.Sprintf("%s:%s/%s", elbs.Kind, elbs.Namespace, elbs.Name)
+}
+
+func parseEnvoyLoadBalancerStatus(s string) (*envoyLoadBalancerStatus, error) {
+	parts := strings.SplitN(s, ":", 2)
+	if len(parts) != 2 {
+		return nil, fmt.Errorf("invalid load-balancer-status: %s", s)
 	}
 
-	notifier := &leadership.Notifier{
-		ToNotify: append([]leadership.NeedLeaderElectionNotification{
-			contourHandler,
-			observer,
-		}, needsNotification...),
+	if parts[1] == "" {
+		return nil, fmt.Errorf("invalid load-balancer-status: empty object reference")
 	}
-	if err := s.mgr.Add(notifier); err != nil {
-		return err
+
+	elbs := envoyLoadBalancerStatus{}
+
+	elbs.Kind = strings.ToLower(parts[0])
+	switch elbs.Kind {
+	case "ingress", "service":
+		parts = strings.Split(parts[1], "/")
+		if len(parts) != 2 {
+			return nil, fmt.Errorf("invalid load-balancer-status: %s is not in the format of <namespace>/<name>", s)
+		}
+
+		if parts[0] == "" || parts[1] == "" {
+			return nil, fmt.Errorf("invalid load-balancer-status: <namespace> or <name> is empty")
+		}
+		elbs.Namespace = parts[0]
+		elbs.Name = parts[1]
+	case "hostname":
+		elbs.FQDNs = parts[1]
+	case "":
+		return nil, fmt.Errorf("invalid load-balancer-status: kind is empty")
+	default:
+		return nil, fmt.Errorf("invalid load-balancer-status: unsupported kind: %s", elbs.Kind)
 	}
 
-	// GO!
-	return s.mgr.Start(signals.SetupSignalHandler())
+	return &elbs, nil
 }
 
 func (s *Server) getExtensionSvcConfig(name string, namespace string) (xdscache_v3.ExtensionServiceConfig, error) {

cmd/contour/serve_test.go

@@ -19,6 +19,7 @@ import (
 	contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
 	"github.com/projectcontour/contour/internal/dag"
 	"github.com/projectcontour/contour/internal/ref"
+	"github.com/projectcontour/contour/pkg/config"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -217,3 +218,109 @@ func mustGetIngressProcessor(t *testing.T, builder *dag.Builder) *dag.IngressPro
 	require.FailNow(t, "IngressProcessor not found in list of DAG builder's processors")
 	return nil
 }
+
+func TestParseEnvoyLoadBalancerStatus(t *testing.T) {
+
+	tests := []struct {
+		name   string
+		status string
+		want   envoyLoadBalancerStatus
+	}{
+		{
+			name:   "Service",
+			status: "service:namespace-1/name-1",
+			want: envoyLoadBalancerStatus{
+				Kind: "service",
+				NamespacedName: config.NamespacedName{
+					Name:      "name-1",
+					Namespace: "namespace-1",
+				},
+			},
+		},
+		{
+			name:   "Ingress",
+			status: "ingress:namespace-1/name-1",
+			want: envoyLoadBalancerStatus{
+				Kind: "ingress",
+				NamespacedName: config.NamespacedName{
+					Name:      "name-1",
+					Namespace: "namespace-1",
+				},
+			},
+		},
+		{
+			name:   "hostname",
+			status: "hostname:example.com",
+			want: envoyLoadBalancerStatus{
+				Kind:  "hostname",
+				FQDNs: "example.com",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r, err := parseEnvoyLoadBalancerStatus(tt.status)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.want, *r)
+		})
+	}
+
+	tests2 := []struct {
+		name   string
+		status string
+		error  string
+	}{
+		{
+			name:   "Empty",
+			status: "",
+			error:  "invalid",
+		},
+		{
+			name:   "No kind",
+			status: ":n",
+			error:  "kind is empty",
+		},
+		{
+			name:   "Invalid kind",
+			status: "test:n",
+			error:  "unsupported kind",
+		},
+		{
+			name:   "No reference",
+			status: "service:",
+			error:  "empty object reference",
+		},
+		{
+			name:   "No colon",
+			status: "service",
+			error:  "invalid",
+		},
+		{
+			name:   "No slash",
+			status: "service:name-1",
+			error:  "not in the format",
+		},
+		{
+			name:   "starts with slash",
+			status: "service:/name-1",
+			error:  "is empty",
+		},
+		{
+			name:   "ends with slash",
+			status: "service:name-1/",
+			error:  "is empty",
+		},
+		{
+			name:   "two many slashes",
+			status: "service:name/x/y",
+			error:  "not in the format",
+		},
+	}
+	for _, tt := range tests2 {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := parseEnvoyLoadBalancerStatus(tt.status)
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), tt.error)
+		})
+	}
+}

cmd/contour/servecontext.go

@@ -544,10 +544,6 @@ func (ctx *serveContext) convertToContourConfigurationSpec() contour_api_v1alpha
 				Name:      ctx.Config.EnvoyServiceName,
 				Namespace: ctx.Config.EnvoyServiceNamespace,
 			},
-			Ingress: &contour_api_v1alpha1.NamespacedName{
-				Name:      ctx.Config.EnvoyIngressName,
-				Namespace: ctx.Config.EnvoyIngressNamespace,
-			},
 			HTTPListener: &contour_api_v1alpha1.EnvoyListener{
 				Address:   ctx.httpAddr,
 				Port:      ctx.httpPort,
@@ -581,6 +577,7 @@ func (ctx *serveContext) convertToContourConfigurationSpec() contour_api_v1alpha
 				XffNumTrustedHops: &ctx.Config.Network.XffNumTrustedHops,
 				EnvoyAdminPort:    &ctx.Config.Network.EnvoyAdminPort,
 			},
+			LoadBalancer: ctx.Config.LoadBalancerStatus,
 		},
 		Gateway: gatewayConfig,
 		HTTPProxy: &contour_api_v1alpha1.HTTPProxyConfig{