Skip to content

Commit 663f878

Browse files
committed
Only update a resource's status if it has changed (nginx#1151)
Problem: NGF updates the status of resources even when the status has not changed. When there are a large number of HTTPRoutes this can significantly decrease processing time. Solution: Modify the status updater to only update status when it has changed. Testing: Re-ran the scale tests. The scale test performance numbers significantly improved with this change.
1 parent f9dab03 commit 663f878

File tree

27 files changed

+3449
-2304
lines changed

27 files changed

+3449
-2304
lines changed

internal/framework/status/setters.go

+226
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
package status
2+
3+
import (
4+
"slices"
5+
6+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
7+
"sigs.k8s.io/controller-runtime/pkg/client"
8+
"sigs.k8s.io/gateway-api/apis/v1beta1"
9+
10+
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
11+
)
12+
13+
// setter is a function that takes an object and sets the status on that object if the status has changed.
14+
// If the status has not changed, and the setter does not set the status, it returns false.
15+
type setter func(client.Object) bool
16+
17+
func newNginxGatewayStatusSetter(clock Clock, status NginxGatewayStatus) func(client.Object) bool {
18+
return func(object client.Object) bool {
19+
ng := object.(*ngfAPI.NginxGateway)
20+
conds := convertConditions(
21+
status.Conditions,
22+
status.ObservedGeneration,
23+
clock.Now(),
24+
)
25+
26+
if conditionsEqual(ng.Status.Conditions, conds) {
27+
return false
28+
}
29+
30+
ng.Status = ngfAPI.NginxGatewayStatus{
31+
Conditions: conds,
32+
}
33+
34+
return true
35+
}
36+
}
37+
38+
func newGatewayClassStatusSetter(clock Clock, gcs GatewayClassStatus) func(client.Object) bool {
39+
return func(object client.Object) bool {
40+
gc := object.(*v1beta1.GatewayClass)
41+
status := prepareGatewayClassStatus(gcs, clock.Now())
42+
43+
if conditionsEqual(gc.Status.Conditions, status.Conditions) {
44+
return false
45+
}
46+
47+
gc.Status = status
48+
49+
return true
50+
}
51+
}
52+
53+
func newGatewayStatusSetter(clock Clock, gs GatewayStatus) func(client.Object) bool {
54+
return func(object client.Object) bool {
55+
gw := object.(*v1beta1.Gateway)
56+
status := prepareGatewayStatus(gs, clock.Now())
57+
58+
if gwStatusEqual(gw.Status, status) {
59+
return false
60+
}
61+
62+
gw.Status = status
63+
64+
return true
65+
}
66+
}
67+
68+
func newHTTPRouteStatusSetter(gatewayCtlrName string, clock Clock, rs HTTPRouteStatus) func(client.Object) bool {
69+
return func(object client.Object) bool {
70+
hr := object.(*v1beta1.HTTPRoute)
71+
status := prepareHTTPRouteStatus(
72+
rs,
73+
gatewayCtlrName,
74+
clock.Now(),
75+
)
76+
77+
if hrStatusEqual(gatewayCtlrName, hr.Status, status) {
78+
return false
79+
}
80+
81+
hr.Status = status
82+
83+
return true
84+
}
85+
}
86+
87+
func gwStatusEqual(prev, cur v1beta1.GatewayStatus) bool {
88+
addressesEqual := slices.EqualFunc(prev.Addresses, cur.Addresses, func(a1, a2 v1beta1.GatewayStatusAddress) bool {
89+
if !equalPointers[v1beta1.AddressType](a1.Type, a2.Type) {
90+
return false
91+
}
92+
93+
return a1.Value == a2.Value
94+
})
95+
96+
if !addressesEqual {
97+
return false
98+
}
99+
100+
if !conditionsEqual(prev.Conditions, cur.Conditions) {
101+
return false
102+
}
103+
104+
return slices.EqualFunc(prev.Listeners, cur.Listeners, func(s1, s2 v1beta1.ListenerStatus) bool {
105+
if s1.Name != s2.Name {
106+
return false
107+
}
108+
109+
if s1.AttachedRoutes != s2.AttachedRoutes {
110+
return false
111+
}
112+
113+
if !conditionsEqual(s1.Conditions, s2.Conditions) {
114+
return false
115+
}
116+
117+
return slices.EqualFunc(s1.SupportedKinds, s2.SupportedKinds, func(k1, k2 v1beta1.RouteGroupKind) bool {
118+
if k1.Kind != k2.Kind {
119+
return false
120+
}
121+
122+
return equalPointers(k1.Group, k2.Group)
123+
})
124+
})
125+
}
126+
127+
func hrStatusEqual(gatewayCtlrName string, prev, cur v1beta1.HTTPRouteStatus) bool {
128+
// Since other controllers may update HTTPRoute status we can't assume anything about the order of the statuses,
129+
// and we have to ignore statuses written by other controllers when checking for equality.
130+
// Therefore, we can't use slices.EqualFunc here because it cares about the order.
131+
132+
// First, we check if the prev status has any RouteParentStatuses that are no longer present in the cur status.
133+
for _, prevParent := range prev.Parents {
134+
if prevParent.ControllerName != v1beta1.GatewayController(gatewayCtlrName) {
135+
continue
136+
}
137+
138+
exists := slices.ContainsFunc(cur.Parents, func(curParent v1beta1.RouteParentStatus) bool {
139+
return routeParentStatusEqual(prevParent, curParent)
140+
})
141+
142+
if !exists {
143+
return false
144+
}
145+
}
146+
147+
// Then, we check if the cur status has any RouteParentStatuses that are no longer present in the prev status.
148+
for _, curParent := range cur.Parents {
149+
exists := slices.ContainsFunc(prev.Parents, func(prevParent v1beta1.RouteParentStatus) bool {
150+
return routeParentStatusEqual(curParent, prevParent)
151+
})
152+
153+
if !exists {
154+
return false
155+
}
156+
}
157+
158+
return true
159+
}
160+
161+
func routeParentStatusEqual(p1, p2 v1beta1.RouteParentStatus) bool {
162+
if p1.ControllerName != p2.ControllerName {
163+
return false
164+
}
165+
166+
if p1.ParentRef.Name != p2.ParentRef.Name {
167+
return false
168+
}
169+
170+
if !equalPointers(p1.ParentRef.Namespace, p2.ParentRef.Namespace) {
171+
return false
172+
}
173+
174+
if !equalPointers(p1.ParentRef.SectionName, p2.ParentRef.SectionName) {
175+
return false
176+
}
177+
178+
// we ignore the rest of the ParentRef fields because we do not set them
179+
180+
return conditionsEqual(p1.Conditions, p2.Conditions)
181+
}
182+
183+
func conditionsEqual(prev, cur []v1.Condition) bool {
184+
return slices.EqualFunc(prev, cur, func(c1, c2 v1.Condition) bool {
185+
if c1.ObservedGeneration != c2.ObservedGeneration {
186+
return false
187+
}
188+
189+
if c1.Type != c2.Type {
190+
return false
191+
}
192+
193+
if c1.Status != c2.Status {
194+
return false
195+
}
196+
197+
if c1.Message != c2.Message {
198+
return false
199+
}
200+
201+
return c1.Reason == c2.Reason
202+
})
203+
}
204+
205+
// equalPointers returns whether two pointers are equal.
206+
// Pointers are considered equal if one of the following is true:
207+
// - They are both nil.
208+
// - One is nil and the other is empty (e.g. nil string and "").
209+
// - They are both non-nil, and their values are the same.
210+
func equalPointers[T comparable](p1, p2 *T) bool {
211+
if p1 == nil && p2 == nil {
212+
return true
213+
}
214+
215+
var p1Val, p2Val T
216+
217+
if p1 != nil {
218+
p1Val = *p1
219+
}
220+
221+
if p2 != nil {
222+
p2Val = *p2
223+
}
224+
225+
return p1Val == p2Val
226+
}

0 commit comments

Comments
 (0)