basic Gateway address support (#4443)

Adds provisioner support for a single
Gateway address of type IP or hostname.
The value will be used for the Envoy
service's spec.loadBalancerIP field.

Sets the Gateway's Ready condition to false
with a reason of AddressNotAssigned if at
lease one address has been requested, but
no requested address has been assigned.

Updates #4235.

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

changelogs/unreleased/4443-skriss-minor.md

@@ -0,0 +1,8 @@
+## Gateway provisioner: support requesting a specific address
+
+The Gateway provisioner now supports requesting a specific Gateway address, via the Gateway's `spec.addresses` field.
+Only one address is supported, and it must be either an `IPAddress` or `Hostname` type.
+The value of this address will be used to set the provisioned Envoy service's `spec.loadBalancerIP` field.
+If for any reason, the requested address is not assigned to the Gateway, the Gateway will have a condition of "Ready: false" with a reason of `AddressesNotAssigned`.
+
+If no address is requested, no value will be specified in the provisioned Envoy service's `spec.loadBalancerIP` field, and an address will be assigned by the load balancer provider.

internal/dag/gatewayapi_processor.go

@@ -20,7 +20,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/projectcontour/contour/internal/errors"
 	"github.com/projectcontour/contour/internal/k8s"
 	"github.com/projectcontour/contour/internal/status"
 
@@ -30,7 +29,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/validation"
-	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/utils/pointer"
 	gatewayapi_v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 )
@@ -67,9 +65,6 @@ type matchConditions struct {
 // Run translates Service APIs into DAG objects and
 // adds them to the DAG.
 func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) {
-	var gatewayErrors field.ErrorList
-	path := field.NewPath("spec")
-
 	p.dag = dag
 	p.source = source
 
@@ -96,15 +91,56 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) {
 	)
 	defer commit()
 
