Skip to content

Commit 17977e1

Browse files
ivanmatmatioktalz
ivanmatmati
authored and
oktalz
committed
TEST: gate: cover the listener conflict verdict diff
TestGatewaysWithChangedConflictVerdict replaces TestGatewaysWithPortConflicts, which tested the helper the verdict diff supersedes. It covers the transitions that must select a Gateway (entering a conflict, leaving one, keeping one with a different reason, a listener appearing in or disappearing from the table, a nil previous table on the first cycle) and, most importantly, the two cases that must select nothing: an unchanged listener without conflict, and an unchanged listener with the same conflict. TestCheckListenerConflictsReUpsert exercises the selection through checkListenerConflicts itself, on two Gateways sharing port 80 with incompatible protocols: - when the older Gateway carries the event, the younger one enters a protocol conflict without changing, and must be re-upserted so the verdict reaches its listeners; - when the same conflict already existed and the cycle was triggered by an unrelated resource, neither Gateway may be re-upserted. The second case is the one the previous union-based selection got wrong; it fails without the verdict diff.
1 parent 2d4d03d commit 17977e1

1 file changed

Lines changed: 233 additions & 32 deletions

File tree

‎k8s/gate/tree/gateway_builder_test.go‎

Lines changed: 233 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
package tree
22

33
import (
4+
"io"
5+
"log/slog"
46
"testing"
7+
"time"
58

9+
"github.com/haproxytech/haproxy-unified-gateway/k8s/gate/protocols"
10+
"github.com/haproxytech/haproxy-unified-gateway/k8s/gate/store"
611
"github.com/stretchr/testify/assert"
712
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
813
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -104,7 +109,7 @@ func TestNbListenersWithAndWithoutConflict(t *testing.T) {
104109
}
105110
}
106111

