Don't fail on nginx errors

Author: Kate Osborn

tests/suite/graceful_recovery_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 	"os/exec"
+	"slices"
 	"strings"
 	"time"
 
@@ -28,7 +29,7 @@ const (
 	ngfContainerName   = "nginx-gateway"
 )
 
-// Since checkContainerLogsForErrors may experience interference from previous tests (as explained in the function
+// Since checkNGFContainerLogsForErrors may experience interference from previous tests (as explained in the function
 // documentation), this test is recommended to be run separate from other tests.
 var _ = Describe("Graceful Recovery test", Ordered, Label("graceful-recovery"), func() {
 	files := []string{
@@ -189,6 +190,9 @@ func runRestartNodeTest(teaURL, coffeeURL string, files []string, ns *core.Names
 	}
 
 	checkNGFFunctionality(teaURL, coffeeURL, ngfPodName, "", files, ns)
+	if errorLogs := getUnexpectedNginxErrorLogs(ngfPodName); errorLogs != "" {
+		Skip(fmt.Sprintf("NGINX has unexpected error logs: \n%s", errorLogs))
+	}
 }
 
 func runRecoveryTest(teaURL, coffeeURL, ngfPodName, containerName string, files []string, ns *core.Namespace) {
@@ -219,6 +223,9 @@ func runRecoveryTest(teaURL, coffeeURL, ngfPodName, containerName string, files
 	}
 
 	checkNGFFunctionality(teaURL, coffeeURL, ngfPodName, containerName, files, ns)
+	if errorLogs := getUnexpectedNginxErrorLogs(ngfPodName); errorLogs != "" {
+		Skip(fmt.Sprintf("NGINX has unexpected error logs: \n%s", errorLogs))
+	}
 }
 
 func restartContainer(ngfPodName, containerName string) {
@@ -347,52 +354,76 @@ func checkNGFFunctionality(teaURL, coffeeURL, ngfPodName, containerName string,
 		WithPolling(500 * time.Millisecond).
 		Should(Succeed())
 
-	checkContainerLogsForErrors(ngfPodName, containerName == nginxContainerName)
+	// When the NGINX process is killed, some errors are expected in the NGF logs while we wait for the
+	// NGINX container to be restarted. Therefore, we don't want to check the NGF logs for errors when the container
+	// we restarted was NGINX.
+	if containerName != nginxContainerName {
+		checkNGFContainerLogsForErrors(ngfPodName)
+	}
 }
 
-// checkContainerLogsForErrors checks both nginx and NGF container's logs for any possible errors.
-// When the NGINX process is killed, some errors are expected in the NGF logs while we wait for the
-// NGINX container to be restarted.
-func checkContainerLogsForErrors(ngfPodName string, checkNginxLogsOnly bool) {
+func getNginxErrorLogs(ngfPodName string) string {
 	nginxLogs, err := resourceManager.GetPodLogs(
 		ngfNamespace,
 		ngfPodName,
 		&core.PodLogOptions{Container: nginxContainerName},
 	)
 	Expect(err).ToNot(HaveOccurred())
 
+	errPrefixes := []string{"[crit]", "[error]", "[warn]", "[alert]", "[emerg]"}
+	errorLogs := ""
+
 	for _, line := range strings.Split(nginxLogs, "\n") {
-		Expect(line).ToNot(ContainSubstring("[crit]"), line)
-		Expect(line).ToNot(ContainSubstring("[alert]"), line)
-		Expect(line).ToNot(ContainSubstring("[emerg]"), line)
-		if strings.Contains(line, "[error]") {
-			expectedError1 := "connect() failed (111: Connection refused)"
-			expectedError2 := "could not be resolved (host not found) during usage report"
-			expectedError3 := "server returned 429"
-			// FIXME(salonichf5) remove this error message check
-			// when https://github.com/nginxinc/nginx-gateway-fabric/issues/2090 is completed.
-			expectedError4 := "no live upstreams while connecting to upstream"
-			Expect(line).To(Or(
-				ContainSubstring(expectedError1),
-				ContainSubstring(expectedError2),
-				ContainSubstring(expectedError3),
-				ContainSubstring(expectedError4),
-			))
+		for _, prefix := range errPrefixes {
+			if strings.Contains(line, prefix) {
+				errorLogs += line + "\n"
+				break
+			}
 		}
 	}
 
-	if !checkNginxLogsOnly {
-		ngfLogs, err := resourceManager.GetPodLogs(
-			ngfNamespace,
-			ngfPodName,
-			&core.PodLogOptions{Container: ngfContainerName},
-		)
-		Expect(err).ToNot(HaveOccurred())
+	return errorLogs
+}
+
+func getUnexpectedNginxErrorLogs(ngfPodName string) string {
+	expectedErrStrings := []string{
+		"connect() failed (111: Connection refused)",
+		"could not be resolved (host not found) during usage report",
+		"server returned 429",
+		// FIXME(salonichf5) remove this error message check
+		// when https://github.com/nginxinc/nginx-gateway-fabric/issues/2090 is completed.
+		"no live upstreams while connecting to upstream",
+	}
+
+	unexpectedErrors := ""
+
+	errorLogs := getNginxErrorLogs(ngfPodName)
 
-		for _, line := range strings.Split(ngfLogs, "\n") {
-			Expect(line).ToNot(ContainSubstring("\"level\":\"error\""), line)
+	for _, line := range strings.Split(errorLogs, "\n") {
+
+		if !slices.ContainsFunc(expectedErrStrings, func(s string) bool {
+			return strings.Contains(line, s)
+		}) {
+			unexpectedErrors += line
 		}
 	}
+
+	return unexpectedErrors
+}
+
+// checkNGFContainerLogsForErrors checks NGF container's logs for any possible errors.
+func checkNGFContainerLogsForErrors(ngfPodName string) {
+	ngfLogs, err := resourceManager.GetPodLogs(
+		ngfNamespace,
+		ngfPodName,
+		&core.PodLogOptions{Container: ngfContainerName},
+	)
+	Expect(err).ToNot(HaveOccurred())
+
+	for _, line := range strings.Split(ngfLogs, "\n") {
+		Expect(line).ToNot(ContainSubstring("\"level\":\"error\""), line)
+	}
+
 }
 
 func checkLeaderLeaseChange(originalLeaseName string) error {

tests/suite/reconfig_test.go

@@ -405,7 +405,8 @@ var _ = Describe("Reconfiguration Performance Testing", Ordered, Label("nfr", "r
 			).WithTimeout(metricExistTimeout).WithPolling(metricExistPolling).Should(Succeed())
 		}
 
-		checkContainerLogsForErrors(ngfPodName, false)
+		checkNGFContainerLogsForErrors(ngfPodName)
+		nginxErrorLogs := getNginxErrorLogs(ngfPodName)
 
 		reloadCount, err := framework.GetReloadCount(promInstance, ngfPodName)
 		Expect(err).ToNot(HaveOccurred())
@@ -447,6 +448,7 @@ var _ = Describe("Reconfiguration Performance Testing", Ordered, Label("nfr", "r
 			TimeToReadyAvgSingle: timeToReadyAvgSingle,
 			NGINXReloads:         int(reloadCount),
 			NGINXReloadAvgTime:   int(reloadAvgTime),
+			NGINXErrorLogs:       nginxErrorLogs,
 			EventsCount:          int(eventsCount),
 			EventsAvgTime:        int(eventsAvgTime),
 		}
@@ -601,6 +603,7 @@ type reconfigTestResults struct {
 	NumResources         int
 	NGINXReloads         int
 	NGINXReloadAvgTime   int
+	NGINXErrorLogs       string
 	EventsCount          int
 	EventsAvgTime        int
 }
@@ -627,6 +630,9 @@ const reconfigResultTemplate = `
 {{- range .EventsBuckets }}
 	- {{ .Le }}ms: {{ .Val }}
 {{- end }}
+
+### NGINX Error Logs
+{{ .NGINXErrorLogs }}
 `
 
 func writeReconfigResults(dest io.Writer, results reconfigTestResults) error {