-	if len(p.source.gateway.Spec.Addresses) > 0 {
-		gatewayErrors = append(gatewayErrors, &field.Error{Type: field.ErrorTypeNotSupported, Field: path.String(), BadValue: p.source.gateway.Spec.Addresses, Detail: "Spec.Addresses is not supported"})
+	var gatewayNotReadyCondition *metav1.Condition
+
+	if !isAddressAssigned(p.source.gateway.Spec.Addresses, p.source.gateway.Status.Addresses) {
+		gatewayNotReadyCondition = &metav1.Condition{
+			Type:    string(gatewayapi_v1alpha2.GatewayConditionReady),
+			Status:  metav1.ConditionFalse,
+			Reason:  string(gatewayapi_v1alpha2.GatewayReasonAddressNotAssigned),
+			Message: "None of the addresses in Spec.Addresses have been assigned to the Gateway",
+		}
 	}
 
 	for _, listener := range p.source.gateway.Spec.Listeners {
-		p.computeListener(listener, gwAccessor, len(gatewayErrors) == 0)
+		p.computeListener(listener, gwAccessor, gatewayNotReadyCondition == nil)
+	}
+
+	p.computeGatewayConditions(gwAccessor, gatewayNotReadyCondition)
+}
+
+// isAddressAssigned returns true if either there are no addresses requested in specAddresses,
+// or if at least one address from specAddresses appears in statusAddresses.
+func isAddressAssigned(specAddresses, statusAddresses []gatewayapi_v1alpha2.GatewayAddress) bool {
+	if len(specAddresses) == 0 {
+		return true
+	}
+
+	for _, specAddress := range specAddresses {
+		for _, statusAddress := range statusAddresses {
+			// Types must match
+			if addressTypeDerefOr(specAddress.Type, gatewayapi_v1alpha2.IPAddressType) != addressTypeDerefOr(statusAddress.Type, gatewayapi_v1alpha2.IPAddressType) {
+				continue
+			}
+
+			// Values must match
+			if specAddress.Value != statusAddress.Value {
+				continue
+			}
+
+			return true
+		}
 	}
 
-	p.computeGatewayConditions(gwAccessor, gatewayErrors)
+	// No match found, so no spec address is assigned.
+	return false
+}
+
+func addressTypeDerefOr(addressType *gatewayapi_v1alpha2.AddressType, defaultAddressType gatewayapi_v1alpha2.AddressType) gatewayapi_v1alpha2.AddressType {
+	if addressType != nil {
+		return *addressType
+	}
+	return defaultAddressType
 }
 
 func (p *GatewayAPIProcessor) computeListener(listener gatewayapi_v1alpha2.Listener, gwAccessor *status.GatewayStatusUpdate, isGatewayValid bool) {
@@ -636,7 +672,7 @@ func routeSelectsGatewayListener(gateway *gatewayapi_v1alpha2.Gateway, listener
 	return false
 }
 
-func (p *GatewayAPIProcessor) computeGatewayConditions(gwAccessor *status.GatewayStatusUpdate, fieldErrs field.ErrorList) {
+func (p *GatewayAPIProcessor) computeGatewayConditions(gwAccessor *status.GatewayStatusUpdate, gatewayNotReadyCondition *metav1.Condition) {
 	// If Contour's running, the Gateway is considered scheduled.
 	gwAccessor.AddCondition(
 		gatewayapi_v1alpha2.GatewayConditionScheduled,
@@ -646,9 +682,13 @@ func (p *GatewayAPIProcessor) computeGatewayConditions(gwAccessor *status.Gatewa
 	)
 
 	switch {
-	case len(fieldErrs) > 0:
-		// If we have Gateway-specific errors, use those to set the Ready=false condition.
-		gwAccessor.AddCondition(gatewayapi_v1alpha2.GatewayConditionReady, metav1.ConditionFalse, status.ReasonInvalidGateway, errors.ParseFieldErrors(fieldErrs))
+	case gatewayNotReadyCondition != nil:
+		gwAccessor.AddCondition(
+			gatewayapi_v1alpha2.GatewayConditionType(gatewayNotReadyCondition.Type),
+			gatewayNotReadyCondition.Status,
+			status.GatewayReasonType(gatewayNotReadyCondition.Reason),
+			gatewayNotReadyCondition.Message,
+		)
 	default:
 		// Check for any listeners with a Ready: false condition.
 		allListenersReady := true

internal/dag/status_test.go

@@ -4541,9 +4541,9 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) {
 				gatewayapi_v1alpha2.GatewayConditionScheduled: gatewayScheduledCondition(),
 				gatewayapi_v1alpha2.GatewayConditionReady: {
 					Type:    string(gatewayapi_v1alpha2.GatewayConditionReady),
-					Status:  contour_api_v1.ConditionFalse,
-					Reason:  status.ReasonInvalidGateway,
-					Message: "Unsupported value for spec; Spec.Addresses is not supported",
+					Status:  metav1.ConditionFalse,
+					Reason:  string(gatewayapi_v1alpha2.GatewayReasonAddressNotAssigned),
+					Message: "None of the addresses in Spec.Addresses have been assigned to the Gateway",
 				},
 			},
 			ListenerStatus: map[string]*gatewayapi_v1alpha2.ListenerStatus{

internal/k8s/statusaddress.go

@@ -168,22 +168,6 @@ func (s *StatusAddressUpdater) OnAdd(obj interface{}) {
 			}
 		}
 
-		// Only set the Gateway's address if it has a condition of "Ready: true".
-		var ready bool
-		for _, cond := range o.Status.Conditions {
-			if cond.Type == string(gatewayapi_v1alpha2.GatewayConditionReady) && cond.Status == metav1.ConditionTrue {
-				ready = true
-				break
-			}
-		}
-		if !ready {
-			s.Logger.
-				WithField("name", o.Name).
-				WithField("namespace", o.Namespace).
-				Debug("Gateway is not ready, not setting address")
-			return
-		}
-
 		s.StatusUpdater.Send(NewStatusUpdate(
 			o.Name,
 			o.Namespace,

internal/k8s/statusaddress_test.go

@@ -466,44 +466,6 @@ func TestStatusAddressUpdater_Gateway(t *testing.T) {
 				},
 			},
 		},
-		"Gateway not ready": {
-			status:                     ipLBStatus,
-			gatewayClassControllerName: "projectcontour.io/contour",
-			preop: &gatewayapi_v1alpha2.Gateway{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: "projectcontour",
-					Name:      "contour-gateway",
-				},
-				Spec: gatewayapi_v1alpha2.GatewaySpec{
-					GatewayClassName: gatewayapi_v1alpha2.ObjectName("contour-gatewayclass"),
-				},
-				Status: gatewayapi_v1alpha2.GatewayStatus{
-					Conditions: []metav1.Condition{
-						{
-							Type:   string(gatewayapi_v1alpha2.GatewayConditionReady),
-							Status: metav1.ConditionFalse,
-						},
-					},
-				},
-			},
-			postop: &gatewayapi_v1alpha2.Gateway{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: "projectcontour",
-					Name:      "contour-gateway",
-				},
-				Spec: gatewayapi_v1alpha2.GatewaySpec{
-					GatewayClassName: gatewayapi_v1alpha2.ObjectName("contour-gatewayclass"),
-				},
-				Status: gatewayapi_v1alpha2.GatewayStatus{
-					Conditions: []metav1.Condition{
-						{
-							Type:   string(gatewayapi_v1alpha2.GatewayConditionReady),
-							Status: metav1.ConditionFalse,
-						},
-					},
-				},
-			},
-		},
 		"Gateway not controlled by this Contour": {
 			status:                     ipLBStatus,
 			gatewayClassControllerName: "projectcontour.io/some-other-controller",

internal/provisioner/controller/gateway.go

@@ -213,6 +213,18 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		},
 	}
 
+	// Currently, only a single address of type IPAddress or Hostname
+	// is supported; anything else will be ignored.
+	if len(gateway.Spec.Addresses) > 0 {
+		address := gateway.Spec.Addresses[0]
+
+		if address.Type == nil ||
+			*address.Type == gatewayapi_v1alpha2.IPAddressType ||
+			*address.Type == gatewayapi_v1alpha2.HostnameAddressType {
+			gatewayContour.Spec.NetworkPublishing.Envoy.LoadBalancer.LoadBalancerIP = address.Value
+		}
+	}
+
 	for _, listener := range gateway.Spec.Listeners {
 		port := model.ServicePort{
 			Name:       string(listener.Name),

internal/provisioner/controller/gateway_test.go

@@ -18,12 +18,15 @@ import (
 	"testing"
 
 	"github.com/go-logr/logr"
+	"github.com/projectcontour/contour/internal/gatewayapi"
 	"github.com/projectcontour/contour/internal/provisioner"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
@@ -179,6 +182,140 @@ func TestGatewayReconcile(t *testing.T) {
 				assert.True(t, errors.IsNotFound(err))
 			},
 		},
+		"A gateway with no addresses results in an Envoy service with no loadBalancerIP": {
+			gatewayClass: reconcilableGatewayClass("gatewayclass-1", controller),
+			gateway: &gatewayv1alpha2.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "gateway-1",
+				},
+				Spec: gatewayv1alpha2.GatewaySpec{
+					GatewayClassName: "gatewayclass-1",
+				},
+			},
+			assertions: func(t *testing.T, r *gatewayReconciler, gw *gatewayv1alpha2.Gateway, reconcileErr error) {
+				require.NoError(t, reconcileErr)
+				assertEnvoyServiceLoadBalancerIP(t, gw, r.client, "")
+			},
+		},
+		"A gateway with one IP address results in an Envoy service with loadBalancerIP set to that IP address": {
+			gatewayClass: reconcilableGatewayClass("gatewayclass-1", controller),
+			gateway: &gatewayv1alpha2.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "gateway-1",
+				},
+				Spec: gatewayv1alpha2.GatewaySpec{
+					GatewayClassName: "gatewayclass-1",
+					Addresses: []gatewayv1alpha2.GatewayAddress{
+						{
+							Type:  gatewayapi.AddressTypePtr(gatewayv1alpha2.IPAddressType),
+							Value: "172.18.255.207",
+						},
+					},
+				},
+			},
+			assertions: func(t *testing.T, r *gatewayReconciler, gw *gatewayv1alpha2.Gateway, reconcileErr error) {
+				require.NoError(t, reconcileErr)
+				assertEnvoyServiceLoadBalancerIP(t, gw, r.client, "172.18.255.207")
+			},
+		},
+		"A gateway with two IP addresses results in an Envoy service with loadBalancerIP set to the first IP address": {
+			gatewayClass: reconcilableGatewayClass("gatewayclass-1", controller),
+			gateway: &gatewayv1alpha2.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "gateway-1",
+				},
+				Spec: gatewayv1alpha2.GatewaySpec{
+					GatewayClassName: "gatewayclass-1",
+					Addresses: []gatewayv1alpha2.GatewayAddress{
+						{
+							Type:  gatewayapi.AddressTypePtr(gatewayv1alpha2.IPAddressType),
+							Value: "172.18.255.207",
+						},
+						{
+							Type:  gatewayapi.AddressTypePtr(gatewayv1alpha2.IPAddressType),
+							Value: "172.18.255.999",
+						},
+					},
+				},
+			},
+			assertions: func(t *testing.T, r *gatewayReconciler, gw *gatewayv1alpha2.Gateway, reconcileErr error) {
+				require.NoError(t, reconcileErr)
+				assertEnvoyServiceLoadBalancerIP(t, gw, r.client, "172.18.255.207")
+			},
+		},
+		"A gateway with one Hostname address results in an Envoy service with loadBalancerIP set to that hostname": {
+			gatewayClass: reconcilableGatewayClass("gatewayclass-1", controller),
+			gateway: &gatewayv1alpha2.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "gateway-1",
+				},
+				Spec: gatewayv1alpha2.GatewaySpec{
+					GatewayClassName: "gatewayclass-1",
+					Addresses: []gatewayv1alpha2.GatewayAddress{
+						{
+							Type:  gatewayapi.AddressTypePtr(gatewayv1alpha2.HostnameAddressType),
+							Value: "projectcontour.io",
+						},
+					},
+				},
+			},
+			assertions: func(t *testing.T, r *gatewayReconciler, gw *gatewayv1alpha2.Gateway, reconcileErr error) {
+				require.NoError(t, reconcileErr)
+				assertEnvoyServiceLoadBalancerIP(t, gw, r.client, "projectcontour.io")
+			},
+		},
+		"A gateway with two Hostname addresses results in an Envoy service with loadBalancerIP set to the first hostname": {
+			gatewayClass: reconcilableGatewayClass("gatewayclass-1", controller),
+			gateway: &gatewayv1alpha2.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "gateway-1",
+				},
+				Spec: gatewayv1alpha2.GatewaySpec{
+					GatewayClassName: "gatewayclass-1",
+					Addresses: []gatewayv1alpha2.GatewayAddress{
+						{
+							Type:  gatewayapi.AddressTypePtr(gatewayv1alpha2.HostnameAddressType),
+							Value: "projectcontour.io",
+						},
+						{
+							Type:  gatewayapi.AddressTypePtr(gatewayv1alpha2.HostnameAddressType),
+							Value: "anotherhost.io",
+						},
+					},
+				},
+			},
+			assertions: func(t *testing.T, r *gatewayReconciler, gw *gatewayv1alpha2.Gateway, reconcileErr error) {
+				require.NoError(t, reconcileErr)
+				assertEnvoyServiceLoadBalancerIP(t, gw, r.client, "projectcontour.io")
+			},
+		},
+		"A gateway with one named address results in an Envoy service with no loadBalancerIP": {
+			gatewayClass: reconcilableGatewayClass("gatewayclass-1", controller),
+			gateway: &gatewayv1alpha2.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "gateway-1",
+				},
+				Spec: gatewayv1alpha2.GatewaySpec{
+					GatewayClassName: "gatewayclass-1",
+					Addresses: []gatewayv1alpha2.GatewayAddress{
+						{
+							Type:  gatewayapi.AddressTypePtr(gatewayv1alpha2.NamedAddressType),
+							Value: "named-addresses-are-not-supported",
+						},
+					},
+				},
+			},
+			assertions: func(t *testing.T, r *gatewayReconciler, gw *gatewayv1alpha2.Gateway, reconcileErr error) {
+				require.NoError(t, reconcileErr)
+				assertEnvoyServiceLoadBalancerIP(t, gw, r.client, "")
+			},
+		},
 	}
 
 	for name, tc := range tests {
@@ -215,3 +352,17 @@ func TestGatewayReconcile(t *testing.T) {
 		})
 	}
 }
+
+func assertEnvoyServiceLoadBalancerIP(t *testing.T, gateway *gatewayv1alpha2.Gateway, client client.Client, want string) {
+	// Get the expected Envoy service from the client.
+	envoyService := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: gateway.Namespace,
+			Name:      "envoy-" + gateway.Name,
+		},
+	}
+	require.NoError(t, client.Get(context.Background(), keyFor(envoyService), envoyService))
+
+	// Verify expected Spec.LoadBalancerIP.
+	assert.Equal(t, want, envoyService.Spec.LoadBalancerIP)
+}