Skip to content

Commit

Permalink
keep Gateway's AttachedRoutes field up to date (#4052)
Browse files Browse the repository at this point in the history 
* feat: attachedRoutes updated

The AttachedRoutes listenerStatus field gets updated and enforced by the
GatewayController.

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

* chore: getAttachedRoutesForListener func created

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

* chore: routeAcceptedByGateways func factorized

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

* chore: PR rebased

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

* Update internal/controllers/gateway/gateway_utils.go

Co-authored-by: Patryk Małek <patryk.malek@konghq.com>

* chore: unit tests for IsRouteAcceptedByListener

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

* chore: PR comments addressed

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

---------

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>
Co-authored-by: Patryk Małek <patryk.malek@konghq.com>
  • Loading branch information
@mlavacca @pmalek
mlavacca and pmalek authored May 26, 2023
1 parent d369756 commit d8c1396
Show file tree
Hide file tree
Showing 9 changed files with 784 additions and 22 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ Adding a new version? You'll need three changes:
- `service`: will make KIC build addresses using the following template:
`pod-ip-address.service-name.my-namespace.svc`.
This is known to not work on GKE becuase it uses `kube-dns` instead of coredns.
- Gateway's `AttachedRoutes` fields get updated with the number of routes referencing
and using each listener.
[#4052](https://github.com/Kong/kubernetes-ingress-controller/pull/4052)

### Changed

Expand Down
30 changes: 30 additions & 0 deletions internal/controllers/gateway/gateway_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,15 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error {
return err
}

// if a HTTPRoute gets accepted by a Gateway, we need to make sure to trigger
// reconciliation on the gateway, as we need to update the number of attachedRoutes.
if err := c.Watch(
source.Kind(mgr.GetCache(), &gatewayv1beta1.HTTPRoute{}),
handler.EnqueueRequestsFromMapFunc(r.listGatewaysForHTTPRoute),
); err != nil {
return err
}

// watch ReferenceGrants, which may invalidate or allow cross-namespace TLSConfigs
if r.enableReferenceGrant {
if err := c.Watch(
Expand Down Expand Up @@ -266,6 +275,27 @@ func (r *GatewayReconciler) listGatewaysForService(ctx context.Context, svc clie
return
}

// listGatewaysForHTTPRoute retrieves all the gateways referenced as parents by the HTTPRoute.
func (r *GatewayReconciler) listGatewaysForHTTPRoute(_ context.Context, obj client.Object) []reconcile.Request {
httpRoute, ok := obj.(*gatewayv1beta1.HTTPRoute)
if !ok {
r.Log.Error(
fmt.Errorf("unexpected object type"),
"httproute watch predicate received unexpected object type",
"expected", "*gatewayv1beta1.HTTPRoute", "found", reflect.TypeOf(obj),
)
return nil
}
recs := []reconcile.Request{}
for _, gateway := range routeAcceptedByGateways(httpRoute.Namespace, httpRoute.Status.Parents) {
recs = append(recs, reconcile.Request{
NamespacedName: gateway,
})
}

return recs
}

// isGatewayService is a watch predicate that filters out events for objects that aren't
// the gateway service referenced by --publish-service or --publish-service-udp.
func (r *GatewayReconciler) isGatewayService(obj client.Object) bool {
Expand Down
119 changes: 103 additions & 16 deletions internal/controllers/gateway/gateway_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,9 @@ func extractListenerSpecFromGateway(gateway *gatewayv1beta1.Gateway, listenerNam
}

type (
protocolPortMap map[ProtocolType]map[PortNumber]bool
portProtocolMap map[PortNumber]ProtocolType
portHostnameMap map[PortNumber]map[Hostname]bool
listenerAttachedMap map[SectionName]int32
protocolPortMap map[ProtocolType]map[PortNumber]bool
portProtocolMap map[PortNumber]ProtocolType
portHostnameMap map[PortNumber]map[Hostname]bool
)

func buildKongPortMap(listens []Listener) protocolPortMap {
Expand All @@ -182,11 +181,9 @@ func buildKongPortMap(listens []Listener) protocolPortMap {
func initializeListenerMaps(gateway *Gateway) (
portProtocolMap,
portHostnameMap,
listenerAttachedMap,
) {
portToProtocol := make(portProtocolMap, len(gateway.Status.Listeners))
portToHostname := make(portHostnameMap, len(gateway.Status.Listeners))
listenerToAttached := make(listenerAttachedMap, len(gateway.Status.Listeners))

existingStatuses := make(map[SectionName]ListenerStatus,
len(gateway.Status.Listeners))
Expand All @@ -196,13 +193,8 @@ func initializeListenerMaps(gateway *Gateway) (

for _, listener := range gateway.Spec.Listeners {
portToHostname[listener.Port] = make(map[Hostname]bool)
if existingStatus, ok := existingStatuses[listener.Name]; ok {
listenerToAttached[listener.Name] = existingStatus.AttachedRoutes
} else {
listenerToAttached[listener.Name] = 0
}
}
return portToProtocol, portToHostname, listenerToAttached
return portToProtocol, portToHostname
}

func canSharePort(requested, existing ProtocolType) bool {
Expand Down Expand Up @@ -244,14 +236,14 @@ func getListenerStatus(
client client.Client,
) ([]ListenerStatus, error) {
statuses := make(map[SectionName]ListenerStatus, len(gateway.Spec.Listeners))
portToProtocol, portToHostname, listenerToAttached := initializeListenerMaps(gateway)
portToProtocol, portToHostname := initializeListenerMaps(gateway)
kongProtocolsToPort := buildKongPortMap(kongListens)
conflictedPorts := make(map[PortNumber]bool, len(gateway.Spec.Listeners))
conflictedHostnames := make(map[PortNumber]map[Hostname]bool, len(gateway.Spec.Listeners))

// TODO we should check transition time rather than always nowing, which we do throughout the below
// https://github.com/Kong/kubernetes-ingress-controller/issues/2556
for _, listener := range gateway.Spec.Listeners {
for listenerIndex, listener := range gateway.Spec.Listeners {
var hostname Hostname
if listener.Hostname != nil {
hostname = *listener.Hostname
Expand Down Expand Up @@ -300,12 +292,16 @@ func getListenerStatus(
}
}

attachedRoutes, err := getAttachedRoutesForListener(ctx, client, *gateway, listenerIndex)
if err != nil {
return nil, err
}

status := ListenerStatus{
Name: listener.Name,
Conditions: []metav1.Condition{},
SupportedKinds: supportedkinds,
// this has been populated by initializeListenerMaps()
AttachedRoutes: listenerToAttached[listener.Name],
AttachedRoutes: attachedRoutes,
}

// if the resolvedRefs condition is not successful, append the resolvedRefs condition failed with the proper reason
Expand Down Expand Up @@ -670,3 +666,94 @@ func isTLSSecretValid(secret *corev1.Secret) bool {
}
return true
}

// routeAcceptedByGateways finds all the Gateways the route has been accepted by
// and returns them in the form of a NamespacedName slice.
func routeAcceptedByGateways(routeNamespace string, parentStatuses []RouteParentStatus) []k8stypes.NamespacedName {
gateways := []k8stypes.NamespacedName{}
for _, routeParentStatus := range parentStatuses {
gatewayNamespace := routeNamespace
parentRef := routeParentStatus.ParentRef
if (parentRef.Group != nil && *parentRef.Group != gatewayV1beta1Group) ||
(parentRef.Kind != nil && *parentRef.Kind != "Gateway") {
continue
}
if parentRef.Namespace != nil {
gatewayNamespace = string(*parentRef.Namespace)
}
if lo.ContainsBy(routeParentStatus.Conditions, func(condition metav1.Condition) bool {
return condition.Type == string(gatewayv1beta1.RouteConditionAccepted) &&
condition.Status == metav1.ConditionTrue
}) {
gateways = append(gateways,
k8stypes.NamespacedName{
Namespace: gatewayNamespace,
Name: string(parentRef.Name),
})
}
}
return gateways
}

// getAttachedRoutesForListener returns the number of all the routes that are attached
// to the provided Gateway.
//
// NOTE: At this point we take into account HTTPRoutes only, as they are the
// only routes in beta.
func getAttachedRoutesForListener(ctx context.Context, mgrc client.Client, gateway gatewayv1beta1.Gateway, listenerIndex int) (int32, error) {
const (
defaultEndpointSliceListPagingLimit = 100
)
var (
httpRoutes = []gatewayv1beta1.HTTPRoute{}
continueToken string
)
for {
httpRouteList := gatewayv1beta1.HTTPRouteList{}
if err := mgrc.List(ctx, &httpRouteList, &client.ListOptions{
Continue: continueToken,
Limit: defaultEndpointSliceListPagingLimit,
}); err != nil {
return 0, err
}
httpRoutes = append(httpRoutes, httpRouteList.Items...)
if httpRouteList.Continue == "" {
break
}
continueToken = httpRouteList.Continue
}

var attachedRoutes int32
for _, route := range httpRoutes {
route := route
acceptedByGateway := func() bool {
for _, g := range routeAcceptedByGateways(route.Namespace, route.Status.Parents) {
if gateway.Namespace == g.Namespace && gateway.Name == g.Name {
return true
}
}
return false
}()
if !acceptedByGateway {
continue
}

for _, parentRef := range route.Spec.ParentRefs {
accepted, err := isRouteAcceptedByListener(
ctx,
mgrc,
&route,
gateway,
listenerIndex,
parentRef,
)
if err != nil {
return 0, err
}
if accepted {
attachedRoutes++
}
}
}
return attachedRoutes, nil
}
127 changes: 127 additions & 0 deletions internal/controllers/gateway/gateway_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8stypes "k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

Expand Down Expand Up @@ -141,3 +142,129 @@ func assertOnlyOneConditionForType(t *testing.T, conditions []metav1.Condition)
assert.Equalf(t, 1, n, "condition %s occurred %d times - expected 1 occurrence", c, n)
}
}

func TestRouteAcceptedByGateways(t *testing.T) {
testCases := []struct {
name string
routeNamespace string
parentStatuses []gatewayv1beta1.RouteParentStatus
gateways []k8stypes.NamespacedName
}{
{
name: "no parentStatus with accepted condition",
routeNamespace: "default",
parentStatuses: []gatewayv1beta1.RouteParentStatus{
{
ParentRef: gatewayv1beta1.ParentReference{
Name: "gateway-1",
},
},
},
gateways: []k8stypes.NamespacedName{},
},
{
name: "a subset of parentStatus with correct params",
routeNamespace: "default",
parentStatuses: []gatewayv1beta1.RouteParentStatus{
{
ParentRef: gatewayv1beta1.ParentReference{
Name: "gateway-1",
Group: lo.ToPtr(gatewayv1beta1.Group("wrong-group")),
},
Conditions: []metav1.Condition{
{
Status: metav1.ConditionTrue,
Type: string(gatewayv1beta1.RouteConditionAccepted),
},
},
},
{
ParentRef: gatewayv1beta1.ParentReference{
Name: "gateway-2",
Kind: lo.ToPtr(gatewayv1beta1.Kind("wrong-kind")),
},
Conditions: []metav1.Condition{
{
Status: metav1.ConditionTrue,
Type: string(gatewayv1beta1.RouteConditionAccepted),
},
},
},
{
ParentRef: gatewayv1beta1.ParentReference{
Name: "gateway-3",
},
Conditions: []metav1.Condition{
{
Status: metav1.ConditionTrue,
Type: string(gatewayv1beta1.RouteConditionAccepted),
},
},
},
{
ParentRef: gatewayv1beta1.ParentReference{
Name: "gateway-4",
},
Conditions: []metav1.Condition{
{
Status: metav1.ConditionFalse,
Type: string(gatewayv1beta1.RouteConditionAccepted),
},
},
},
},
gateways: []k8stypes.NamespacedName{
{
Namespace: "default",
Name: "gateway-3",
},
},
},
{
name: "all parentStatuses",
routeNamespace: "default",
parentStatuses: []gatewayv1beta1.RouteParentStatus{
{
ParentRef: gatewayv1beta1.ParentReference{
Name: "gateway-1",
},
Conditions: []metav1.Condition{
{
Status: metav1.ConditionTrue,
Type: string(gatewayv1beta1.RouteConditionAccepted),
},
},
},
{
ParentRef: gatewayv1beta1.ParentReference{
Name: "gateway-2",
Namespace: lo.ToPtr(gatewayv1beta1.Namespace("namespace-2")),
},
Conditions: []metav1.Condition{
{
Status: metav1.ConditionTrue,
Type: string(gatewayv1beta1.RouteConditionAccepted),
},
},
},
},
gateways: []k8stypes.NamespacedName{
{
Namespace: "default",
Name: "gateway-1",
},
{
Namespace: "namespace-2",
Name: "gateway-2",
},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
gateways := routeAcceptedByGateways(tc.routeNamespace, tc.parentStatuses)
assert.Equal(t, tc.gateways, gateways)
})
}
}
Loading

0 comments on commit d8c1396

Please sign in to comment.