Skip to content

Commit feab697

Browse files
sjbermansalonichf5
sjberman
authored and
salonichf5
committed
Fix failing tests; remove flag
1 parent 93acaf9 commit feab697

File tree

9 files changed

+157
-134
lines changed

9 files changed

+157
-134
lines changed

‎cmd/gateway/commands.go

+9-19
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"
@@ -93,9 +92,8 @@ func createControllerCommand() *cobra.Command {
9392
validator: validateResourceName,
9493
}
9594

96-
updateGCStatus bool
97-
gateway = namespacedNameValue{}
98-
configName = stringValidatingValue{
95+
gateway = namespacedNameValue{}
96+
configName = stringValidatingValue{
9997
validator: validateResourceName,
10098
}
10199
serviceName = stringValidatingValue{
@@ -225,14 +223,13 @@ func createControllerCommand() *cobra.Command {
225223
}
226224

227225
conf := config.Config{
228-
GatewayCtlrName: gatewayCtlrName.value,
229-
ConfigName: configName.String(),
230-
Logger: logger,
231-
AtomicLevel: atom,
232-
GatewayClassName: gatewayClassName.value,
233-
GatewayNsName: gwNsName,
234-
UpdateGatewayClassStatus: updateGCStatus,
235-
GatewayPodConfig: podConfig,
226+
GatewayCtlrName: gatewayCtlrName.value,
227+
ConfigName: configName.String(),
228+
Logger: logger,
229+
AtomicLevel: atom,
230+
GatewayClassName: gatewayClassName.value,
231+
GatewayNsName: gwNsName,
232+
GatewayPodConfig: podConfig,
236233
HealthConfig: config.HealthConfig{
237234
Enabled: !disableHealth,
238235
Port: healthListenPort.value,
@@ -321,13 +318,6 @@ func createControllerCommand() *cobra.Command {
321318
`NGINX Gateway Fabric control plane is running in (default namespace: nginx-gateway).`,
322319
)
323320

324-
cmd.Flags().BoolVar(
325-
&updateGCStatus,
326-
updateGCStatusFlag,
327-
true,
328-
"Update the status of the GatewayClass resource.",
329-
)
330-
331321
cmd.Flags().BoolVar(
332322
&disableMetrics,
333323
metricsDisableFlag,

‎cmd/gateway/commands_test.go

-17
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",
@@ -234,22 +233,6 @@ func TestControllerCmdFlagValidation(t *testing.T) {
234233
wantErr: true,
235234
expectedErrPrefix: `invalid argument "!@#$" for "--agent-tls-secret" flag: invalid format`,
236235
},
237-
{
238-
name: "update-gatewayclass-status is set to empty string",
239-
args: []string{
240-
"--update-gatewayclass-status=",
241-
},
242-
wantErr: true,
243-
expectedErrPrefix: `invalid argument "" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
244-
},
245-
{
246-
name: "update-gatewayclass-status is invalid",
247-
args: []string{
248-
"--update-gatewayclass-status=invalid", // not a boolean
249-
},
250-
wantErr: true,
251-
expectedErrPrefix: `invalid argument "invalid" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
252-
},
253236
{
254237
name: "metrics-port is invalid type",
255238
args: []string{

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

-2
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ type Config struct {
4444
MetricsConfig MetricsConfig
4545
// HealthConfig specifies the health probe config.
4646
HealthConfig HealthConfig
47-
// UpdateGatewayClassStatus enables updating the status of the GatewayClass resource.
48-
UpdateGatewayClassStatus bool
4947
// Plus indicates whether NGINX Plus is being used.
5048
Plus bool
5149
// ExperimentalFeatures indicates if experimental features are enabled.

‎internal/mode/static/handler.go

+10-6
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

+1-65
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

+14-15
Original file line numberDiff line numberDiff line change
@@ -242,21 +242,20 @@ func StartManager(cfg config.Config) error {
242242
&cfg.UsageReportConfig,
243243
cfg.Logger.WithName("generator"),
244244
),
245-
k8sClient: mgr.GetClient(),
246-
k8sReader: mgr.GetAPIReader(),
247-
logger: cfg.Logger.WithName("eventHandler"),
248-
logLevelSetter: logLevelSetter,
249-
eventRecorder: recorder,
250-
deployCtxCollector: deployCtxCollector,
251-
graphBuiltHealthChecker: healthChecker,
252-
gatewayPodConfig: cfg.GatewayPodConfig,
253-
controlConfigNSName: controlConfigNSName,
254-
gatewayCtlrName: cfg.GatewayCtlrName,
255-
gatewayClassName: cfg.GatewayClassName,
256-
updateGatewayClassStatus: cfg.UpdateGatewayClassStatus,
257-
plus: cfg.Plus,
258-
statusQueue: statusQueue,
259-
nginxDeployments: nginxUpdater.NginxDeployments,
245+
k8sClient: mgr.GetClient(),
246+
k8sReader: mgr.GetAPIReader(),
247+
logger: cfg.Logger.WithName("eventHandler"),
248+
logLevelSetter: logLevelSetter,
249+
eventRecorder: recorder,
250+
deployCtxCollector: deployCtxCollector,
251+
graphBuiltHealthChecker: healthChecker,
252+
gatewayPodConfig: cfg.GatewayPodConfig,
253+
controlConfigNSName: controlConfigNSName,
254+
gatewayCtlrName: cfg.GatewayCtlrName,
255+
gatewayClassName: cfg.GatewayClassName,
256+
plus: cfg.Plus,
257+
statusQueue: statusQueue,
258+
nginxDeployments: nginxUpdater.NginxDeployments,
260259
})
261260

262261
objects, objectLists := prepareFirstEventBatchPreparerArgs(cfg)

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

+1-1
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

+111
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)