Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion controllers/gateway/eventhandlers/grpc_route_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (h *enqueueRequestsForGRPCRouteEvent) Generic(ctx context.Context, e event.
}

func (h *enqueueRequestsForGRPCRouteEvent) enqueueImpactedGateways(ctx context.Context, route *gatewayv1.GRPCRoute, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Namespace, constants.ALBGatewayController)
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Status.Parents, route.Namespace, constants.ALBGatewayController)
if err != nil {
h.logger.V(1).Info("ignoring unknown gateways referred by", "grpcroute", route.Name, "error", err)
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/gateway/eventhandlers/http_route_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (h *enqueueRequestsForHTTPRouteEvent) Generic(ctx context.Context, e event.
}

func (h *enqueueRequestsForHTTPRouteEvent) enqueueImpactedGateways(ctx context.Context, route *gatewayv1.HTTPRoute, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Namespace, constants.ALBGatewayController)
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Status.Parents, route.Namespace, constants.ALBGatewayController)
if err != nil {
h.logger.V(1).Info("ignoring unknown gateways referred by", "httproute", route.Name, "error", err)
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/gateway/eventhandlers/tcp_route_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (h *enqueueRequestsForTCPRouteEvent) Generic(ctx context.Context, e event.T
}

func (h *enqueueRequestsForTCPRouteEvent) enqueueImpactedGateways(ctx context.Context, route *gwalpha2.TCPRoute, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Namespace, constants.NLBGatewayController)
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Status.Parents, route.Namespace, constants.NLBGatewayController)
if err != nil {
h.logger.V(1).Info("ignoring unknown gateways referred by", "tcproute", route.Name, "error", err)
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/gateway/eventhandlers/tls_route_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (h *enqueueRequestsForTLSRouteEvent) Generic(ctx context.Context, e event.T
}

func (h *enqueueRequestsForTLSRouteEvent) enqueueImpactedGateways(ctx context.Context, route *gwalpha2.TLSRoute, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Namespace, constants.NLBGatewayController)
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Status.Parents, route.Namespace, constants.NLBGatewayController)
if err != nil {
h.logger.V(1).Info("ignoring unknown gateways referred by", "tlsroute", route.Name, "error", err)
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/gateway/eventhandlers/udp_route_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (h *enqueueRequestsForUDPRouteEvent) Generic(ctx context.Context, e event.T
}

func (h *enqueueRequestsForUDPRouteEvent) enqueueImpactedGateways(ctx context.Context, route *gwalpha2.UDPRoute, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Namespace, constants.NLBGatewayController)
gateways, err := GetImpactedGatewaysFromParentRefs(ctx, h.k8sClient, route.Spec.ParentRefs, route.Status.Parents, route.Namespace, constants.NLBGatewayController)
if err != nil {
h.logger.V(1).Info("ignoring unknown gateways referred by", "udproute", route.Name, "error", err)
}
Expand Down
27 changes: 26 additions & 1 deletion controllers/gateway/eventhandlers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,12 @@ func GetGatewaysManagedByLBController(ctx context.Context, k8sClient client.Clie

// GetImpactedGatewaysFromParentRefs identifies Gateways affected by changes in parent references.
// Returns Gateways that are impacted and managed by the LB controller.
func GetImpactedGatewaysFromParentRefs(ctx context.Context, k8sClient client.Client, parentRefs []gwv1.ParentReference, resourceNamespace string, gwController string) ([]types.NamespacedName, error) {
func GetImpactedGatewaysFromParentRefs(ctx context.Context, k8sClient client.Client, parentRefs []gwv1.ParentReference, originalParentRefsFromStatus []gwv1.RouteParentStatus, resourceNamespace string, gwController string) ([]types.NamespacedName, error) {
for _, originalParentRef := range originalParentRefsFromStatus {
parentRefs = append(parentRefs, originalParentRef.ParentRef)
}
parentRefs = removeDuplicateParentRefs(parentRefs, resourceNamespace)

if len(parentRefs) == 0 {
return nil, nil
}
Expand Down Expand Up @@ -129,3 +134,23 @@ func GetImpactedGatewaysFromLbConfig(ctx context.Context, k8sClient client.Clien
}
return impactedGateways
}

// removeDuplicateParentRefs make sure parentRefs in list is unique
func removeDuplicateParentRefs(parentRefs []gwv1.ParentReference, resourceNamespace string) []gwv1.ParentReference {
result := make([]gwv1.ParentReference, 0, len(parentRefs))
exist := sets.Set[types.NamespacedName]{}
for _, parentRef := range parentRefs {
if parentRef.Namespace != nil {
resourceNamespace = string(*parentRef.Namespace)
}
namespacedName := types.NamespacedName{
Namespace: resourceNamespace,
Name: string(parentRef.Name),
}
if !exist.Has(namespacedName) {
exist.Insert(namespacedName)
result = append(result, parentRef)
}
}
return result
}
148 changes: 142 additions & 6 deletions controllers/gateway/eventhandlers/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,12 @@ func Test_GetGatewayClassesManagedByLBController(t *testing.T) {

func Test_GetImpactedGatewaysFromParentRefs(t *testing.T) {
type args struct {
parentRefs []gwv1.ParentReference
resourceNS string
gateways []*gwv1.Gateway
gatewayClasses []*gwv1.GatewayClass
gwController string
parentRefs []gwv1.ParentReference
originalParentRefsFromRouteStatus []gwv1.RouteParentStatus
resourceNS string
gateways []*gwv1.Gateway
gatewayClasses []*gwv1.GatewayClass
gwController string
}
tests := []struct {
name string
Expand Down Expand Up @@ -271,6 +272,68 @@ func Test_GetImpactedGatewaysFromParentRefs(t *testing.T) {
},
wantErr: nil,
},
{
name: "valid parent refs with managed gateways and originalParentRefsFromRouteStatus",
args: args{
parentRefs: []gwv1.ParentReference{
{
Name: "test-gw",
Namespace: (*gwv1.Namespace)(ptr.To("test-ns")),
},
},
resourceNS: "test-ns",
originalParentRefsFromRouteStatus: []gwv1.RouteParentStatus{
{
ParentRef: gwv1.ParentReference{
Name: "test-gw-1",
Namespace: (*gwv1.Namespace)(ptr.To("test-ns")),
},
},
},
gateways: []*gwv1.Gateway{
{
ObjectMeta: metav1.ObjectMeta{
Name: "test-gw",
Namespace: "test-ns",
},
Spec: gwv1.GatewaySpec{
GatewayClassName: "nlb-class",
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "test-gw-1",
Namespace: "test-ns",
},
Spec: gwv1.GatewaySpec{
GatewayClassName: "nlb-class",
},
},
},
gatewayClasses: []*gwv1.GatewayClass{
{
ObjectMeta: metav1.ObjectMeta{
Name: "nlb-class",
},
Spec: gwv1.GatewayClassSpec{
ControllerName: constants.NLBGatewayController,
},
},
},
gwController: constants.NLBGatewayController,
},
want: []types.NamespacedName{
{
Namespace: "test-ns",
Name: "test-gw",
},
{
Namespace: "test-ns",
Name: "test-gw-1",
},
},
wantErr: nil,
},
{
name: "valid parent refs with unmanaged gateways",
args: args{
Expand Down Expand Up @@ -454,7 +517,7 @@ func Test_GetImpactedGatewaysFromParentRefs(t *testing.T) {
k8sClient.Create(context.Background(), gwClass)
}

got, err := GetImpactedGatewaysFromParentRefs(context.Background(), k8sClient, tt.args.parentRefs, tt.args.resourceNS, tt.args.gwController)
got, err := GetImpactedGatewaysFromParentRefs(context.Background(), k8sClient, tt.args.parentRefs, tt.args.originalParentRefsFromRouteStatus, tt.args.resourceNS, tt.args.gwController)

assert.Equal(t, err, tt.wantErr)
assert.Equal(t, tt.want, got)
Expand Down Expand Up @@ -768,3 +831,76 @@ func Test_GetImpactedGatewaysFromLbConfig(t *testing.T) {
})
}
}

func TestRemoveDuplicateParentRefs(t *testing.T) {
namespace := "test-namespace"
tests := []struct {
name string
parentRefs []gwv1.ParentReference
resourceNamespace string
want []gwv1.ParentReference
}{
{
name: "no duplicates",
parentRefs: []gwv1.ParentReference{
{Name: "gateway1"},
{Name: "gateway2"},
},
resourceNamespace: namespace,
want: []gwv1.ParentReference{
{Name: "gateway1"},
{Name: "gateway2"},
},
},
{
name: "one duplicate",
parentRefs: []gwv1.ParentReference{
{Name: "gateway1"},
{Name: "gateway1"},
{Name: "gateway2"},
},
resourceNamespace: namespace,
want: []gwv1.ParentReference{
{Name: "gateway1"},
{Name: "gateway2"},
},
},
{
name: "multiple duplicates",
parentRefs: []gwv1.ParentReference{
{Name: "gateway1"},
{Name: "gateway2"},
{Name: "gateway1"},
{Name: "gateway2"},
{Name: "gateway3"},
{Name: "gateway1"},
},
resourceNamespace: namespace,
want: []gwv1.ParentReference{
{Name: "gateway1"},
{Name: "gateway2"},
{Name: "gateway3"},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := removeDuplicateParentRefs(tt.parentRefs, tt.resourceNamespace)

assert.Equal(t, len(tt.want), len(got))

actual := make(map[string]bool)
for _, ref := range got {
actual[string(ref.Name)] = true
}

expected := make(map[string]bool)
for _, ref := range tt.want {
expected[string(ref.Name)] = true
}

assert.Equal(t, expected, actual)
})
}
}
21 changes: 12 additions & 9 deletions controllers/gateway/route_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@ func (d *routeReconcilerImpl) handleRouteStatusUpdate(routeData routeutils.Route
return client.IgnoreNotFound(err)
}
routeOld := route.DeepCopyObject().(client.Object)

// update route with current status
if err := d.updateRouteStatus(route, routeData); err != nil {
return err
}

// compare it with original status, patch if different
if !d.isRouteStatusIdentical(routeOld, route) {
if err := d.k8sClient.Status().Patch(context.Background(), route, client.MergeFrom(routeOld)); err != nil {
Expand Down Expand Up @@ -173,19 +175,20 @@ func (d *routeReconcilerImpl) updateRouteStatus(route client.Object, routeData r
if parentRef.Namespace != nil {
namespace = string(*parentRef.Namespace)
}
// for a given parentRef, if it has a statusInfo, this means condition is updated, override route condition based on route status info
if namespace == routeData.ParentRefGateway.Namespace {
if string(parentRef.Name) == routeData.ParentRefGateway.Name {

// do not allow backward generation update, Accepted and ResolvedRef always have same generation based on our implementation
if (len(newRouteParentStatus.Conditions) != 0 && newRouteParentStatus.Conditions[0].ObservedGeneration <= routeData.RouteMetadata.RouteGeneration) || len(newRouteParentStatus.Conditions) == 0 {
// for a given parentRef, if it has a statusInfo, this means condition is updated, override route condition based on route status info
if namespace == routeData.ParentRefGateway.Namespace && string(parentRef.Name) == routeData.ParentRefGateway.Name {
d.setConditionsWithRouteStatusInfo(route, &newRouteParentStatus, routeData.RouteStatusInfo)
}
}

// resolve ref Gateway, if parentRef does not have namespace, getting it from Route
if _, err := d.resolveRefGateway(parentRef, route.GetNamespace()); err != nil {
// set conditions if resolvedRef = false
d.setConditionsBasedOnResolveRefGateway(route, &newRouteParentStatus, err)
// resolve ref Gateway, if parentRef does not have namespace, getting it from Route
if _, err := d.resolveRefGateway(parentRef, route.GetNamespace()); err != nil {
// set conditions if resolvedRef = false
d.setConditionsBasedOnResolveRefGateway(route, &newRouteParentStatus, err)
}
}

newRouteStatus = append(newRouteStatus, newRouteParentStatus)
}

Expand Down
1 change: 1 addition & 0 deletions pkg/gateway/routeutils/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type routeMetadataDescriptor interface {
GetParentRefs() []gwv1.ParentReference
GetRawRoute() interface{}
GetBackendRefs() []gwv1.BackendRef
GetRouteGeneration() int64
}

// preLoadRouteDescriptor this object is used to represent a route description that has not loaded its child data (services, tg config)
Expand Down
4 changes: 4 additions & 0 deletions pkg/gateway/routeutils/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ func (grpcRoute *grpcRouteDescription) GetBackendRefs() []gwv1.BackendRef {
return backendRefs
}

func (grpcRoute *grpcRouteDescription) GetRouteGeneration() int64 {
return grpcRoute.route.Generation
}

var _ RouteDescriptor = &grpcRouteDescription{}

// Can we use an indexer here to query more efficiently?
Expand Down
4 changes: 4 additions & 0 deletions pkg/gateway/routeutils/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ func (httpRoute *httpRouteDescription) GetRouteKind() RouteKind {
return HTTPRouteKind
}

func (httpRoute *httpRouteDescription) GetRouteGeneration() int64 {
return httpRoute.route.Generation
}

func (httpRoute *httpRouteDescription) GetRouteNamespacedName() types.NamespacedName {
return k8s.NamespacedName(httpRoute.route)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/gateway/routeutils/listener_attachment_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
}
if !namespaceOK {
deferredRouteReconciler.Enqueue(
GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route, gw),
GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
)

return false, nil
Expand All @@ -51,7 +51,7 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
kindOK := attachmentHelper.kindCheck(listener, route)
if !kindOK {
deferredRouteReconciler.Enqueue(
GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route, gw),
GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
)
return false, nil
}
Expand All @@ -65,7 +65,7 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
if !hostnameOK {
// hostname is not ok, print out gwName and gwNamespace test-gw-alb gateway-alb
deferredRouteReconciler.Enqueue(
GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route, gw),
GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
)
return false, nil
}
Expand Down
17 changes: 11 additions & 6 deletions pkg/gateway/routeutils/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,22 +92,27 @@ func (l *loaderImpl) LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway,
return nil, err
}

// 3. Load the underlying resource(s) for each route that is configured.
loadedRoute, err := l.loadChildResources(ctx, mappedRoutes)
if err != nil {
// TODO: add route status update for those with error
return nil, err
}

// update status for accepted routes
for _, routeList := range mappedRoutes {
for _, routeList := range loadedRoute {
for _, route := range routeList {
deferredRouteReconciler.Enqueue(
GenerateRouteData(true, true, string(gwv1.RouteConditionAccepted), RouteStatusInfoAcceptedMessage, route, gw),
GenerateRouteData(true, true, string(gwv1.RouteConditionAccepted), RouteStatusInfoAcceptedMessage, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
)
}
}

// 3. Load the underlying resource(s) for each route that is configured.
return l.loadChildResources(ctx, mappedRoutes)
return loadedRoute, nil
}

// loadChildResources responsible for loading all resources that a route descriptor references.
func (l *loaderImpl) loadChildResources(ctx context.Context, preloadedRoutes map[int][]preLoadRouteDescriptor) (map[int32][]RouteDescriptor, error) {
// Cache to reduce duplicate route look ups.
// Cache to reduce duplicate route lookups.
// Kind -> [NamespacedName:Previously Loaded Descriptor]
resourceCache := make(map[string]RouteDescriptor)

Expand Down
Loading
Loading