107-
func TestGatewaysWithPortConflicts(t *testing.T) {
112+
func TestGatewaysWithChangedConflictVerdict(t *testing.T) {
108113
gwNamespace := "default"
109114

110115
// Helper to create ObjectKey
@@ -122,71 +127,267 @@ func TestGatewaysWithPortConflicts(t *testing.T) {
122127
}
123128
}
124129

130+
protocolConflict := listenerConflictCondition{
131+
hasConflict: true,
132+
reason: string(gatewayv1.ListenerReasonProtocolConflict),
133+
protocol: protocols.ProtocolCategorySecure,
134+
}
135+
hostnameConflict := listenerConflictCondition{
136+
hasConflict: true,
137+
reason: string(gatewayv1.ListenerReasonHostnameConflict),
138+
protocol: protocols.ProtocolCategorySecure,
139+
}
140+
noConflict := listenerConflictCondition{
141+
hasConflict: false,
142+
protocol: protocols.ProtocolCategorySecure,
143+
}
144+
125145
tests := []struct {
126-
name string
127-
mapPort2ListenerConflict map[gatewayv1.PortNumber]listenerConflict
128-
expectedGwKeys map[client.ObjectKey]struct{}
146+
previous map[gatewayv1.PortNumber]listenerConflict
147+
current map[gatewayv1.PortNumber]listenerConflict
148+
expectedGwKeys map[client.ObjectKey]struct{}
149+
name string
129150
}{
130151
{
131-
name: "No conflicts",
132-
mapPort2ListenerConflict: map[gatewayv1.PortNumber]listenerConflict{},
133-
expectedGwKeys: map[client.ObjectKey]struct{}{},
152+
name: "Both tables empty",
153+
previous: map[gatewayv1.PortNumber]listenerConflict{},
154+
current: map[gatewayv1.PortNumber]listenerConflict{},
155+
expectedGwKeys: map[client.ObjectKey]struct{}{},
134156
},
135157
{
136-
name: "Single gateway with conflict",
137-
mapPort2ListenerConflict: map[gatewayv1.PortNumber]listenerConflict{
138-
80: {
139-
mkKey("gw1", "l1"): {hasConflict: true},
140-
},
158+
// The listener is fine and stays fine: nothing to recompute.
159+
name: "Unchanged listener without conflict",
160+
previous: map[gatewayv1.PortNumber]listenerConflict{
161+
80: {mkKey("gw1", "l1"): noConflict},
162+
},
163+
current: map[gatewayv1.PortNumber]listenerConflict{
164+
80: {mkKey("gw1", "l1"): noConflict},
165+
},
166+
expectedGwKeys: map[client.ObjectKey]struct{}{},
167+
},
168+
{
169+
// A lasting misconfiguration must not be re-checked on every cycle:
170+
// this is the case the previous old-union-new selection re-upserted
171+
// for nothing.
172+
name: "Unchanged listener with the same conflict",
173+
previous: map[gatewayv1.PortNumber]listenerConflict{
174+
80: {mkKey("gw1", "l1"): protocolConflict},
175+
},
176+
current: map[gatewayv1.PortNumber]listenerConflict{
177+
80: {mkKey("gw1", "l1"): protocolConflict},
178+
},
179+
expectedGwKeys: map[client.ObjectKey]struct{}{},
180+
},
181+
{
182+
name: "Listener enters conflict",
183+
previous: map[gatewayv1.PortNumber]listenerConflict{
184+
80: {mkKey("gw1", "l1"): noConflict},
185+
},
186+
current: map[gatewayv1.PortNumber]listenerConflict{
187+
80: {mkKey("gw1", "l1"): protocolConflict},
141188
},
142189
expectedGwKeys: map[client.ObjectKey]struct{}{
143190
mkGwKey("gw1"): {},
144191
},
145192
},
146193
{
147-
name: "Single gateway without conflict",
148-
mapPort2ListenerConflict: map[gatewayv1.PortNumber]listenerConflict{
149-
80: {
150-
mkKey("gw1", "l1"): {hasConflict: false},
151-
},
194+
name: "Listener leaves conflict",
195+
previous: map[gatewayv1.PortNumber]listenerConflict{
196+
80: {mkKey("gw1", "l1"): protocolConflict},
197+
},
198+
current: map[gatewayv1.PortNumber]listenerConflict{
199+
80: {mkKey("gw1", "l1"): noConflict},
200+
},
201+
expectedGwKeys: map[client.ObjectKey]struct{}{
202+
mkGwKey("gw1"): {},
152203
},
153-
expectedGwKeys: map[client.ObjectKey]struct{}{},
154204
},
155205
{
156-
name: "Multiple gateways with conflicts",
157-
mapPort2ListenerConflict: map[gatewayv1.PortNumber]listenerConflict{
158-
80: {
159-
mkKey("gw1", "l1"): {hasConflict: true},
160-
mkKey("gw2", "l1"): {hasConflict: true},
161-
},
206+
// Still conflicting, but the reason changed: the condition message
207+
// must be refreshed, which a membership-only comparison would miss.
208+
name: "Conflict reason changes",
209+
previous: map[gatewayv1.PortNumber]listenerConflict{
210+
80: {mkKey("gw1", "l1"): protocolConflict},
211+
},
212+
current: map[gatewayv1.PortNumber]listenerConflict{
213+
80: {mkKey("gw1", "l1"): hostnameConflict},
214+
},
215+
expectedGwKeys: map[client.ObjectKey]struct{}{
216+
mkGwKey("gw1"): {},
217+
},
218+
},
219+
{
220+
name: "Listener disappeared from the table",
221+
previous: map[gatewayv1.PortNumber]listenerConflict{
222+
80: {mkKey("gw1", "l1"): protocolConflict},
223+
},
224+
current: map[gatewayv1.PortNumber]listenerConflict{},
225+
expectedGwKeys: map[client.ObjectKey]struct{}{mkGwKey("gw1"): {}},
226+
},
227+
{
228+
name: "Listener appeared in the table",
229+
previous: map[gatewayv1.PortNumber]listenerConflict{},
230+
current: map[gatewayv1.PortNumber]listenerConflict{
231+
80: {mkKey("gw1", "l1"): noConflict},
232+
},
233+
expectedGwKeys: map[client.ObjectKey]struct{}{mkGwKey("gw1"): {}},
234+
},
235+
{
236+
// First cycle: previousMapPort2Listeners is still nil.
237+
name: "Nil previous table",
238+
previous: nil,
239+
current: map[gatewayv1.PortNumber]listenerConflict{
240+
80: {mkKey("gw1", "l1"): noConflict},
241+
443: {mkKey("gw2", "l1"): protocolConflict},
162242
},
163243
expectedGwKeys: map[client.ObjectKey]struct{}{
164244
mkGwKey("gw1"): {},
165245
mkGwKey("gw2"): {},
166246
},
167247
},
168248
{
169-
name: "Multiple gateways mixed",
170-
mapPort2ListenerConflict: map[gatewayv1.PortNumber]listenerConflict{
249+
name: "Only the Gateway whose verdict changed is selected",
250+
previous: map[gatewayv1.PortNumber]listenerConflict{
171251
80: {
172-
mkKey("gw1", "l1"): {hasConflict: true},
173-
mkKey("gw2", "l1"): {hasConflict: false},
252+
mkKey("gw1", "l1"): noConflict,
253+
mkKey("gw2", "l1"): protocolConflict,
174254
},
175-
443: {
176-
mkKey("gw3", "l1"): {hasConflict: true},
255+
443: {mkKey("gw3", "l1"): noConflict},
256+
},
257+
current: map[gatewayv1.PortNumber]listenerConflict{
258+
80: {
259+
mkKey("gw1", "l1"): noConflict,
260+
mkKey("gw2", "l1"): protocolConflict,
177261
},
262+
443: {mkKey("gw3", "l1"): hostnameConflict},
178263
},
179264
expectedGwKeys: map[client.ObjectKey]struct{}{
180-
mkGwKey("gw1"): {},
181265
mkGwKey("gw3"): {},
182266
},
183267
},
268+
{
269+
name: "Several listeners of the same Gateway yield a single key",
270+
previous: map[gatewayv1.PortNumber]listenerConflict{
271+
80: {mkKey("gw1", "l1"): noConflict},
272+
443: {mkKey("gw1", "l2"): noConflict},
273+
},
274+
current: map[gatewayv1.PortNumber]listenerConflict{
275+
80: {mkKey("gw1", "l1"): protocolConflict},
276+
443: {mkKey("gw1", "l2"): hostnameConflict},
277+
},
278+
expectedGwKeys: map[client.ObjectKey]struct{}{
279+
mkGwKey("gw1"): {},
280+
},
281+
},
184282
}
185283

186284
for _, tt := range tests {
187285
t.Run(tt.name, func(t *testing.T) {
188-
gwKeys := gatewaysWithPortConflicts(tt.mapPort2ListenerConflict)
286+
gwKeys := gatewaysWithChangedConflictVerdict(tt.previous, tt.current)
189287
assert.Equal(t, tt.expectedGwKeys, gwKeys)
190288
})
191289
}
192290
}
291+
292+
// TestCheckListenerConflictsReUpsert covers the two situations the verdict diff is
293+
// meant to separate: a Gateway whose verdict changed because of another Gateway
294+
// must be re-upserted so the new verdict reaches its listeners, while a Gateway
295+
// whose verdict is unchanged must be left alone even though it is conflicting.
296+
func TestCheckListenerConflictsReUpsert(t *testing.T) {
297+
const ns = "default"
298+
discard := slog.New(slog.NewTextHandler(io.Discard, nil))
299+
300+
older := metav1.NewTime(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC))
301+
younger := metav1.NewTime(time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC))
302+
303+
// mkGateway builds a Gateway with a single listener on port 80. The
304+
// GatewayClass is deliberately absent from the tree so processManagementChecks
305+
// returns early: this test is about the selection, not about management.
306+
mkGateway := func(name string, created metav1.Time, protocol gatewayv1.ProtocolType, status store.Status) *Gateway {
307+
return &Gateway{
308+
K8sResource: &gatewayv1.Gateway{
309+
ObjectMeta: metav1.ObjectMeta{
310+
Name: name,
311+
Namespace: ns,
312+
CreationTimestamp: created,
313+
},
314+
Spec: gatewayv1.GatewaySpec{
315+
GatewayClassName: "not-in-tree",
316+
Listeners: []gatewayv1.Listener{
317+
{Name: "l1", Port: 80, Protocol: protocol},
318+
},
319+
},
320+
},
321+
TreeStatus: TreeUpdate[Gateway]{Status: status},
322+
}
323+
}
324+
325+
listenerKey := func(gwName string) client.ObjectKey {
326+
return client.ObjectKey{Namespace: ns, Name: gwName + "_l1"}
327+
}
328+
329+
newBuilder := func(previous map[gatewayv1.PortNumber]listenerConflict, gws ...*Gateway) *GatewayBuilderImpl {
330+
gateways := map[client.ObjectKey]*Gateway{}
331+
for _, gw := range gws {
332+
gateways[client.ObjectKeyFromObject(gw.K8sResource)] = gw
333+
}
334+
return &GatewayBuilderImpl{
335+
ControllerStore: &ControllerStore{
336+
Logger: discard,
337+
GateTree: &GateTree{Gateways: gateways, GatewayClasses: map[client.ObjectKey]*GatewayClass{}},
338+
UnmanagedGateTree: &GateTree{Gateways: map[client.ObjectKey]*Gateway{}},
339+
previousMapPort2Listeners: previous,
340+
mapPort2Listeners: map[gatewayv1.PortNumber]listenerConflict{},
341+
},
342+
}
343+
}
344+
345+
t.Run("gateway entering conflict because of another gateway is re-upserted", func(t *testing.T) {
346+
// gw1 is older and is the one carrying an event this cycle; it wins port 80
347+
// and pushes gw2 into a protocol conflict without gw2 changing at all.
348+
gw1 := mkGateway("gw1", older, gatewayv1.HTTPProtocolType, store.StatusUpserted)
349+
gw2 := mkGateway("gw2", younger, gatewayv1.HTTPSProtocolType, "")
350+
351+
// Previous cycle: gw2 was alone on port 80, hence no conflict.
352+
previous := map[gatewayv1.PortNumber]listenerConflict{
353+
80: {
354+
listenerKey("gw2"): {protocol: protocols.ProtocolCategorySecure},
355+
},
356+
}
357+
358+
b := newBuilder(previous, gw1, gw2)
359+
b.checkListenerConflicts()
360+
361+
assert.True(t, b.ControllerStore.mapPort2Listeners[80][listenerKey("gw2")].hasConflict,
362+
"gw2 listener must be detected as conflicting")
363+
assert.Equal(t, store.StatusUpserted, gw2.TreeStatus.Status,
364+
"gw2 must be re-upserted so the new verdict reaches its listeners")
365+
})
366+
367+
t.Run("gateway with an unchanged conflict is not re-upserted", func(t *testing.T) {
368+
// Same two Gateways, but the conflict already existed and nothing changed:
369+
// this cycle was triggered by an unrelated resource, so neither Gateway is
370+
// in ClusterStore.Updates.
371+
gw1 := mkGateway("gw1", older, gatewayv1.HTTPProtocolType, "")
372+
gw2 := mkGateway("gw2", younger, gatewayv1.HTTPSProtocolType, "")
373+
374+
previous := map[gatewayv1.PortNumber]listenerConflict{
375+
80: {
376+
listenerKey("gw1"): {protocol: protocols.ProtocolCategoryInsecure},
377+
listenerKey("gw2"): {
378+
hasConflict: true,
379+
reason: string(gatewayv1.ListenerReasonProtocolConflict),
380+
protocol: protocols.ProtocolCategorySecure,
381+
},
382+
},
383+
}
384+
385+
b := newBuilder(previous, gw1, gw2)
386+
b.checkListenerConflicts()
387+
388+
assert.Equal(t, previous, b.ControllerStore.mapPort2Listeners,
389+
"the recomputed table must be identical to the previous one")
390+
assert.Empty(t, gw1.TreeStatus.Status, "gw1 must not be re-upserted")
391+
assert.Empty(t, gw2.TreeStatus.Status, "gw2 must not be re-upserted for an unchanged conflict")
392+
})
393+
}

0 commit comments

Comments
 (0)