Skip to content

Commit 95379ec

Browse files
bjee19bjee19
committed
Add updates to functional tests
1 parent 80d72b6 commit 95379ec

22 files changed

+836
-534
lines changed

‎internal/mode/static/provisioner/handler.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,13 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger,
104104
panic(fmt.Errorf("unknown resource type %T", e.Resource))
105105
}
106106
case *events.DeleteEvent:
107+
obj := e
107108
switch e.Type.(type) {
108109
case *gatewayv1.Gateway:
110+
if !h.provisioner.isLeader() {
111+
h.provisioner.setResourceToDelete(obj.NamespacedName)
112+
}
113+
109114
if err := h.provisioner.deprovisionNginx(ctx, e.NamespacedName); err != nil {
110115
logger.Error(err, "error deprovisioning nginx resources")
111116
}

‎internal/mode/static/provisioner/handler_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,25 @@ func TestHandleEventBatch_Delete(t *testing.T) {
316316
verifySecret(clientTestSecretName, userClientSSLSecret)
317317
verifySecret(dockerTestSecretName, userDockerSecret)
318318

319-
// delete Gateway
319+
// delete Gateway when provisioner is not leader
320+
provisioner.leader = false
321+
322+
deleteEvent = &events.DeleteEvent{Type: gateway, NamespacedName: client.ObjectKeyFromObject(gateway)}
323+
batch = events.EventBatch{deleteEvent}
324+
handler.HandleEventBatch(ctx, logger, batch)
325+
326+
g.Expect(provisioner.resourcesToDeleteOnStartup).To(Equal([]types.NamespacedName{
327+
{
328+
Namespace: "default",
329+
Name: "gw",
330+
},
331+
}))
332+
g.Expect(store.getGateway(client.ObjectKeyFromObject(gateway))).To(BeNil())
333+
g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(deployment), &appsv1.Deployment{})).To(Succeed())
334+
335+
// delete Gateway when provisioner is leader
336+
provisioner.leader = true
337+
320338
deleteEvent = &events.DeleteEvent{Type: gateway, NamespacedName: client.ObjectKeyFromObject(gateway)}
321339
batch = events.EventBatch{deleteEvent}
322340
handler.HandleEventBatch(ctx, logger, batch)

‎internal/mode/static/provisioner/objects.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"maps"
8+
"sort"
89
"strconv"
910
"time"
1011

@@ -43,6 +44,10 @@ func (p *NginxProvisioner) buildNginxResourceObjects(
4344
gateway *gatewayv1.Gateway,
4445
nProxyCfg *graph.EffectiveNginxProxy,
4546
) ([]client.Object, error) {
47+
// Need to ensure nginx resource objects are generated deterministically. Specifically when generating
48+
// an object's field by ranging over a map, since ranging over a map is done in random order, we need to
49+
// do some processing to ensure the generated results are the same each time.
50+
4651
ngxIncludesConfigMapName := controller.CreateNginxResourceName(resourceName, nginxIncludesConfigMapNameSuffix)
4752
ngxAgentConfigMapName := controller.CreateNginxResourceName(resourceName, nginxAgentConfigMapNameSuffix)
4853

@@ -174,6 +179,12 @@ func (p *NginxProvisioner) buildNginxSecrets(
174179
}
175180
}
176181

