Skip to content

Commit b57371c

Browse files
sjbermansjberman
committed
Fix failing tests; remove flag
1 parent c76ad14 commit b57371c

File tree

9 files changed

+157
-134
lines changed

9 files changed

+157
-134
lines changed

‎cmd/gateway/commands.go

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ func createControllerCommand() *cobra.Command {
6363
configFlag = "config"
6464
serviceFlag = "service"
6565
agentTLSSecretFlag = "agent-tls-secret"
66-
updateGCStatusFlag = "update-gatewayclass-status"
6766
metricsDisableFlag = "metrics-disable"
6867
metricsSecureFlag = "metrics-secure-serving"
6968
metricsPortFlag = "metrics-port"
@@ -94,9 +93,8 @@ func createControllerCommand() *cobra.Command {
9493
validator: validateResourceName,
9594
}
9695

97-
updateGCStatus bool
98-
gateway = namespacedNameValue{}
99-
configName = stringValidatingValue{
96+
gateway = namespacedNameValue{}
97+
configName = stringValidatingValue{
10098
validator: validateResourceName,
10199
}
102100
serviceName = stringValidatingValue{
@@ -229,14 +227,13 @@ func createControllerCommand() *cobra.Command {
229227
}
230228

231229
conf := config.Config{
232-
GatewayCtlrName: gatewayCtlrName.value,
233-
ConfigName: configName.String(),
234-
Logger: logger,
235-
AtomicLevel: atom,
236-
GatewayClassName: gatewayClassName.value,
237-
GatewayNsName: gwNsName,
238-
UpdateGatewayClassStatus: updateGCStatus,
239-
GatewayPodConfig: podConfig,
230+
GatewayCtlrName: gatewayCtlrName.value,
231+
ConfigName: configName.String(),
232+
Logger: logger,
233+
AtomicLevel: atom,
234+
GatewayClassName: gatewayClassName.value,
235+
GatewayNsName: gwNsName,
236+
GatewayPodConfig: podConfig,
240237
HealthConfig: config.HealthConfig{
241238
Enabled: !disableHealth,
242239
Port: healthListenPort.value,
@@ -326,13 +323,6 @@ func createControllerCommand() *cobra.Command {
326323
`NGINX Gateway Fabric control plane is running in (default namespace: nginx-gateway).`,
327324
)
328325

329-
cmd.Flags().BoolVar(
330-
&updateGCStatus,
331-
updateGCStatusFlag,
332-
true,
333-
"Update the status of the GatewayClass resource.",
334-
)
335-
336326
cmd.Flags().BoolVar(
337327
&disableMetrics,
338328
metricsDisableFlag,

‎cmd/gateway/commands_test.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ func TestControllerCmdFlagValidation(t *testing.T) {
141141
"--config=nginx-gateway-config",
142142
"--service=nginx-gateway",
143143
"--agent-tls-secret=agent-tls",
144-
"--update-gatewayclass-status=true",
145144
"--metrics-port=9114",
146145
"--metrics-disable",
147146
"--metrics-secure-serving",
@@ -235,22 +234,6 @@ func TestControllerCmdFlagValidation(t *testing.T) {
235234
wantErr: true,
236235
expectedErrPrefix: `invalid argument "!@#$" for "--agent-tls-secret" flag: invalid format`,
237236
},
238-
{
239-
name: "update-gatewayclass-status is set to empty string",
240-
args: []string{
241-
"--update-gatewayclass-status=",
242-
},
243-
wantErr: true,
244-
expectedErrPrefix: `invalid argument "" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
245-
},
246-
{
247-
name: "update-gatewayclass-status is invalid",
248-
args: []string{
249-
"--update-gatewayclass-status=invalid", // not a boolean
250-
},
251-
wantErr: true,
252-
expectedErrPrefix: `invalid argument "invalid" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
253-
},
254237
{
255238
name: "metrics-port is invalid type",
256239
args: []string{

‎internal/mode/static/config/config.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ type Config struct {
4646
MetricsConfig MetricsConfig
4747
// HealthConfig specifies the health probe config.
4848
HealthConfig HealthConfig
49-
// UpdateGatewayClassStatus enables updating the status of the GatewayClass resource.
50-
UpdateGatewayClassStatus bool
5149
// Plus indicates whether NGINX Plus is being used.
5250
Plus bool
5351
// ExperimentalFeatures indicates if experimental features are enabled.

‎internal/mode/static/handler.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,6 @@ type eventHandlerConfig struct {
7979
gatewayCtlrName string
8080
// gatewayClassName is the name of the GatewayClass.
8181
gatewayClassName string
82-
// updateGatewayClassStatus enables updating the status of the GatewayClass resource.
83-
updateGatewayClassStatus bool
8482
// plus is whether or not we are running NGINX Plus.
8583
plus bool
8684
}
@@ -185,6 +183,15 @@ func (h *eventHandlerImpl) sendNginxConfig(
185183
return
186184
}
187185

186+
if len(gr.Gateways) == 0 {
187+
// still need to update GatewayClass status
188+
obj := &status.QueueObject{
189+
UpdateType: status.UpdateAll,
190+
}
191+
h.cfg.statusQueue.Enqueue(obj)
192+
return
193+
}
194+
188195
for _, gw := range gr.Gateways {
189196
if gw == nil {
190197
// still need to update GatewayClass status
@@ -361,10 +368,7 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, gr *graph.Graph,
361368

362369
transitionTime := metav1.Now()
363370

364-
var gcReqs []frameworkStatus.UpdateRequest
365-
if h.cfg.updateGatewayClassStatus {
366-
gcReqs = status.PrepareGatewayClassRequests(gr.GatewayClass, gr.IgnoredGatewayClasses, transitionTime)
367-
}
371+
gcReqs := status.PrepareGatewayClassRequests(gr.GatewayClass, gr.IgnoredGatewayClasses, transitionTime)
368372
routeReqs := status.PrepareRouteRequests(
369373
gr.L4Routes,
370374
gr.Routes,

‎internal/mode/static/handler_test.go

Lines changed: 1 addition & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,7 @@ var _ = Describe("eventHandler", func() {
140140
ServiceName: "nginx-gateway",
141141
Namespace: "nginx-gateway",
142142
},
143-
metricsCollector: collectors.NewControllerNoopCollector(),
144-
updateGatewayClassStatus: true,
143+
metricsCollector: collectors.NewControllerNoopCollector(),
145144
})
146145
Expect(handler.cfg.graphBuiltHealthChecker.ready).To(BeFalse())
147146
})
@@ -246,69 +245,6 @@ var _ = Describe("eventHandler", func() {
246245
})
247246
})
248247

249-
DescribeTable(
250-
"updating statuses of GatewayClass conditionally based on handler configuration",
251-
func(updateGatewayClassStatus bool) {
252-
handler.cfg.updateGatewayClassStatus = updateGatewayClassStatus
253-
254-
gc := &gatewayv1.GatewayClass{
255-
ObjectMeta: metav1.ObjectMeta{
256-
Name: "test",
257-
},
258-
}
259-
ignoredGC := &gatewayv1.GatewayClass{
260-
ObjectMeta: metav1.ObjectMeta{
261-
Name: "ignored",
262-
},
263-
}
264-
265-
gr := &graph.Graph{
266-
GatewayClass: &graph.GatewayClass{
267-
Source: gc,
268-
Valid: true,
269-
},
270-
IgnoredGatewayClasses: map[types.NamespacedName]*gatewayv1.GatewayClass{
271-
client.ObjectKeyFromObject(ignoredGC): ignoredGC,
272-
},
273-
Gateways: map[types.NamespacedName]*graph.Gateway{},
274-
}
275-
276-
fakeProcessor.ProcessReturns(state.ClusterStateChange, gr)
277-
fakeProcessor.GetLatestGraphReturns(gr)
278-
279-
e := &events.UpsertEvent{
280-
Resource: &gatewayv1.HTTPRoute{}, // any supported is OK
281-
}
282-
283-
batch := []interface{}{e}
284-
285-
var expectedReqsCount int
286-
if updateGatewayClassStatus {
287-
expectedReqsCount = 2
288-
}
289-
290-
handler.HandleEventBatch(context.Background(), logr.Discard(), batch)
291-
292-
Eventually(
293-
func() int {
294-
return fakeStatusUpdater.UpdateGroupCallCount()
295-
}).Should(Equal(2))
296-
297-
_, name, reqs := fakeStatusUpdater.UpdateGroupArgsForCall(0)
298-
Expect(name).To(Equal(groupAllExceptGateways))
299-
Expect(reqs).To(HaveLen(expectedReqsCount))
300-
for _, req := range reqs {
301-
Expect(req.NsName).To(BeElementOf(
302-
client.ObjectKeyFromObject(gc),
303-
client.ObjectKeyFromObject(ignoredGC),
304-
))
305-
Expect(req.ResourceType).To(Equal(&gatewayv1.GatewayClass{}))
306-
}
307-
},
308-
Entry("should update statuses of GatewayClass", true),
309-
Entry("should not update statuses of GatewayClass", false),
310-
)
311-
312248
When("receiving control plane configuration updates", func() {
313249
cfg := func(level ngfAPI.ControllerLogLevel) *ngfAPI.NginxGateway {
314250
return &ngfAPI.NginxGateway{

‎internal/mode/static/manager.go

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -245,21 +245,20 @@ func StartManager(cfg config.Config) error {
245245
&cfg.UsageReportConfig,
246246
cfg.Logger.WithName("generator"),
247247
),
248-
k8sClient: mgr.GetClient(),
249-
k8sReader: mgr.GetAPIReader(),
250-
logger: cfg.Logger.WithName("eventHandler"),
251-
logLevelSetter: logLevelSetter,
252-
eventRecorder: recorder,
253-
deployCtxCollector: deployCtxCollector,
254-
graphBuiltHealthChecker: healthChecker,
255-
gatewayPodConfig: cfg.GatewayPodConfig,
256-
controlConfigNSName: controlConfigNSName,
257-
gatewayCtlrName: cfg.GatewayCtlrName,
258-
gatewayClassName: cfg.GatewayClassName,
259-
updateGatewayClassStatus: cfg.UpdateGatewayClassStatus,
260-
plus: cfg.Plus,
261-
statusQueue: statusQueue,
262-
nginxDeployments: nginxUpdater.NginxDeployments,
248+
k8sClient: mgr.GetClient(),
249+
k8sReader: mgr.GetAPIReader(),
250+
logger: cfg.Logger.WithName("eventHandler"),
251+
logLevelSetter: logLevelSetter,
252+
eventRecorder: recorder,
253+
deployCtxCollector: deployCtxCollector,
254+
graphBuiltHealthChecker: healthChecker,
255+
gatewayPodConfig: cfg.GatewayPodConfig,
256+
controlConfigNSName: controlConfigNSName,
257+
gatewayCtlrName: cfg.GatewayCtlrName,
258+
gatewayClassName: cfg.GatewayClassName,
259+
plus: cfg.Plus,
260+
statusQueue: statusQueue,
261+
nginxDeployments: nginxUpdater.NginxDeployments,
263262
})
264263

265264
objects, objectLists := prepareFirstEventBatchPreparerArgs(cfg)

‎internal/mode/static/state/change_processor_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1478,7 +1478,7 @@ var _ = Describe("ChangeProcessor", func() {
14781478
})
14791479
})
14801480
When("the second Gateway is upserted", func() {
1481-
It("returns populated graph using first gateway", func() {
1481+
It("returns populated graph with second gateway", func() {
14821482
processor.CaptureUpsertChange(gw2)
14831483

14841484
grpcRoute := expGraph.Routes[grpcRouteKey1]

‎internal/mode/static/state/graph/multiple_gateways_test.go

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,115 @@
11
package graph
22

33
// func Test_Gateways(t *testing.T) {
4+
// const gcName = "my-gateway-class"
5+
// createListener := func(
6+
// name string,
7+
// hostname string,
8+
// port int,
9+
// protocol v1.ProtocolType,
10+
// tls *v1.GatewayTLSConfig,
11+
// ) v1.Listener {
12+
// return v1.Listener{
13+
// Name: v1.SectionName(name),
14+
// Hostname: (*v1.Hostname)(helpers.GetPointer(hostname)),
15+
// Port: v1.PortNumber(port), //nolint:gosec // port number will not overflow int32
16+
// Protocol: protocol,
17+
// TLS: tls,
18+
// }
19+
// }
20+
21+
// createHTTPListener := func(name, hostname string, port int) v1.Listener {
22+
// return createListener(name, hostname, port, v1.HTTPProtocolType, nil)
23+
// }
24+
// createTCPListener := func(name, hostname string, port int) v1.Listener {
25+
// return createListener(name, hostname, port, v1.TCPProtocolType, nil)
26+
// }
27+
// createTLSListener := func(name, hostname string, port int) v1.Listener {
28+
// return createListener(
29+
// name,
30+
// hostname,
31+
// port,
32+
// v1.TLSProtocolType,
33+
// &v1.GatewayTLSConfig{Mode: helpers.GetPointer(v1.TLSModePassthrough)},
34+
// )
35+
// }
36+
37+
// createHTTPSListener := func(name, hostname string, port int, tls *v1.GatewayTLSConfig) v1.Listener {
38+
// return createListener(name, hostname, port, v1.HTTPSProtocolType, tls)
39+
// }
40+
41+
// var lastCreatedGateway *v1.Gateway
42+
// type gatewayCfg struct {
43+
// name string
44+
// ref *v1.LocalParametersReference
45+
// listeners []v1.Listener
46+
// addresses []v1.GatewayAddress
47+
// }
48+
// createGateway := func(cfg gatewayCfg) map[types.NamespacedName]*v1.Gateway {
49+
// gatewayMap := make(map[types.NamespacedName]*v1.Gateway)
50+
// lastCreatedGateway = &v1.Gateway{
51+
// ObjectMeta: metav1.ObjectMeta{
52+
// Name: cfg.name,
53+
// Namespace: "test",
54+
// },
55+
// Spec: v1.GatewaySpec{
56+
// GatewayClassName: gcName,
57+
// Listeners: cfg.listeners,
58+
// Addresses: cfg.addresses,
59+
// },
60+
// }
61+
62+
// if cfg.ref != nil {
63+
// lastCreatedGateway.Spec.Infrastructure = &v1.GatewayInfrastructure{
64+
// ParametersRef: cfg.ref,
65+
// }
66+
// }
67+
68+
// gatewayMap[types.NamespacedName{
69+
// Namespace: lastCreatedGateway.Namespace,
70+
// Name: lastCreatedGateway.Name,
71+
// }] = lastCreatedGateway
72+
// return gatewayMap
73+
// }
74+
75+
// getLastCreatedGateway := func() *v1.Gateway {
76+
// return lastCreatedGateway
77+
// }
78+
79+
// validGwNp := &ngfAPIv1alpha2.NginxProxy{
80+
// ObjectMeta: metav1.ObjectMeta{
81+
// Namespace: "test",
82+
// Name: "valid-gw-np",
83+
// },
84+
// Spec: ngfAPIv1alpha2.NginxProxySpec{
85+
// Logging: &ngfAPIv1alpha2.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelError)},
86+
// },
87+
// }
88+
89+
// validGC := &GatewayClass{
90+
// Valid: true,
91+
// }
92+
93+
// gatewayTLSConfigDiffNs := &v1.GatewayTLSConfig{
94+
// Mode: helpers.GetPointer(v1.TLSModeTerminate),
95+
// CertificateRefs: []v1.SecretObjectReference{
96+
// {
97+
// Kind: helpers.GetPointer[v1.Kind]("Secret"),
98+
// Name: v1.ObjectName(secretDiffNamespace.Name),
99+
// Namespace: (*v1.Namespace)(&secretDiffNamespace.Namespace),
100+
// },
101+
// },
102+
// }
103+
104+
// secretDiffNamespace := &apiv1.Secret{
105+
// ObjectMeta: metav1.ObjectMeta{
106+
// Namespace: "diff-ns",
107+
// Name: "secret",
108+
// },
109+
// Data: map[string][]byte{
110+
// apiv1.TLSCertKey: cert,
111+
// apiv1.TLSPrivateKeyKey: key,
112+
// },
113+
// Type: apiv1.SecretTypeTLS,
114+
// }
4115
// }

0 commit comments

Comments
 (0)