Merge branch 'main' into chore/more-linters

Author: lucacome

tests/framework/resourcemanager.go

@@ -683,3 +683,57 @@ func countNumberOfReadyParents(parents []v1.RouteParentStatus) int {
 
 	return readyCount
 }
+
+func (rm *ResourceManager) WaitForAppsToBeReadyWithPodCount(namespace string, podCount int) error {
+	ctx, cancel := context.WithTimeout(context.Background(), rm.TimeoutConfig.CreateTimeout)
+	defer cancel()
+
+	return rm.WaitForAppsToBeReadyWithCtxWithPodCount(ctx, namespace, podCount)
+}
+
+func (rm *ResourceManager) WaitForAppsToBeReadyWithCtxWithPodCount(
+	ctx context.Context,
+	namespace string,
+	podCount int,
+) error {
+	if err := rm.WaitForPodsToBeReadyWithCount(ctx, namespace, podCount); err != nil {
+		return err
+	}
+
+	if err := rm.waitForHTTPRoutesToBeReady(ctx, namespace); err != nil {
+		return err
+	}
+
+	if err := rm.waitForGRPCRoutesToBeReady(ctx, namespace); err != nil {
+		return err
+	}
+
+	return rm.waitForGatewaysToBeReady(ctx, namespace)
+}
+
+// WaitForPodsToBeReady waits for all Pods in the specified namespace to be ready or
+// until the provided context is canceled.
+func (rm *ResourceManager) WaitForPodsToBeReadyWithCount(ctx context.Context, namespace string, count int) error {
+	return wait.PollUntilContextCancel(
+		ctx,
+		500*time.Millisecond,
+		true, /* poll immediately */
+		func(ctx context.Context) (bool, error) {
+			var podList core.PodList
+			if err := rm.K8sClient.List(ctx, &podList, client.InNamespace(namespace)); err != nil {
+				return false, err
+			}
+
+			var podsReady int
+			for _, pod := range podList.Items {
+				for _, cond := range pod.Status.Conditions {
+					if cond.Type == core.PodReady && cond.Status == core.ConditionTrue {
+						podsReady++
+					}
+				}
+			}
+
+			return podsReady == count, nil
+		},
+	)
+}

tests/suite/graceful_recovery_test.go

