Skip to content

Commit 4c5857e

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4210 from shuqz/main
[feat gw-api]handle resolvedRef for route status update and update ho…
2 parents 372f5b9 + dd7b733 commit 4c5857e

File tree

15 files changed

+465
-32
lines changed

15 files changed

+465
-32
lines changed

‎apis/gateway/v1beta1/targetgroupconfig_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ const (
129129
ProtocolTCP_UDP Protocol = "TCP_UDP"
130130
)
131131

132-
// +kubebuilder:validation:Enum=http1;http2;grpc
132+
// +kubebuilder:validation:Enum=HTTP1;HTTP2;GRPC
133133
type ProtocolVersion string
134134

135135
const (

‎config/crd/gateway/gateway-crds.yaml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -491,9 +491,9 @@ spec:
491491
description: protocolVersion [HTTP/HTTPS protocol] The protocol
492492
version. The possible values are GRPC , HTTP1 and HTTP2
493493
enum:
494-
- http1
495-
- http2
496-
- grpc
494+
- HTTP1
495+
- HTTP2
496+
- GRPC
497497
type: string
498498
tags:
499499
description: Tags defines list of Tags on target group.
@@ -694,9 +694,9 @@ spec:
694694
description: protocolVersion [HTTP/HTTPS protocol] The protocol
695695
version. The possible values are GRPC , HTTP1 and HTTP2
696696
enum:
697-
- http1
698-
- http2
699-
- grpc
697+
- HTTP1
698+
- HTTP2
699+
- GRPC
700700
type: string
701701
tags:
702702
description: Tags defines list of Tags on target group.

‎config/crd/gateway/gateway.k8s.aws_targetgroupconfigurations.yaml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,9 @@ spec:
186186
description: protocolVersion [HTTP/HTTPS protocol] The protocol
187187
version. The possible values are GRPC , HTTP1 and HTTP2
188188
enum:
189-
- http1
190-
- http2
191-
- grpc
189+
- HTTP1
190+
- HTTP2
191+
- GRPC
192192
type: string
193193
tags:
194194
description: Tags defines list of Tags on target group.
@@ -389,9 +389,9 @@ spec:
389389
description: protocolVersion [HTTP/HTTPS protocol] The protocol
390390
version. The possible values are GRPC , HTTP1 and HTTP2
391391
enum:
392-
- http1
393-
- http2
394-
- grpc
392+
- HTTP1
393+
- HTTP2
394+
- GRPC
395395
type: string
396396
tags:
397397
description: Tags defines list of Tags on target group.

‎controllers/gateway/gateway_controller.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R
196196
mergedLbConfig, err := r.cfgResolver.getLoadBalancerConfigForGateway(ctx, r.k8sClient, gw, gwClass)
197197

198198
if err != nil {
199-
statusErr := r.updateGatewayStatusFailure(ctx, gw, gwv1.GatewayReasonInvalid, err)
199+
statusErr := r.updateGatewayStatusFailure(ctx, gw, gwv1.GatewayReasonInvalid, err.Error())
200200
if statusErr != nil {
201201
r.logger.Error(statusErr, "Unable to update gateway status on failure to retrieve attached config")
202202
}
@@ -208,7 +208,7 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R
208208
if err != nil {
209209
var loaderErr routeutils.LoaderError
210210
if errors.As(err, &loaderErr) {
211-
statusErr := r.updateGatewayStatusFailure(ctx, gw, loaderErr.GetGatewayReason(), loaderErr)
211+
statusErr := r.updateGatewayStatusFailure(ctx, gw, loaderErr.GetGatewayReason(), loaderErr.GetGatewayMessage())
212212
if statusErr != nil {
213213
r.logger.Error(statusErr, "Unable to update gateway status on failure to build routes")
214214
}
@@ -354,10 +354,10 @@ func (r *gatewayReconciler) updateGatewayStatusSuccess(ctx context.Context, lbSt
354354
return nil
355355
}
356356

357-
func (r *gatewayReconciler) updateGatewayStatusFailure(ctx context.Context, gw *gwv1.Gateway, reason gwv1.GatewayConditionReason, err error) error {
357+
func (r *gatewayReconciler) updateGatewayStatusFailure(ctx context.Context, gw *gwv1.Gateway, reason gwv1.GatewayConditionReason, errMessage string) error {
358358
gwOld := gw.DeepCopy()
359359

360-
needPatch := r.gatewayConditionUpdater(gw, string(gwv1.GatewayConditionAccepted), metav1.ConditionFalse, string(reason), err.Error())
360+
needPatch := r.gatewayConditionUpdater(gw, string(gwv1.GatewayConditionAccepted), metav1.ConditionFalse, string(reason), errMessage)
361361

362362
if needPatch {
363363
if err := r.k8sClient.Status().Patch(ctx, gw, client.MergeFrom(gwOld)); err != nil {

‎controllers/gateway/route_reconciler_test.go‎

Lines changed: 294 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"sigs.k8s.io/controller-runtime/pkg/log"
1313
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
1414
"testing"
15+
"time"
1516
)
1617

1718
func TestDeferredReconcilerConstructor(t *testing.T) {
@@ -421,3 +422,296 @@ func TestEnqueue(t *testing.T) {
421422
})
422423
}
423424
}
425+
426+
func Test_updateRouteStatus(t *testing.T) {
427+
tests := []struct {
428+
name string
429+
route client.Object
430+
routeData routeutils.RouteData
431+
validateResult func(t *testing.T, route client.Object)
432+
}{
433+
{
434+
name: "update HTTPRoute status - condition accepted",
435+
route: &gwv1.HTTPRoute{
436+
ObjectMeta: metav1.ObjectMeta{
437+
Name: "test-route",
438+
Namespace: "test-namespace",
439+
},
440+
Spec: gwv1.HTTPRouteSpec{
441+
Hostnames: []gwv1.Hostname{"example.com"},
442+
},
443+
},
444+
routeData: routeutils.RouteData{
445+
RouteStatusInfo: routeutils.RouteStatusInfo{
446+
Accepted: true,
447+
ResolvedRefs: true,
448+
Reason: string(gwv1.RouteConditionAccepted),
449+
Message: "route accepted",
450+
},
451+
ParentRefGateway: routeutils.ParentRefGateway{
452+
Name: "test-gateway",
453+
Namespace: "test-namespace",
454+
},
455+
},
456+
validateResult: func(t *testing.T, route client.Object) {
457+
httpRoute := route.(*gwv1.HTTPRoute)
458+
if httpRoute.Status.Parents == nil {
459+
assert.Len(t, httpRoute.Status.Parents, 0)
460+
}
461+
},
462+
},
463+
}
464+
465+
for _, tt := range tests {
466+
t.Run(tt.name, func(t *testing.T) {
467+
logger := logr.New(&log.NullLogSink{})
468+
reconciler := &routeReconcilerImpl{
469+
logger: logger,
470+
}
471+
err := reconciler.updateRouteStatus(tt.route, tt.routeData)
472+
assert.NoError(t, err)
473+
if tt.validateResult != nil {
474+
tt.validateResult(t, tt.route)
475+
}
476+
})
477+
}
478+
}
479+
480+
func Test_setConditionsWithRouteStatusInfo(t *testing.T) {
481+
route := &gwv1.HTTPRoute{
482+
ObjectMeta: metav1.ObjectMeta{
483+
Generation: 1,
484+
},
485+
}
486+
487+
tests := []struct {
488+
name string
489+
info routeutils.RouteStatusInfo
490+
validateResult func(t *testing.T, conditions []metav1.Condition)
491+
}{
492+
{
493+
name: "accepted true and resolvedRef true",
494+
info: routeutils.RouteStatusInfo{
495+
Accepted: true,
496+
ResolvedRefs: true,
497+
Reason: string(gwv1.RouteConditionAccepted),
498+
Message: "route accepted",
499+
},
500+
validateResult: func(t *testing.T, conditions []metav1.Condition) {
501+
assert.Len(t, conditions, 2)
502+
acceptedCondition := findCondition(conditions, string(gwv1.RouteConditionAccepted))
503+
assert.NotNil(t, acceptedCondition)
504+
assert.Equal(t, metav1.ConditionTrue, acceptedCondition.Status)
505+
506+
resolvedRefCondition := findCondition(conditions, string(gwv1.RouteConditionResolvedRefs))
507+
assert.NotNil(t, resolvedRefCondition)
508+
assert.Equal(t, metav1.ConditionTrue, resolvedRefCondition.Status)
509+
},
510+
},
511+
{
512+
name: "accepted false and resolvedRef true",
513+
info: routeutils.RouteStatusInfo{
514+
Accepted: false,
515+
ResolvedRefs: true,
516+
Reason: string(gwv1.RouteReasonNotAllowedByListeners),
517+
Message: "route not allowed by listeners",
518+
},
519+
validateResult: func(t *testing.T, conditions []metav1.Condition) {
520+
assert.Len(t, conditions, 2)
521+
acceptedCondition := findCondition(conditions, string(gwv1.RouteConditionAccepted))
522+
assert.NotNil(t, acceptedCondition)
523+
assert.Equal(t, metav1.ConditionFalse, acceptedCondition.Status)
524+
525+
resolvedRefCondition := findCondition(conditions, string(gwv1.RouteConditionResolvedRefs))
526+
assert.NotNil(t, resolvedRefCondition)
527+
assert.Equal(t, metav1.ConditionTrue, resolvedRefCondition.Status)
528+
},
529+
},
530+
{
531+
name: "accepted false and resolvedRef false",
532+
info: routeutils.RouteStatusInfo{
533+
Accepted: false,
534+
ResolvedRefs: false,
535+
Reason: string(gwv1.RouteReasonBackendNotFound),
536+
Message: "backend not found",
537+
},
538+
validateResult: func(t *testing.T, conditions []metav1.Condition) {
539+
assert.Len(t, conditions, 2)
540+
acceptedCondition := findCondition(conditions, string(gwv1.RouteConditionAccepted))
541+
assert.NotNil(t, acceptedCondition)
542+
assert.Equal(t, metav1.ConditionFalse, acceptedCondition.Status)
543+
544+
resolvedRefCondition := findCondition(conditions, string(gwv1.RouteConditionResolvedRefs))
545+
assert.NotNil(t, resolvedRefCondition)
546+
assert.Equal(t, metav1.ConditionFalse, resolvedRefCondition.Status)
547+
},
548+
},
549+
}
550+
551+
for _, tt := range tests {
552+
t.Run(tt.name, func(t *testing.T) {
553+
logger := logr.New(&log.NullLogSink{})
554+
reconciler := &routeReconcilerImpl{
555+
logger: logger,
556+
}
557+
parentStatus := &gwv1.RouteParentStatus{}
558+
reconciler.setConditionsWithRouteStatusInfo(route, parentStatus, tt.info)
559+
if tt.validateResult != nil {
560+
tt.validateResult(t, parentStatus.Conditions)
561+
}
562+
})
563+
}
564+
}
565+
566+
func Test_areConditionsEqual(t *testing.T) {
567+
tests := []struct {
568+
name string
569+
oldCon []metav1.Condition
570+
newCon []metav1.Condition
571+
expected bool
572+
}{
573+
{
574+
name: "same conditions - true",
575+
oldCon: []metav1.Condition{
576+
{
577+
Type: string(gwv1.RouteConditionAccepted),
578+
Status: metav1.ConditionTrue,
579+
},
580+
{
581+
Type: string(gwv1.RouteConditionResolvedRefs),
582+
Status: metav1.ConditionTrue,
583+
},
584+
},
585+
newCon: []metav1.Condition{
586+
{
587+
Type: string(gwv1.RouteConditionAccepted),
588+
Status: metav1.ConditionTrue,
589+
},
590+
{
591+
Type: string(gwv1.RouteConditionResolvedRefs),
592+
Status: metav1.ConditionTrue,
593+
},
594+
},
595+
expected: true,
596+
},
597+
{
598+
name: "different conditions - false",
599+
oldCon: []metav1.Condition{
600+
{
601+
Type: string(gwv1.RouteConditionAccepted),
602+
Status: metav1.ConditionTrue,
603+
},
604+
{
605+
Type: string(gwv1.RouteConditionResolvedRefs),
606+
Status: metav1.ConditionTrue,
607+
},
608+
},
609+
newCon: []metav1.Condition{
610+
{
611+
Type: string(gwv1.RouteConditionAccepted),
612+
Status: metav1.ConditionFalse,
613+
},
614+
{
615+
Type: string(gwv1.RouteConditionResolvedRefs),
616+
Status: metav1.ConditionTrue,
617+
},
618+
},
619+
expected: false,
620+
},
621+
{
622+
name: "different conditions on Reason - false",
623+
oldCon: []metav1.Condition{
624+
{
625+
Type: string(gwv1.RouteConditionAccepted),
626+
Status: metav1.ConditionTrue,
627+
Reason: "good",
628+
},
629+
{
630+
Type: string(gwv1.RouteConditionResolvedRefs),
631+
Status: metav1.ConditionTrue,
632+
},
633+
},
634+
newCon: []metav1.Condition{
635+
{
636+
Type: string(gwv1.RouteConditionAccepted),
637+
Status: metav1.ConditionTrue,
638+
Reason: "test-good",
639+
},
640+
{
641+
Type: string(gwv1.RouteConditionResolvedRefs),
642+
Status: metav1.ConditionTrue,
643+
},
644+
},
645+
expected: false,
646+
},
647+
{
648+
name: "different conditions on LastTransitionTime - true",
649+
oldCon: []metav1.Condition{
650+
{
651+
Type: string(gwv1.RouteConditionAccepted),
652+
Status: metav1.ConditionTrue,
653+
LastTransitionTime: metav1.NewTime(time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)),
654+
},
655+
{
656+
Type: string(gwv1.RouteConditionResolvedRefs),
657+
Status: metav1.ConditionTrue,
658+
},
659+
},
660+
newCon: []metav1.Condition{
661+
{
662+
Type: string(gwv1.RouteConditionAccepted),
663+
Status: metav1.ConditionTrue,
664+
LastTransitionTime: metav1.NewTime(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)),
665+
},
666+
{
667+
Type: string(gwv1.RouteConditionResolvedRefs),
668+
Status: metav1.ConditionTrue,
669+
},
670+
},
671+
expected: true,
672+
},
673+
{
674+
name: "different conditions on ObservedGeneration - true",
675+
oldCon: []metav1.Condition{
676+
{
677+
Type: string(gwv1.RouteConditionAccepted),
678+
Status: metav1.ConditionTrue,
679+
ObservedGeneration: 1,
680+
},
681+
{
682+
Type: string(gwv1.RouteConditionResolvedRefs),
683+
Status: metav1.ConditionTrue,
684+
},
685+
},
686+
newCon: []metav1.Condition{
687+
{
688+
Type: string(gwv1.RouteConditionAccepted),
689+
Status: metav1.ConditionTrue,
690+
ObservedGeneration: 2,
691+
},
692+
{
693+
Type: string(gwv1.RouteConditionResolvedRefs),
694+
Status: metav1.ConditionTrue,
695+
},
696+
},
697+
expected: true,
698+
},
699+
}
700+
701+
for _, tt := range tests {
702+
t.Run(tt.name, func(t *testing.T) {
703+
result := areConditionsEqual(tt.oldCon, tt.newCon)
704+
assert.Equal(t, tt.expected, result)
705+
})
706+
}
707+
}
708+
709+
// helper function
710+
func findCondition(conditions []metav1.Condition, conditionType string) *metav1.Condition {
711+
for _, condition := range conditions {
712+
if condition.Type == conditionType {
713+
return &condition
714+
}
715+
}
716+
return nil
717+
}

0 commit comments

Comments
 (0)