182+
// need to sort secrets so everytime buildNginxSecrets is called it will generate the exact same
183+
// array of secrets. This is needed to satisfy deterministic results of the method.
184+
sort.Slice(secrets, func(i, j int) bool {
185+
return secrets[i].GetName() < secrets[j].GetName()
186+
})
187+
177188
if jwtSecretName != "" {
178189
newSecret, err := p.getAndUpdateSecret(
179190
p.cfg.PlusUsageConfig.SecretName,
@@ -358,6 +369,12 @@ func buildNginxService(
358369
servicePorts = append(servicePorts, servicePort)
359370
}
360371

372+
// need to sort ports so everytime buildNginxService is called it will generate the exact same
373+
// array of ports. This is needed to satisfy deterministic results of the method.
374+
sort.Slice(servicePorts, func(i, j int) bool {
375+
return servicePorts[i].Port < servicePorts[j].Port
376+
})
377+
361378
svc := &corev1.Service{
362379
ObjectMeta: objectMeta,
363380
Spec: corev1.ServiceSpec{
@@ -467,6 +484,12 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec(
467484
podAnnotations["prometheus.io/port"] = strconv.Itoa(int(metricsPort))
468485
}
469486

487+
// need to sort ports so everytime buildNginxPodTemplateSpec is called it will generate the exact same
488+
// array of ports. This is needed to satisfy deterministic results of the method.
489+
sort.Slice(containerPorts, func(i, j int) bool {
490+
return containerPorts[i].ContainerPort < containerPorts[j].ContainerPort
491+
})
492+
470493
image, pullPolicy := p.buildImage(nProxyCfg)
471494

472495
spec := corev1.PodTemplateSpec{
@@ -622,6 +645,12 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec(
622645
spec.Spec.ImagePullSecrets = append(spec.Spec.ImagePullSecrets, ref)
623646
}
624647

648+
// need to sort ports so everytime buildNginxPodTemplateSpec is called it will generate the exact same
649+
// array of secrets. This is needed to satisfy deterministic results of the method.
650+
sort.Slice(spec.Spec.ImagePullSecrets, func(i, j int) bool {
651+
return spec.Spec.ImagePullSecrets[i].Name < spec.Spec.ImagePullSecrets[j].Name
652+
})
653+
625654
if p.cfg.Plus {
626655
initCmd := spec.Spec.InitContainers[0].Command
627656
initCmd = append(initCmd,

‎internal/mode/static/provisioner/objects_test.go

Lines changed: 89 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ func TestBuildNginxResourceObjects(t *testing.T) {
5959
{
6060
Port: 80,
6161
},
62+
{
63+
Port: 8888,
64+
},
65+
{
66+
Port: 9999,
67+
},
6268
},
6369
},
6470
}
@@ -116,10 +122,24 @@ func TestBuildNginxResourceObjects(t *testing.T) {
116122
validateMeta(svc)
117123
g.Expect(svc.Spec.Type).To(Equal(defaultServiceType))
118124
g.Expect(svc.Spec.ExternalTrafficPolicy).To(Equal(defaultServicePolicy))
119-
g.Expect(svc.Spec.Ports).To(ContainElement(corev1.ServicePort{
120-
Port: 80,
121-
Name: "port-80",
122-
TargetPort: intstr.FromInt(80),
125+
126+
// service ports is sorted in ascending order by port number when we make the nginx object
127+
g.Expect(svc.Spec.Ports).To(Equal([]corev1.ServicePort{
128+
{
129+
Port: 80,
130+
Name: "port-80",
131+
TargetPort: intstr.FromInt(80),
132+
},
133+
{
134+
Port: 8888,
135+
Name: "port-8888",
136+
TargetPort: intstr.FromInt(8888),
137+
},
138+
{
139+
Port: 9999,
140+
Name: "port-9999",
141+
TargetPort: intstr.FromInt(9999),
142+
},
123143
}))
124144

125145
depObj := objects[4]
@@ -132,13 +152,24 @@ func TestBuildNginxResourceObjects(t *testing.T) {
132152
g.Expect(template.Spec.Containers).To(HaveLen(1))
133153
container := template.Spec.Containers[0]
134154

135-
g.Expect(container.Ports).To(ContainElement(corev1.ContainerPort{
136-
ContainerPort: config.DefaultNginxMetricsPort,
137-
Name: "metrics",
138-
}))
139-
g.Expect(container.Ports).To(ContainElement(corev1.ContainerPort{
140-
ContainerPort: 80,
141-
Name: "port-80",
155+
// container ports is sorted in ascending order by port number when we make the nginx object
156+
g.Expect(container.Ports).To(Equal([]corev1.ContainerPort{
157+
{
158+
ContainerPort: 80,
159+
Name: "port-80",
160+
},
161+
{
162+
ContainerPort: 8888,
163+
Name: "port-8888",
164+
},
165+
{
166+
ContainerPort: config.DefaultNginxMetricsPort,
167+
Name: "metrics",
168+
},
169+
{
170+
ContainerPort: 9999,
171+
Name: "port-9999",
172+
},
142173
}))
143174

144175
g.Expect(container.Image).To(Equal(fmt.Sprintf("%s:1.0.0", defaultNginxImagePath)))
@@ -415,14 +446,32 @@ func TestBuildNginxResourceObjects_DockerSecrets(t *testing.T) {
415446
},
416447
Data: map[string][]byte{"data": []byte("docker")},
417448
}
418-
fakeClient := fake.NewFakeClient(dockerSecret)
449+
450+
dockerSecretTeaName := dockerTestSecretName + "-Tea"
451+
dockerSecretTea := &corev1.Secret{
452+
ObjectMeta: metav1.ObjectMeta{
453+
Name: dockerSecretTeaName,
454+
Namespace: ngfNamespace,
455+
},
456+
Data: map[string][]byte{"data": []byte("docker-Tea")},
457+
}
458+
459+
dockerSecretChaiName := dockerTestSecretName + "-Chai"
460+
dockerSecretChai := &corev1.Secret{
461+
ObjectMeta: metav1.ObjectMeta{
462+
Name: dockerSecretChaiName,
463+
Namespace: ngfNamespace,
464+
},
465+
Data: map[string][]byte{"data": []byte("docker-Chai")},
466+
}
467+
fakeClient := fake.NewFakeClient(dockerSecret, dockerSecretTea, dockerSecretChai)
419468

420469
provisioner := &NginxProvisioner{
421470
cfg: Config{
422471
GatewayPodConfig: &config.GatewayPodConfig{
423472
Namespace: ngfNamespace,
424473
},
425-
NginxDockerSecretNames: []string{dockerTestSecretName},
474+
NginxDockerSecretNames: []string{dockerTestSecretName, dockerSecretTeaName, dockerSecretChaiName},
426475
},
427476
k8sClient: fakeClient,
428477
baseLabelSelector: metav1.LabelSelector{
@@ -443,26 +492,49 @@ func TestBuildNginxResourceObjects_DockerSecrets(t *testing.T) {
443492
objects, err := provisioner.buildNginxResourceObjects(resourceName, gateway, &graph.EffectiveNginxProxy{})
444493
g.Expect(err).ToNot(HaveOccurred())
445494

446-
g.Expect(objects).To(HaveLen(6))
495+
g.Expect(objects).To(HaveLen(8))
447496

448497
expLabels := map[string]string{
449498
"app": "nginx",
450499
"gateway.networking.k8s.io/gateway-name": "gw",
451500
"app.kubernetes.io/name": "gw-nginx",
452501
}
453502

503+
// the (docker-only) secret order in the object list is sorted by secret name
504+
454505
secretObj := objects[0]
455506
secret, ok := secretObj.(*corev1.Secret)
456507
g.Expect(ok).To(BeTrue())
457508
g.Expect(secret.GetName()).To(Equal(controller.CreateNginxResourceName(resourceName, dockerTestSecretName)))
458509
g.Expect(secret.GetLabels()).To(Equal(expLabels))
459510

460-
depObj := objects[5]
511+
chaiSecretObj := objects[1]
512+
secret, ok = chaiSecretObj.(*corev1.Secret)
513+
g.Expect(ok).To(BeTrue())
514+
g.Expect(secret.GetName()).To(Equal(controller.CreateNginxResourceName(resourceName, dockerSecretChaiName)))
515+
g.Expect(secret.GetLabels()).To(Equal(expLabels))
516+
517+
teaSecretObj := objects[2]
518+
secret, ok = teaSecretObj.(*corev1.Secret)
519+
g.Expect(ok).To(BeTrue())
520+
g.Expect(secret.GetName()).To(Equal(controller.CreateNginxResourceName(resourceName, dockerSecretTeaName)))
521+
g.Expect(secret.GetLabels()).To(Equal(expLabels))
522+
523+
depObj := objects[7]
461524
dep, ok := depObj.(*appsv1.Deployment)
462525
g.Expect(ok).To(BeTrue())
463526

464-
g.Expect(dep.Spec.Template.Spec.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{
465-
Name: controller.CreateNginxResourceName(resourceName, dockerTestSecretName),
527+
// imagePullSecrets is sorted by name when we make the nginx object
528+
g.Expect(dep.Spec.Template.Spec.ImagePullSecrets).To(Equal([]corev1.LocalObjectReference{
529+
{
530+
Name: controller.CreateNginxResourceName(resourceName, dockerTestSecretName),
531+
},
532+
{
533+
Name: controller.CreateNginxResourceName(resourceName, dockerSecretChaiName),
534+
},
535+
{
536+
Name: controller.CreateNginxResourceName(resourceName, dockerSecretTeaName),
537+
},
466538
}))
467539
}
468540

‎internal/mode/static/telemetry/collector.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) {
145145
return Data{}, fmt.Errorf("failed to collect cluster information: %w", err)
146146
}
147147

148-
graphResourceCount, err := collectGraphResourceCount(g, c.cfg.ConfigurationGetter)
149-
if err != nil {
150-
return Data{}, fmt.Errorf("failed to collect NGF resource counts: %w", err)
151-
}
148+
graphResourceCount := collectGraphResourceCount(g, c.cfg.ConfigurationGetter)
152149

153150
replicaSet, err := getPodReplicaSet(ctx, c.cfg.K8sClientReader, c.cfg.PodNSName)
154151
if err != nil {
@@ -193,14 +190,10 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) {
193190
func collectGraphResourceCount(
194191
g *graph.Graph,
195192
configurationGetter ConfigurationGetter,
196-
) (NGFResourceCounts, error) {
193+
) NGFResourceCounts {
197194
ngfResourceCounts := NGFResourceCounts{}
198195
cfg := configurationGetter.GetLatestConfiguration()
199196

200-
if cfg == nil {
201-
return ngfResourceCounts, errors.New("latest configuration cannot be nil")
202-
}
203-
204197
ngfResourceCounts.GatewayClassCount = int64(len(g.IgnoredGatewayClasses))
205198
if g.GatewayClass != nil {
206199
ngfResourceCounts.GatewayClassCount++
@@ -219,9 +212,11 @@ func collectGraphResourceCount(
219212
ngfResourceCounts.SecretCount = int64(len(g.ReferencedSecrets))
220213
ngfResourceCounts.ServiceCount = int64(len(g.ReferencedServices))
221214

222-
for _, upstream := range cfg.Upstreams {
223-
if upstream.ErrorMsg == "" {
224-
ngfResourceCounts.EndpointCount += int64(len(upstream.Endpoints))
215+
if cfg != nil {
216+
for _, upstream := range cfg.Upstreams {
217+
if upstream.ErrorMsg == "" {
218+
ngfResourceCounts.EndpointCount += int64(len(upstream.Endpoints))
219+
}
225220
}
226221
}
227222

@@ -249,7 +244,7 @@ func collectGraphResourceCount(
249244
ngfResourceCounts.NginxProxyCount = int64(len(g.ReferencedNginxProxies))
250245
ngfResourceCounts.SnippetsFilterCount = int64(len(g.SnippetsFilters))
251246

252-
return ngfResourceCounts, nil
247+
return ngfResourceCounts
253248
}
254249

255250
type RouteCounts struct {

‎internal/mode/static/telemetry/collector_test.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -728,14 +728,6 @@ var _ = Describe("Collector", Ordered, func() {
728728
_, err := dataCollector.Collect(ctx)
729729
Expect(err).To(MatchError(expectedError))
730730
})
731-
732-
It("should error on nil latest configuration", func(ctx SpecContext) {
733-
expectedError := errors.New("latest configuration cannot be nil")
734-
fakeConfigurationGetter.GetLatestConfigurationReturns(nil)
735-
736-
_, err := dataCollector.Collect(ctx)
737-
Expect(err).To(MatchError(expectedError))
738-
})
739731
})
740732
})
741733
})

‎tests/framework/collector.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ func InstallCollector() ([]byte, error) {
2828
return output, err
2929
}
3030

31+
if output, err := exec.Command(
32+
"helm",
33+
"repo",
34+
"update",
35+
).CombinedOutput(); err != nil {
36+
return output, fmt.Errorf("failed to update helm repos: %w; output: %s", err, string(output))
37+
}
38+
3139
args := []string{
3240
"install",
3341
collectorChartReleaseName,

‎tests/framework/crossplane.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ func injectCrossplaneContainer(
203203
func createCrossplaneExecutor(
204204
k8sClient kubernetes.Interface,
205205
k8sConfig *rest.Config,
206-
ngfPodName,
206+
nginxPodName,
207207
namespace string,
208208
) (remotecommand.Executor, error) {
209209
cmd := []string{"./crossplane", "/etc/nginx/nginx.conf"}
@@ -217,7 +217,7 @@ func createCrossplaneExecutor(
217217
req := k8sClient.CoreV1().RESTClient().Post().
218218
Resource("pods").
219219
SubResource("exec").
220-
Name(ngfPodName).
220+
Name(nginxPodName).
221221
Namespace(namespace).
222222
VersionedParams(opts, scheme.ParameterCodec)
223223

‎tests/framework/portforward.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,13 @@ func PortForward(config *rest.Config, namespace, podName string, ports []string,
5252
for {
5353
if err := forward(); err != nil {
5454
slog.Error("error forwarding ports", "error", err)
55-
slog.Info("retrying port forward in 100ms...")
55+
slog.Info("retrying port forward in 1s...")
5656
}
5757

5858
select {
5959
case <-stopCh:
6060
return
61-
case <-time.After(100 * time.Millisecond):
61+
case <-time.After(1 * time.Second):
6262
// retrying
6363
}
6464
}

0 commit comments

Comments
 (0)