@@ -28,7 +28,7 @@ const (
 
 // Since checkContainerLogsForErrors may experience interference from previous tests (as explained in the function
 // documentation), this test is recommended to be run separate from other nfr tests.
-var _ = Describe("Graceful Recovery test", Ordered, Label("nfr", "graceful-recovery"), func() {
+var _ = Describe("Graceful Recovery test", Ordered, Label("functional", "graceful-recovery"), func() {
 	files := []string{
 		"graceful-recovery/cafe.yaml",
 		"graceful-recovery/cafe-secret.yaml",
@@ -38,8 +38,10 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("nfr", "graceful-recov
 
 	var ns core.Namespace
 
-	teaURL := "https://cafe.example.com/tea"
-	coffeeURL := "http://cafe.example.com/coffee"
+	baseHTTPURL := "http://cafe.example.com"
+	baseHTTPSURL := "https://cafe.example.com"
+	teaURL := baseHTTPSURL + "/tea"
+	coffeeURL := baseHTTPURL + "/coffee"
 
 	var ngfPodName string
 
@@ -56,6 +58,12 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("nfr", "graceful-recov
 		Expect(podNames).To(HaveLen(1))
 
 		ngfPodName = podNames[0]
+		if portFwdPort != 0 {
+			coffeeURL = fmt.Sprintf("%s:%d/coffee", baseHTTPURL, portFwdPort)
+		}
+		if portFwdHTTPSPort != 0 {
+			teaURL = fmt.Sprintf("%s:%d/tea", baseHTTPSURL, portFwdHTTPSPort)
+		}
 	})
 
 	BeforeEach(func() {
@@ -67,7 +75,7 @@ var _ = Describe("Graceful Recovery test", Ordered, Label("nfr", "graceful-recov
 
 		Expect(resourceManager.Apply([]client.Object{&ns})).To(Succeed())
 		Expect(resourceManager.ApplyFromFiles(files, ns.Name)).To(Succeed())
-		Expect(resourceManager.WaitForAppsToBeReady(ns.Name)).To(Succeed())
+		Expect(resourceManager.WaitForAppsToBeReadyWithPodCount(ns.Name, 2)).To(Succeed())
 
 		Eventually(
 			func() error {
@@ -101,7 +109,7 @@ func runRecoveryTest(teaURL, coffeeURL, ngfPodName, containerName string, files
 	)
 
 	if containerName != nginxContainerName {
-		// Since we have already deployed resources and ran resourceManager.WaitForAppsToBeReady(ns.Name) earlier,
+		// Since we have already deployed resources and ran resourceManager.WaitForAppsToBeReadyWithPodCount earlier,
 		// we know that the applications are ready at this point. This could only be the case if NGF has written
 		// statuses, which could only be the case if NGF has the leader lease. Since there is only one instance
 		// of NGF in this test, we can be certain that this is the correct leaseholder name.
@@ -140,7 +148,7 @@ func runRecoveryTest(teaURL, coffeeURL, ngfPodName, containerName string, files
 		Should(Succeed())
 
 	Expect(resourceManager.ApplyFromFiles(files, ns.Name)).To(Succeed())
-	Expect(resourceManager.WaitForAppsToBeReady(ns.Name)).To(Succeed())
+	Expect(resourceManager.WaitForAppsToBeReadyWithPodCount(ns.Name, 2)).To(Succeed())
 
 	Eventually(
 		func() error {
@@ -265,7 +273,11 @@ func checkContainerLogsForErrors(ngfPodName string) {
 		Expect(line).ToNot(ContainSubstring("[alert]"), line)
 		Expect(line).ToNot(ContainSubstring("[emerg]"), line)
 		if strings.Contains(line, "[error]") {
-			Expect(line).To(ContainSubstring("connect() failed (111: Connection refused)"), line)
+			expectedError1 := "connect() failed (111: Connection refused)"
+			// FIXME(salonichf5) remove this error message check
+			// when https://github.com/nginxinc/nginx-gateway-fabric/issues/2090 is completed.
+			expectedError2 := "no live upstreams while connecting to upstream"
+			Expect(line).To(Or(ContainSubstring(expectedError1), ContainSubstring(expectedError2)))
 		}
 	}
 
@@ -275,7 +287,14 @@ func checkContainerLogsForErrors(ngfPodName string) {
 		&core.PodLogOptions{Container: ngfContainerName},
 	)
 	Expect(err).ToNot(HaveOccurred())
-	Expect(logs).ToNot(ContainSubstring("\"level\":\"error\""), logs)
+
+	for _, line := range strings.Split(logs, "\n") {
+		if *plusEnabled && strings.Contains(line, "\"level\":\"error\"") {
+			Expect(line).To(ContainSubstring("Usage reporting must be enabled when using NGINX Plus"), line)
+		} else {
+			Expect(line).ToNot(ContainSubstring("\"level\":\"error\""), line)
+		}
+	}
 }
 
 func checkLeaderLeaseChange(originalLeaseName string) error {

tests/suite/system_suite_test.go

@@ -259,7 +259,6 @@ var _ = BeforeSuite(func() {
 		"upgrade",            // - running upgrade test (this test will deploy its own version)
 		"longevity-teardown", // - running longevity teardown (deployment will already exist)
 		"telemetry",          // - running telemetry test (NGF will be deployed as part of the test)
-		"graceful-recovery",  // - running graceful recovery test (this test will deploy its own version)
 		"scale",              // - running scale test (this test will deploy its own version)
 	}
 	for _, s := range skipSubstrings {
@@ -299,6 +298,5 @@ func isNFR(labelFilter string) bool {
 		strings.Contains(labelFilter, "longevity") ||
 		strings.Contains(labelFilter, "performance") ||
 		strings.Contains(labelFilter, "upgrade") ||
-		strings.Contains(labelFilter, "graceful-recovery") ||
 		strings.Contains(labelFilter, "scale")
 }