From 01a18c674f4536dc10614a380b09b5852cb60da7 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Tue, 4 Jun 2024 23:00:23 -0600 Subject: [PATCH] Run graceful recovery tests in pipeline (#2045) Adds graceful recovery test to be run in pipeline Problem: As a user, I want to automate graceful recovery tests Solution: Added a job to run graceful recovery tests in pipeline --- tests/framework/resourcemanager.go | 54 +++++++++++++++++++++++++++ tests/suite/graceful_recovery_test.go | 35 +++++++++++++---- tests/suite/system_suite_test.go | 2 - 3 files changed, 81 insertions(+), 10 deletions(-) diff --git a/tests/framework/resourcemanager.go b/tests/framework/resourcemanager.go index 3612c7af34..c7c231c69b 100644 --- a/tests/framework/resourcemanager.go +++ b/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 + }, + ) +} diff --git a/tests/suite/graceful_recovery_test.go b/tests/suite/graceful_recovery_test.go index 2288f1b0a0..6b593958d5 100644 --- a/tests/suite/graceful_recovery_test.go +++ b/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 { diff --git a/tests/suite/system_suite_test.go b/tests/suite/system_suite_test.go index ab927649fd..2a7a391ea5 100644 --- a/tests/suite/system_suite_test.go +++ b/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") }