Skip to content

Commit

Permalink
fix: GatewayReconciler will fall into a loop and cannot converge to a…
Browse files Browse the repository at this point in the history
… stable state
  • Loading branch information
potterhe committed Feb 12, 2025
1 parent 28b9c10 commit c9a9a1e
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ Adding a new version? You'll need three changes:
collected properly when the Konnect integration was enabled (only Konnect-related
metrics were collected, omitting regular DP metrics). This has been fixed.
[#6881](https://github.com/Kong/kubernetes-ingress-controller/pull/6881)
- GatewayReconciler will fall into a loop and cannot converge to stable state
[#7111](https://github.com/Kong/kubernetes-ingress-controller/pull/7111)

## [3.4.1]

Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/gateway/gateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ func (r *GatewayReconciler) updateAddressesAndListenersStatus(
err := r.Status().Update(ctx, pruneGatewayStatusConds(gateway))
return handleUpdateError(err, r.Log, gateway)
}
if !reflect.DeepEqual(gateway.Status.Listeners, listenerStatuses) {
if !isEqualListenersStatus(gateway.Status.Listeners, listenerStatuses) {
gateway.Status.Listeners = listenerStatuses

err := r.Status().Update(ctx, gateway)
Expand Down
41 changes: 41 additions & 0 deletions internal/controllers/gateway/gateway_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"reflect"
"sort"
"time"

"github.com/go-logr/logr"
"github.com/samber/lo"
Expand Down Expand Up @@ -738,3 +739,43 @@ func getAttachedRoutesForListener(ctx context.Context, mgrc client.Client, gatew
}
return attachedRoutes, nil
}

// GatewayReconciler will determine the change of ListenerStatus to decide whether to update Gateway.status.
// When Gateway.spec.listeners has many entries (greater than 10 and has a certain relationship with the number of HTTPRoutes),
// GatewayReconciler will fall into a loop and cannot converge to a stable state (updating status
// will trigger a new gateway reconciler event, and the new round of reconcile will always find differences).
// The reason here is that when using reflect.DeepEqual to compare two slice structs,
// the order of elements will affect the result, and Gateway.status.listeners[].conditions[].lastTransitionTime
// will always be assigned the current time in each round of reconcile, which should be excluded.
func isEqualListenersStatus(a, b []gatewayapi.ListenerStatus) bool {
if len(a) != len(b) {
return false
}

t := metav1.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
ax := prepareListenersStatusForDeepEqual(a, t)
bx := prepareListenersStatusForDeepEqual(b, t)

return reflect.DeepEqual(ax, bx)
}

func prepareListenersStatusForDeepEqual(ls []gatewayapi.ListenerStatus, t metav1.Time) []gatewayapi.ListenerStatus {
ret := make([]gatewayapi.ListenerStatus, len(ls))

for k, v := range ls {
co := v.DeepCopy()

sort.Slice(co.Conditions, func(i, j int) bool {
return co.Conditions[i].Type < co.Conditions[j].Type
})
for kc, _ := range co.Conditions {
co.Conditions[kc].LastTransitionTime = t
}
ret[k] = *co
}

sort.Slice(ret, func(i, j int) bool {
return ret[i].Name < ret[j].Name
})
return ret
}
126 changes: 126 additions & 0 deletions internal/controllers/gateway/gateway_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package gateway
import (
"context"
"testing"
"time"

"github.com/samber/lo"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -526,3 +527,128 @@ func TestRouteAcceptedByGateways(t *testing.T) {
})
}
}

func Test_isEqualListenersStatus(t *testing.T) {
type args struct {
a []gatewayapi.ListenerStatus
b []gatewayapi.ListenerStatus
}

lastTransitionTime := metav1.Now()
lastTransitionTime2 := metav1.NewTime(lastTransitionTime.Add(time.Second))

tests := []struct {
name string
args args
want bool
}{
// TODO: Add test cases.
{
name: "",
args: args{
a: []gatewayapi.ListenerStatus{
{
Name: "http",
SupportedKinds: []gatewayapi.RouteGroupKind{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: gatewayapi.Kind("HTTPRoute"),
},
},
},
{
Name: "https",
SupportedKinds: []gatewayapi.RouteGroupKind{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: gatewayapi.Kind("HTTPRoute"),
},
},
Conditions: []metav1.Condition{
{
Type: "Accepted",
Status: metav1.ConditionTrue,
LastTransitionTime: lastTransitionTime,
},
},
},
},
b: []gatewayapi.ListenerStatus{
{
Name: "https",
SupportedKinds: []gatewayapi.RouteGroupKind{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: gatewayapi.Kind("HTTPRoute"),
},
},
Conditions: []metav1.Condition{
{
Type: "Accepted",
Status: metav1.ConditionTrue,
LastTransitionTime: lastTransitionTime2,
},
},
},
{
Name: "http",
SupportedKinds: []gatewayapi.RouteGroupKind{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: gatewayapi.Kind("HTTPRoute"),
},
},
},
},
},
want: true,
},
{
name: "",
args: args{
a: []gatewayapi.ListenerStatus{
{
Name: "http",
SupportedKinds: []gatewayapi.RouteGroupKind{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: gatewayapi.Kind("HTTPRoute"),
},
},
Conditions: []metav1.Condition{
{
Type: "Programmed",
Status: metav1.ConditionTrue,
},
},
},
},
b: []gatewayapi.ListenerStatus{
{
Name: "http",
SupportedKinds: []gatewayapi.RouteGroupKind{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: gatewayapi.Kind("HTTPRoute"),
},
},
Conditions: []metav1.Condition{
{
Type: "Accepted",
Status: metav1.ConditionTrue,
},
},
},
},
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isEqualListenersStatus(tt.args.a, tt.args.b); got != tt.want {
t.Errorf("isEqualListenersStatus() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit c9a9a1e

Please sign in to comment.