Skip to content

Commit ab38764

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4483 from shuqz/route-status
[feat gw-api]modify route status update per parentRef
2 parents a175162 + 2bb935c commit ab38764

20 files changed

+514
-139
lines changed

‎controllers/gateway/route_reconciler.go‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -156,35 +156,34 @@ func (d *routeReconcilerImpl) updateRouteStatus(route client.Object, routeData r
156156
originalRouteStatus = r.Status.Parents
157157
ParentRefs = r.Spec.ParentRefs
158158
}
159+
routeNamespace := route.GetNamespace()
159160

160161
// set conditions
161162
var newRouteStatus []gwv1.RouteParentStatus
162-
originalRouteStatusMap := createOriginalRouteStatusMap(originalRouteStatus)
163-
163+
originalRouteStatusMap := createOriginalRouteStatusMap(originalRouteStatus, routeNamespace)
164164
for _, parentRef := range ParentRefs {
165165
newRouteParentStatus := gwv1.RouteParentStatus{
166166
ParentRef: parentRef,
167167
ControllerName: gwv1.GatewayController(controllerName),
168168
Conditions: []metav1.Condition{},
169169
}
170170
// if status related to parentRef exists, keep the condition first
171-
if status, exists := originalRouteStatusMap[getParentStatusKey(parentRef)]; exists {
171+
if status, exists := originalRouteStatusMap[getParentStatusKey(parentRef, routeNamespace)]; exists {
172172
newRouteParentStatus.Conditions = status.Conditions
173173
}
174-
// Namespace is the namespace of the referent. When unspecified, this refers to the local namespace of the Route.
175-
namespace := route.GetNamespace()
176-
if parentRef.Namespace != nil {
177-
namespace = string(*parentRef.Namespace)
178-
}
174+
175+
// Generate key for routeData's parentRef
176+
routeDataParentRefKey := getParentRefKeyFromRouteData(routeData.ParentRef, routeData.RouteMetadata.RouteNamespace)
179177

180178
// do not allow backward generation update, Accepted and ResolvedRef always have same generation based on our implementation
181179
if (len(newRouteParentStatus.Conditions) != 0 && newRouteParentStatus.Conditions[0].ObservedGeneration <= routeData.RouteMetadata.RouteGeneration) || len(newRouteParentStatus.Conditions) == 0 {
182180
// for a given parentRef, if it has a statusInfo, this means condition is updated, override route condition based on route status info
183-
if namespace == routeData.ParentRefGateway.Namespace && string(parentRef.Name) == routeData.ParentRefGateway.Name {
181+
parentRefKey := getParentStatusKey(parentRef, routeNamespace)
182+
if parentRefKey == routeDataParentRefKey {
184183
d.setConditionsWithRouteStatusInfo(route, &newRouteParentStatus, routeData.RouteStatusInfo)
185184
}
186185

187-
// resolve ref Gateway, if parentRef does not have namespace, getting it from Route
186+
// handle parentRefNotExist: resolve ref Gateway, if parentRef does not have namespace, getting it from Route
188187
if _, err := d.resolveRefGateway(parentRef, route.GetNamespace()); err != nil {
189188
// set conditions if resolvedRef = false
190189
d.setConditionsBasedOnResolveRefGateway(route, &newRouteParentStatus, err)
@@ -283,9 +282,9 @@ func (d *routeReconcilerImpl) setConditionsBasedOnResolveRefGateway(route client
283282
},
284283
{
285284
Type: string(gwv1.RouteConditionResolvedRefs),
286-
Status: metav1.ConditionFalse,
287-
Reason: routeutils.RouteStatusInfoRejectedParentRefNotExist,
288-
Message: resolveErr.Error(),
285+
Status: metav1.ConditionTrue,
286+
Reason: string(gwv1.RouteConditionResolvedRefs),
287+
Message: "",
289288
LastTransitionTime: timeNow,
290289
ObservedGeneration: route.GetGeneration(),
291290
},
@@ -310,12 +309,12 @@ func (d *routeReconcilerImpl) isRouteStatusIdentical(routeOld client.Object, rou
310309

311310
// build maps, key is a string which is combined from parentRef fields
312311
for _, status := range routeOldStatus {
313-
key := getParentStatusKey(status.ParentRef)
312+
key := getParentStatusKey(status.ParentRef, routeOld.GetNamespace())
314313
oldStatusMap[key] = status
315314
}
316315

317316
for _, status := range routeNewStatus {
318-
key := getParentStatusKey(status.ParentRef)
317+
key := getParentStatusKey(status.ParentRef, route.GetNamespace())
319318
newStatusMap[key] = status
320319
}
321320

@@ -359,11 +358,27 @@ func getRouteStatus(route client.Object) []gwv1.RouteParentStatus {
359358
return routeStatus
360359
}
361360

361+
// Helper function to generate key from RouteData's ParentReference, use same format as getParentStatusKey
362+
func getParentRefKeyFromRouteData(parentRef gwv1.ParentReference, routeNamespace string) string {
363+
return getParentStatusKey(parentRef, routeNamespace)
364+
}
365+
362366
// Helper function to generate a unique key for a RouteParentStatus
363-
func getParentStatusKey(ref gwv1.ParentReference) string {
367+
func getParentStatusKey(ref gwv1.ParentReference, routeNamespace string) string {
368+
group := ""
369+
if ref.Group != nil {
370+
group = string(*ref.Group)
371+
}
372+
kind := ""
373+
if ref.Kind != nil {
374+
kind = string(*ref.Kind)
375+
}
376+
364377
namespace := ""
365378
if ref.Namespace != nil {
366379
namespace = string(*ref.Namespace)
380+
} else {
381+
namespace = routeNamespace
367382
}
368383

369384
sectionName := ""
@@ -376,16 +391,6 @@ func getParentStatusKey(ref gwv1.ParentReference) string {
376391
port = strconv.Itoa(int(*ref.Port))
377392
}
378393

379-
group := ""
380-
if ref.Group != nil {
381-
group = string(*ref.Group)
382-
}
383-
384-
kind := ""
385-
if ref.Kind != nil {
386-
kind = string(*ref.Kind)
387-
}
388-
389394
key := fmt.Sprintf("%s/%s/%s/%s/%s/%s",
390395
group,
391396
kind,
@@ -427,10 +432,10 @@ func areConditionsEqual(oldConditions, newConditions []metav1.Condition) bool {
427432
return true
428433
}
429434

430-
func createOriginalRouteStatusMap(originalRouteStatus []gwv1.RouteParentStatus) map[string]gwv1.RouteParentStatus {
435+
func createOriginalRouteStatusMap(originalRouteStatus []gwv1.RouteParentStatus, routeNamespace string) map[string]gwv1.RouteParentStatus {
431436
originalStatusMap := make(map[string]gwv1.RouteParentStatus)
432437
for i := range originalRouteStatus {
433-
key := getParentStatusKey(originalRouteStatus[i].ParentRef)
438+
key := getParentStatusKey(originalRouteStatus[i].ParentRef, routeNamespace)
434439
originalStatusMap[key] = originalRouteStatus[i]
435440
}
436441
return originalStatusMap

‎controllers/gateway/route_reconciler_test.go‎

Lines changed: 91 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ func Test_getParentStatusKey(t *testing.T) {
289289
portPtr := func(p gwv1.PortNumber) *gwv1.PortNumber {
290290
return &p
291291
}
292+
routeNamespace := "route-namespace"
292293

293294
tests := []struct {
294295
name string
@@ -309,6 +310,19 @@ func Test_getParentStatusKey(t *testing.T) {
309310
},
310311
want: "networking.k8s.io/Gateway/test-namespace/test-gateway/test-section/80",
311312
},
313+
{
314+
name: "no namespace should use route namespace",
315+
status: gwv1.RouteParentStatus{
316+
ParentRef: gwv1.ParentReference{
317+
Group: (*gwv1.Group)(ptr.To("networking.k8s.io")),
318+
Kind: (*gwv1.Kind)(ptr.To("Gateway")),
319+
Name: "test-gateway",
320+
SectionName: (*gwv1.SectionName)(ptr.To("test-section")),
321+
Port: portPtr(80),
322+
},
323+
},
324+
want: "networking.k8s.io/Gateway/route-namespace/test-gateway/test-section/80",
325+
},
312326
{
313327
name: "no section or port",
314328
status: gwv1.RouteParentStatus{
@@ -325,7 +339,7 @@ func Test_getParentStatusKey(t *testing.T) {
325339

326340
for _, tt := range tests {
327341
t.Run(tt.name, func(t *testing.T) {
328-
got := getParentStatusKey(tt.status.ParentRef)
342+
got := getParentStatusKey(tt.status.ParentRef, routeNamespace)
329343
if got != tt.want {
330344
t.Errorf("getParentStatusKey() = %v, want %v", got, tt.want)
331345
}
@@ -339,7 +353,7 @@ func TestEnqueue(t *testing.T) {
339353
routeData routeutils.RouteData
340354
routeStatusInfo routeutils.RouteStatusInfo
341355
routeMetadataDescriptor routeutils.RouteMetadata
342-
parentRefGateway routeutils.ParentRefGateway
356+
parentRefGateway gwv1.ParentReference
343357

344358
validateEnqueued func(t *testing.T, enqueued []routeutils.EnqueuedType) // Use the type here
345359
}{
@@ -355,7 +369,7 @@ func TestEnqueue(t *testing.T) {
355369
RouteNamespace: "test-namespace",
356370
RouteKind: "test-kind",
357371
},
358-
ParentRefGateway: routeutils.ParentRefGateway{},
372+
ParentRef: gwv1.ParentReference{},
359373
},
360374
validateEnqueued: func(t *testing.T, enqueued []routeutils.EnqueuedType) {
361375
assert.Len(t, enqueued, 1)
@@ -378,7 +392,7 @@ func TestEnqueue(t *testing.T) {
378392
RouteNamespace: "test-namespace",
379393
RouteKind: "test-kind",
380394
},
381-
ParentRefGateway: routeutils.ParentRefGateway{},
395+
ParentRef: gwv1.ParentReference{},
382396
},
383397
validateEnqueued: func(t *testing.T, enqueued []routeutils.EnqueuedType) {
384398
assert.Len(t, enqueued, 1)
@@ -400,7 +414,7 @@ func TestEnqueue(t *testing.T) {
400414
RouteNamespace: "test-namespace",
401415
RouteKind: "test-kind",
402416
},
403-
ParentRefGateway: routeutils.ParentRefGateway{},
417+
ParentRef: gwv1.ParentReference{},
404418
},
405419
validateEnqueued: func(t *testing.T, enqueued []routeutils.EnqueuedType) {
406420
assert.Len(t, enqueued, 1)
@@ -425,6 +439,7 @@ func TestEnqueue(t *testing.T) {
425439
}
426440

427441
func Test_updateRouteStatus(t *testing.T) {
442+
testNamespace := gwv1.Namespace("test-namespace")
428443
tests := []struct {
429444
name string
430445
route client.Object
@@ -449,9 +464,9 @@ func Test_updateRouteStatus(t *testing.T) {
449464
Reason: string(gwv1.RouteConditionAccepted),
450465
Message: "route accepted",
451466
},
452-
ParentRefGateway: routeutils.ParentRefGateway{
467+
ParentRef: gwv1.ParentReference{
453468
Name: "test-gateway",
454-
Namespace: "test-namespace",
469+
Namespace: &testNamespace,
455470
},
456471
},
457472
validateResult: func(t *testing.T, route client.Object) {
@@ -718,3 +733,72 @@ func findCondition(conditions []metav1.Condition, conditionType string) *metav1.
718733
}
719734
return nil
720735
}
736+
737+
func Test_getParentRefKeyFromRouteData(t *testing.T) {
738+
testNamespace := gwv1.Namespace("test-namespace")
739+
testGroup := gwv1.Group("test-group")
740+
testKind := gwv1.Kind("test-kind")
741+
routeNamespace := "route-namespace"
742+
tests := []struct {
743+
name string
744+
gatewayRef gwv1.ParentReference
745+
want string
746+
}{
747+
{
748+
name: "all fields provided",
749+
gatewayRef: gwv1.ParentReference{
750+
Group: &testGroup,
751+
Kind: &testKind,
752+
Namespace: &testNamespace,
753+
Name: "test-gateway",
754+
SectionName: ptr.To(gwv1.SectionName("test-section")),
755+
Port: ptr.To(gwv1.PortNumber(80)),
756+
},
757+
want: "test-group/test-kind/test-namespace/test-gateway/test-section/80",
758+
},
759+
{
760+
name: "no namespace provided",
761+
gatewayRef: gwv1.ParentReference{
762+
Group: &testGroup,
763+
Kind: &testKind,
764+
Name: "test-gateway",
765+
SectionName: ptr.To(gwv1.SectionName("test-section")),
766+
Port: ptr.To(gwv1.PortNumber(80)),
767+
},
768+
want: "test-group/test-kind/route-namespace/test-gateway/test-section/80",
769+
},
770+
{
771+
name: "no section or port",
772+
gatewayRef: gwv1.ParentReference{
773+
Namespace: &testNamespace,
774+
Name: "test-gateway",
775+
},
776+
want: "//test-namespace/test-gateway//",
777+
},
778+
{
779+
name: "with port only",
780+
gatewayRef: gwv1.ParentReference{
781+
Namespace: &testNamespace,
782+
Name: "test-gateway",
783+
Port: ptr.To(gwv1.PortNumber(443)),
784+
},
785+
want: "//test-namespace/test-gateway//443",
786+
},
787+
{
788+
name: "with section only",
789+
gatewayRef: gwv1.ParentReference{
790+
Namespace: &testNamespace,
791+
Name: "test-gateway",
792+
SectionName: ptr.To(gwv1.SectionName("https")),
793+
},
794+
want: "//test-namespace/test-gateway/https/",
795+
},
796+
}
797+
798+
for _, tt := range tests {
799+
t.Run(tt.name, func(t *testing.T) {
800+
got := getParentRefKeyFromRouteData(tt.gatewayRef, routeNamespace)
801+
assert.Equal(t, tt.want, got)
802+
})
803+
}
804+
}

‎pkg/gateway/routeutils/descriptor.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
type routeMetadataDescriptor interface {
1616
GetRouteNamespacedName() types.NamespacedName
1717
GetRouteKind() RouteKind
18+
GetRouteIdentifier() string
1819
GetHostnames() []gwv1.Hostname
1920
GetParentRefs() []gwv1.ParentReference
2021
GetRawRoute() interface{}

‎pkg/gateway/routeutils/grpc.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ func (grpcRoute *grpcRouteDescription) GetRouteNamespacedName() types.Namespaced
9898
return k8s.NamespacedName(grpcRoute.route)
9999
}
100100

101+
func (grpcRoute *grpcRouteDescription) GetRouteIdentifier() string {
102+
return string(grpcRoute.GetRouteKind()) + "-" + grpcRoute.GetRouteNamespacedName().String()
103+
}
104+
101105
func convertGRPCRoute(r gwv1.GRPCRoute) *grpcRouteDescription {
102106
return &grpcRouteDescription{route: &r, ruleAccumulator: defaultGRPCRuleAccumulator}
103107
}

‎pkg/gateway/routeutils/http.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ func (httpRoute *httpRouteDescription) GetRouteNamespacedName() types.Namespaced
103103
return k8s.NamespacedName(httpRoute.route)
104104
}
105105

106+
func (httpRoute *httpRouteDescription) GetRouteIdentifier() string {
107+
return string(httpRoute.GetRouteKind()) + "-" + httpRoute.GetRouteNamespacedName().String()
108+
}
109+
106110
func (httpRoute *httpRouteDescription) GetBackendRefs() []gwv1.BackendRef {
107111
backendRefs := make([]gwv1.BackendRef, 0)
108112
if httpRoute.route.Spec.Rules != nil {

‎pkg/gateway/routeutils/listener_attachment_helper.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
// listenerAttachmentHelper is an internal utility interface that can be used to determine if a listener will allow
1616
// a route to attach to it.
1717
type listenerAttachmentHelper interface {
18-
listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error)
18+
listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, matchedParentRef *gwv1.ParentReference, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error)
1919
}
2020

2121
var _ listenerAttachmentHelper = &listenerAttachmentHelperImpl{}
@@ -36,21 +36,23 @@ func newListenerAttachmentHelper(k8sClient client.Client, logger logr.Logger) li
3636
// listenerAllowsAttachment utility method to determine if a listener will allow a route to connect using
3737
// Gateway API rules to determine compatibility between lister and route.
3838
// Returns: (compatibleHostnames, allowed, failedRouteData, error)
39-
func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error) {
39+
func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, matchedParentRef *gwv1.ParentReference, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error) {
40+
port := matchedParentRef.Port
41+
sectionName := matchedParentRef.SectionName
4042
// check namespace
4143
namespaceOK, err := attachmentHelper.namespaceCheck(ctx, gw, listener, route)
4244
if err != nil {
4345
return nil, false, nil, err
4446
}
4547
if !namespaceOK {
46-
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
48+
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw, port, sectionName)
4749
return nil, false, &rd, nil
4850
}
4951

5052
// check kind
5153
kindOK := attachmentHelper.kindCheck(listener, route)
5254
if !kindOK {
53-
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
55+
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw, port, sectionName)
5456
return nil, false, &rd, nil
5557
}
5658

@@ -63,7 +65,7 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
6365
return nil, false, nil, err
6466
}
6567
if !hostnameOK {
66-
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
68+
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw, port, sectionName)
6769
return nil, false, &rd, nil
6870
}
6971
}
@@ -73,7 +75,7 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
7375
hostnameUniquenessOK, conflictRoute := attachmentHelper.crossServingHostnameUniquenessCheck(route, hostnamesFromHttpRoutes, hostnamesFromGrpcRoutes)
7476
if !hostnameUniquenessOK {
7577
message := fmt.Sprintf("HTTPRoute and GRPCRoute have overlap hostname, attachment is rejected. Conflict route: %s", conflictRoute)
76-
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), message, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
78+
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), message, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw, port, sectionName)
7779
return nil, false, &rd, nil
7880
}
7981
}

0 commit comments

Comments
 (0)