Skip to content

Commit 71ed507

Browse files
bjee19bjee19
committed
Run pipeline
1 parent f151a94 commit 71ed507

File tree

6 files changed

+138
-131
lines changed

6 files changed

+138
-131
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func newEventHandler(
4949
}, nil
5050
}
5151

52+
//nolint:gocyclo // will refactor at some point
5253
func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger, batch events.EventBatch) {
5354
for _, event := range batch {
5455
switch e := event.(type) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ func (p *NginxProvisioner) deprovisionNginx(ctx context.Context, gatewayNSName t
308308
Name: controller.CreateNginxResourceName(gatewayNSName.Name, p.cfg.GCName),
309309
Namespace: gatewayNSName.Namespace,
310310
}
311-
311+
312312
if p.isLeader() {
313313
p.cfg.Logger.Info(
314314
"Removing nginx resources for Gateway",

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,6 @@ func collectGraphResourceCount(
194194
ngfResourceCounts := NGFResourceCounts{}
195195
cfg := configurationGetter.GetLatestConfiguration()
196196

197-
// might need to change this and make this optional, if cfg is nil then we can't report on upstream count but
198-
// we can do everything else.
199-
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++

‎tests/suite/graceful_recovery_test.go‎

Lines changed: 131 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"net/http"
88
"os/exec"
9-
"slices"
109
"strings"
1110
"time"
1211

@@ -65,61 +64,6 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
6564
return nil
6665
}
6766

68-
// var ngfPodName string
69-
70-
BeforeEach(func() {
71-
// this test is unique in that it will check the entire log of both ngf and nginx containers
72-
// for any errors, so in order to avoid errors generated in previous tests we will uninstall
73-
// NGF installed at the suite level, then re-deploy our own. We will also uninstall and re-install
74-
// NGF between each graceful-recovery test for the same reason.
75-
teardown(releaseName)
76-
77-
setup(getDefaultSetupCfg())
78-
79-
podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
80-
Expect(err).ToNot(HaveOccurred())
81-
Expect(podNames).To(HaveLen(1))
82-
83-
ns = core.Namespace{
84-
ObjectMeta: metav1.ObjectMeta{
85-
Name: "graceful-recovery",
86-
},
87-
}
88-
89-
Expect(resourceManager.Apply([]client.Object{&ns})).To(Succeed())
90-
Expect(resourceManager.ApplyFromFiles(files, ns.Name)).To(Succeed())
91-
Expect(resourceManager.WaitForAppsToBeReady(ns.Name)).To(Succeed())
92-
// Expect(resourceManager.WaitForAppsToBeReadyWithPodCount(ns.Name, 2)).To(Succeed())
93-
94-
nginxPodNames, err := framework.GetReadyNginxPodNames(k8sClient, ns.Name, timeoutConfig.GetTimeout)
95-
Expect(err).ToNot(HaveOccurred())
96-
Expect(nginxPodNames).To(HaveLen(1))
97-
98-
setUpPortForward(nginxPodNames[0], ns.Name)
99-
100-
if portFwdPort != 0 {
101-
coffeeURL = fmt.Sprintf("%s:%d/coffee", baseHTTPURL, portFwdPort)
102-
}
103-
if portFwdHTTPSPort != 0 {
104-
teaURL = fmt.Sprintf("%s:%d/tea", baseHTTPSURL, portFwdHTTPSPort)
105-
}
106-
107-
Eventually(
108-
func() error {
109-
return checkForWorkingTraffic(teaURL, coffeeURL)
110-
}).
111-
WithTimeout(timeoutConfig.TestForTrafficTimeout).
112-
WithPolling(500 * time.Millisecond).
113-
Should(Succeed())
114-
})
115-
116-
AfterEach(func() {
117-
cleanUpPortForward()
118-
119-
Expect(resourceManager.DeleteFromFiles(files, ns.Name)).To(Succeed())
120-
Expect(resourceManager.DeleteNamespace(ns.Name)).To(Succeed())
121-
})
122-
12367
getContainerRestartCount := func(podName, namespace, containerName string) (int, error) {
12468
ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetTimeout)
12569
defer cancel()
@@ -234,7 +178,7 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
234178
)).To(Succeed())
235179
}
236180

237-
checkNGFFunctionality := func(teaURL, coffeeURL, ngfPodName, containerName string, files []string, ns *core.Namespace) {
181+
checkNGFFunctionality := func(teaURL, coffeeURL, _ string, files []string, ns *core.Namespace) {
238182
Eventually(
239183
func() error {
240184
return checkForWorkingTraffic(teaURL, coffeeURL)
@@ -256,7 +200,6 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
256200

257201
Expect(resourceManager.ApplyFromFiles(files, ns.Name)).To(Succeed())
258202
Expect(resourceManager.WaitForAppsToBeReady(ns.Name)).To(Succeed())
259-
// Expect(resourceManager.WaitForAppsToBeReadyWithPodCount(ns.Name, 2)).To(Succeed())
260203

261204
var nginxPodNames []string
262205
var err error
@@ -342,7 +285,12 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
342285
var podNames []string
343286
Eventually(
344287
func() bool {
345-
podNames, err = framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetStatusTimeout)
288+
podNames, err = framework.GetReadyNGFPodNames(
289+
k8sClient,
290+
ngfNamespace,
291+
releaseName,
292+
timeoutConfig.GetStatusTimeout,
293+
)
346294
return len(podNames) == 1 && err == nil
347295
}).
348296
WithTimeout(timeoutConfig.CreateTimeout * 2).
@@ -369,7 +317,7 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
369317

370318
setUpPortForward(nginxPodName, ns.Name)
371319

372-
checkNGFFunctionality(teaURL, coffeeURL, ngfPodName, "", files, ns)
320+
checkNGFFunctionality(teaURL, coffeeURL, "", files, ns)
373321
// if errorLogs := getUnexpectedNginxErrorLogs(ngfPodName, ns.Name); errorLogs != "" {
374322
// Skip(fmt.Sprintf("NGINX has unexpected error logs: \n%s", errorLogs))
375323
//}
@@ -383,6 +331,92 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
383331
runRestartNodeTest(teaURL, coffeeURL, files, ns, false)
384332
}
385333

334+
getLeaderElectionLeaseHolderName := func() (string, error) {
335+
ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetTimeout)
336+
defer cancel()
337+
338+
var lease coordination.Lease
339+
key := types.NamespacedName{Name: "ngf-test-nginx-gateway-fabric-leader-election", Namespace: ngfNamespace}
340+
341+
if err := k8sClient.Get(ctx, key, &lease); err != nil {
342+
return "", errors.New("could not retrieve leader election lease")
343+
}
344+
345+
if *lease.Spec.HolderIdentity == "" {
346+
return "", errors.New("leader election lease holder identity is empty")
347+
}
348+
349+
return *lease.Spec.HolderIdentity, nil
350+
}
351+
352+
checkLeaderLeaseChange := func(originalLeaseName string) error {
353+
leaseName, err := getLeaderElectionLeaseHolderName()
354+
if err != nil {
355+
return err
356+
}
357+
358+
if originalLeaseName == leaseName {
359+
return fmt.Errorf(
360+
"expected originalLeaseName: %s, to not match current leaseName: %s",
361+
originalLeaseName,
362+
leaseName,
363+
)
364+
}
365+
366+
return nil
367+
}
368+
369+
BeforeEach(func() {
370+
// this test is unique in that it will check the entire log of both ngf and nginx containers
371+
// for any errors, so in order to avoid errors generated in previous tests we will uninstall
372+
// NGF installed at the suite level, then re-deploy our own. We will also uninstall and re-install
373+
// NGF between each graceful-recovery test for the same reason.
374+
teardown(releaseName)
375+
376+
setup(getDefaultSetupCfg())
377+
378+
podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
379+
Expect(err).ToNot(HaveOccurred())
380+
Expect(podNames).To(HaveLen(1))
381+
382+
ns = core.Namespace{
383+
ObjectMeta: metav1.ObjectMeta{
384+
Name: "graceful-recovery",
385+
},
386+
}
387+
388+
Expect(resourceManager.Apply([]client.Object{&ns})).To(Succeed())
389+
Expect(resourceManager.ApplyFromFiles(files, ns.Name)).To(Succeed())
390+
Expect(resourceManager.WaitForAppsToBeReady(ns.Name)).To(Succeed())
391+
392+
nginxPodNames, err := framework.GetReadyNginxPodNames(k8sClient, ns.Name, timeoutConfig.GetTimeout)
393+
Expect(err).ToNot(HaveOccurred())
394+
Expect(nginxPodNames).To(HaveLen(1))
395+
396+
setUpPortForward(nginxPodNames[0], ns.Name)
397+
398+
if portFwdPort != 0 {
399+
coffeeURL = fmt.Sprintf("%s:%d/coffee", baseHTTPURL, portFwdPort)
400+
}
401+
if portFwdHTTPSPort != 0 {
402+
teaURL = fmt.Sprintf("%s:%d/tea", baseHTTPSURL, portFwdHTTPSPort)
403+
}
404+
405+
Eventually(
406+
func() error {
407+
return checkForWorkingTraffic(teaURL, coffeeURL)
408+
}).
409+
WithTimeout(timeoutConfig.TestForTrafficTimeout).
410+
WithPolling(500 * time.Millisecond).
411+
Should(Succeed())
412+
})
413+
414+
AfterEach(func() {
415+
cleanUpPortForward()
416+
Expect(resourceManager.DeleteFromFiles(files, ns.Name)).To(Succeed())
417+
Expect(resourceManager.DeleteNamespace(ns.Name)).To(Succeed())
418+
})
419+
386420
It("recovers when nginx container is restarted", func() {
387421
nginxPodNames, err := framework.GetReadyNginxPodNames(k8sClient, ns.Name, timeoutConfig.GetTimeout)
388422
Expect(err).ToNot(HaveOccurred())
@@ -398,7 +432,7 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
398432

399433
setUpPortForward(nginxPodNames[0], ns.Name)
400434

401-
checkNGFFunctionality(teaURL, coffeeURL, nginxPodName, nginxContainerName, files, &ns)
435+
checkNGFFunctionality(teaURL, coffeeURL, nginxContainerName, files, &ns)
402436
// if errorLogs := getUnexpectedNginxErrorLogs(nginxPodName, ns.Name); errorLogs != "" {
403437
// Skip(fmt.Sprintf("NGINX has unexpected error logs: \n%s", errorLogs))
404438
//}
@@ -417,6 +451,9 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
417451
ngfPod, err := resourceManager.GetPod(ngfNamespace, startingNGFPodNames[0])
418452
Expect(err).ToNot(HaveOccurred())
419453

454+
leaseName, err := getLeaderElectionLeaseHolderName()
455+
Expect(err).ToNot(HaveOccurred())
456+
420457
ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.DeleteTimeout)
421458
defer cancel()
422459

@@ -443,7 +480,15 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"),
443480

444481
Expect(newNGFPodName).ToNot(Equal(startingNGFPodNames[0]))
445482

446-
checkNGFFunctionality(teaURL, coffeeURL, newNGFPodName, "", files, &ns)
483+
Eventually(
484+
func() error {
485+
return checkLeaderLeaseChange(leaseName)
486+
}).
487+
WithTimeout(timeoutConfig.GetLeaderLeaseTimeout).
488+
WithPolling(500 * time.Millisecond).
489+
Should(Succeed())
490+
491+
checkNGFFunctionality(teaURL, coffeeURL, "", files, &ns)
447492
})
448493

449494
It("recovers when drained node is restarted", func() {
@@ -548,30 +593,30 @@ func getNginxErrorLogs(nginxPodName, namespace string) string {
548593
return errorLogs
549594
}
550595

551-
func getUnexpectedNginxErrorLogs(nginxPodName, namespace string) string {
552-
expectedErrStrings := []string{
553-
"connect() failed (111: Connection refused)",
554-
"could not be resolved (host not found) during usage report",
555-
"server returned 429",
556-
// FIXME(salonichf5) remove this error message check
557-
// when https://github.com/nginx/nginx-gateway-fabric/issues/2090 is completed.
558-
"no live upstreams while connecting to upstream",
559-
}
560-
561-
unexpectedErrors := ""
562-
563-
errorLogs := getNginxErrorLogs(nginxPodName, namespace)
564-
565-
for _, line := range strings.Split(errorLogs, "\n") {
566-
if !slices.ContainsFunc(expectedErrStrings, func(s string) bool {
567-
return strings.Contains(line, s)
568-
}) {
569-
unexpectedErrors += line
570-
}
571-
}
572-
573-
return unexpectedErrors
574-
}
596+
// func getUnexpectedNginxErrorLogs(nginxPodName, namespace string) string {
597+
// expectedErrStrings := []string{
598+
// "connect() failed (111: Connection refused)",
599+
// "could not be resolved (host not found) during usage report",
600+
// "server returned 429",
601+
// // FIXME(salonichf5) remove this error message check
602+
// // when https://github.com/nginx/nginx-gateway-fabric/issues/2090 is completed.
603+
// "no live upstreams while connecting to upstream",
604+
// }
605+
//
606+
// unexpectedErrors := ""
607+
//
608+
// errorLogs := getNginxErrorLogs(nginxPodName, namespace)
609+
//
610+
// for _, line := range strings.Split(errorLogs, "\n") {
611+
// if !slices.ContainsFunc(expectedErrStrings, func(s string) bool {
612+
// return strings.Contains(line, s)
613+
// }) {
614+
// unexpectedErrors += line
615+
// }
616+
// }
617+
//
618+
// return unexpectedErrors
619+
//}
575620

576621
// checkNGFContainerLogsForErrors checks NGF container's logs for any possible errors.
577622
func checkNGFContainerLogsForErrors(ngfPodName string) {
@@ -586,34 +631,3 @@ func checkNGFContainerLogsForErrors(ngfPodName string) {
586631
Expect(line).ToNot(ContainSubstring("\"level\":\"error\""), line)
587632
}
588633
}
589-
590-
func checkLeaderLeaseChange(originalLeaseName string) error {
591-
leaseName, err := getLeaderElectionLeaseHolderName()
592-
if err != nil {
593-
return err
594-
}
595-
596-
if originalLeaseName == leaseName {
597-
return fmt.Errorf("expected originalLeaseName: %s, to not match current leaseName: %s", originalLeaseName, leaseName)
598-
}
599-
600-
return nil
601-
}
602-
603-
func getLeaderElectionLeaseHolderName() (string, error) {
604-
ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetTimeout)
605-
defer cancel()
606-
607-
var lease coordination.Lease
608-
key := types.NamespacedName{Name: "ngf-test-nginx-gateway-fabric-leader-election", Namespace: ngfNamespace}
609-
610-
if err := k8sClient.Get(ctx, key, &lease); err != nil {
611-
return "", errors.New("could not retrieve leader election lease")
612-
}
613-
614-
if *lease.Spec.HolderIdentity == "" {
615-
return "", errors.New("leader election lease holder identity is empty")
616-
}
617-
618-
return *lease.Spec.HolderIdentity, nil
619-
}

‎tests/suite/system_suite_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ var _ = BeforeSuite(func() {
321321
"telemetry", // - running telemetry test (NGF will be deployed as part of the test)
322322
"scale", // - running scale test (this test will deploy its own version)
323323
"reconfiguration", // - running reconfiguration test (test will deploy its own instances)
324-
// "graceful-recovery",
324+
"graceful-recovery",
325325
}
326326
for _, s := range skipSubstrings {
327327
if strings.Contains(labelFilter, s) {

‎tests/suite/tracing_test.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,20 @@ import (
2424
"github.com/nginx/nginx-gateway-fabric/tests/framework"
2525
)
2626

27-
// To run the tracing test, you must build NGF with the following values:
28-
// TELEMETRY_ENDPOINT=otel-collector-opentelemetry-collector.collector.svc.cluster.local:4317
29-
// TELEMETRY_ENDPOINT_INSECURE = true
30-
//
3127
// This test can be flaky when waiting to see traces show up in the collector logs.
3228
// Sometimes they get there right away, sometimes it takes 30 seconds. Retries were
3329
// added to attempt to mitigate the issue, but it didn't fix it 100%.
3430
var _ = Describe("Tracing", FlakeAttempts(2), Ordered, Label("functional", "tracing"), func() {
31+
// To run the tracing test, you must build NGF with the following values:
32+
// TELEMETRY_ENDPOINT=otel-collector-opentelemetry-collector.collector.svc.cluster.local:4317
33+
// TELEMETRY_ENDPOINT_INSECURE = true
34+
3535
var (
3636
files = []string{
3737
"hello-world/apps.yaml",
3838
"hello-world/gateway.yaml",
3939
"hello-world/routes.yaml",
4040
}
41-
// nginxProxyFile = "tracing/nginxproxy.yaml"
4241
policySingleFile = "tracing/policy-single.yaml"
4342
policyMultipleFile = "tracing/policy-multiple.yaml"
4443

0 commit comments

Comments
 (0)