Skip to content

Commit b67ffc3

Browse files
committed
fix: gateway router should wait for accepted condition
It can take some time for changes to propagate for cloud load balancers, so flagger should ensure the route changes are current before proceeding with any more.
1 parent 6f165a1 commit b67ffc3

File tree

3 files changed

+125
-1
lines changed

3 files changed

+125
-1
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ require (
2929
k8s.io/client-go v0.32.3
3030
k8s.io/code-generator v0.31.4
3131
k8s.io/klog/v2 v2.130.1
32+
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
3233
knative.dev/serving v0.44.0
3334
)
3435

@@ -99,7 +100,6 @@ require (
99100
gopkg.in/yaml.v3 v3.0.1 // indirect
100101
k8s.io/gengo/v2 v2.0.0-20240911193312-2b36238f13e9 // indirect
101102
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f // indirect
102-
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect
103103
knative.dev/networking v0.0.0-20250117155906-67d1c274ba6a // indirect
104104
knative.dev/pkg v0.0.0-20250117084104-c43477f0052b // indirect
105105
sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect

pkg/router/gateway_api.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,26 @@ func (gwr *GatewayAPIRouter) GetRoutes(canary *flaggerv1.Canary) (
273273
err = fmt.Errorf("HTTPRoute %s.%s get error: %w", apexSvcName, hrNamespace, err)
274274
return
275275
}
276+
277+
currentGeneration := httpRoute.GetGeneration()
278+
for _, parentRef := range httpRoute.Spec.CommonRouteSpec.ParentRefs {
279+
for _, parentStatus := range httpRoute.Status.Parents {
280+
if !reflect.DeepEqual(parentStatus.ParentRef, parentRef) {
281+
continue
282+
}
283+
284+
for _, condition := range parentStatus.Conditions {
285+
if condition.Type == string(v1.RouteConditionAccepted) && (condition.Status != metav1.ConditionTrue || condition.ObservedGeneration < currentGeneration) {
286+
err = fmt.Errorf(
287+
"HTTPRoute %s.%s parent %s is not ready (status: %s, observed generation: %d, current generation: %d)",
288+
apexSvcName, hrNamespace, parentRef.Name, string(condition.Status), condition.ObservedGeneration, currentGeneration,
289+
)
290+
return 0, 0, false, err
291+
}
292+
}
293+
}
294+
}
295+
276296
var weightedRule *v1.HTTPRouteRule
277297
for _, rule := range httpRoute.Spec.Rules {
278298
// If session affinity is enabled, then we are only interested in the rule

pkg/router/gateway_api_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/stretchr/testify/assert"
3232
"github.com/stretchr/testify/require"
3333
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
34+
"k8s.io/utils/ptr"
3435
)
3536

3637
func TestGatewayAPIRouter_Reconcile(t *testing.T) {
@@ -516,3 +517,106 @@ func TestGatewayAPIRouter_makeFilters(t *testing.T) {
516517
assert.Equal(t, "", filtersDiff)
517518
}
518519
}
520+
521+
func TestGatewayAPIRouter_GetRoutes(t *testing.T) {
522+
canary := newTestGatewayAPICanary()
523+
mocks := newFixture(canary)
524+
router := &GatewayAPIRouter{
525+
gatewayAPIClient: mocks.meshClient,
526+
kubeClient: mocks.kubeClient,
527+
logger: mocks.logger,
528+
}
529+
530+
httpRoute := &v1.HTTPRoute{
531+
ObjectMeta: metav1.ObjectMeta{
532+
Name: "podinfo",
533+
Generation: 1,
534+
},
535+
Spec: v1.HTTPRouteSpec{
536+
Rules: []v1.HTTPRouteRule{
537+
{
538+
BackendRefs: []v1.HTTPBackendRef{
539+
{
540+
BackendRef: v1.BackendRef{
541+
BackendObjectReference: v1.BackendObjectReference{
542+
Name: "podinfo-canary",
543+
},
544+
Weight: ptr.To(int32(10)),
545+
},
546+
},
547+
{
548+
BackendRef: v1.BackendRef{
549+
BackendObjectReference: v1.BackendObjectReference{
550+
Name: "podinfo-primary",
551+
},
552+
Weight: ptr.To(int32(90)),
553+
},
554+
},
555+
},
556+
},
557+
},
558+
CommonRouteSpec: v1.CommonRouteSpec{
559+
ParentRefs: []v1.ParentReference{
560+
{
561+
Name: "podinfo",
562+
},
563+
},
564+
},
565+
},
566+
}
567+
httpRoute, err := router.gatewayAPIClient.GatewayapiV1().HTTPRoutes("default").Create(context.TODO(), httpRoute, metav1.CreateOptions{})
568+
require.NoError(t, err)
569+
570+
t.Run("httproute generation", func(t *testing.T) {
571+
httpRoute.ObjectMeta.Generation = 5
572+
httpRoute.Status.Parents = []v1.RouteParentStatus{
573+
{
574+
ParentRef: v1.ParentReference{
575+
Name: "podinfo",
576+
SectionName: ptr.To(v1.SectionName("https")),
577+
},
578+
Conditions: []metav1.Condition{
579+
{
580+
Type: string(v1.RouteConditionAccepted),
581+
Status: metav1.ConditionTrue,
582+
ObservedGeneration: 1,
583+
},
584+
},
585+
},
586+
{
587+
ParentRef: v1.ParentReference{
588+
Name: "podinfo",
589+
},
590+
Conditions: []metav1.Condition{
591+
{
592+
Type: string(v1.RouteConditionAccepted),
593+
Status: metav1.ConditionFalse,
594+
ObservedGeneration: 4,
595+
},
596+
},
597+
},
598+
}
599+
httpRoute, err := router.gatewayAPIClient.GatewayapiV1().HTTPRoutes("default").Update(context.TODO(), httpRoute, metav1.UpdateOptions{})
600+
require.NoError(t, err)
601+
602+
_, _, _, err = router.GetRoutes(canary)
603+
require.Error(t, err)
604+
605+
httpRoute.Status.Parents[1].Conditions[0].ObservedGeneration = 5
606+
_, err = router.gatewayAPIClient.GatewayapiV1().HTTPRoutes("default").Update(context.TODO(), httpRoute, metav1.UpdateOptions{})
607+
require.NoError(t, err)
608+
609+
_, _, _, err = router.GetRoutes(canary)
610+
require.Error(t, err)
611+
612+
httpRoute.Status.Parents[1].Conditions[0].Status = metav1.ConditionTrue
613+
_, err = router.gatewayAPIClient.GatewayapiV1().HTTPRoutes("default").Update(context.TODO(), httpRoute, metav1.UpdateOptions{})
614+
require.NoError(t, err)
615+
616+
primaryWeight, canaryWeight, mirrored, err := router.GetRoutes(canary)
617+
require.NoError(t, err)
618+
assert.Equal(t, 90, primaryWeight)
619+
assert.Equal(t, 10, canaryWeight)
620+
assert.False(t, mirrored)
621+
})
622+
}

0 commit comments

Comments
 (0)