From f0c703ed7da7952397202ecaeff501be42ad8b9d Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 29 Aug 2024 15:14:45 -0700 Subject: [PATCH 01/21] Add NginxGateway functional test --- .../nginxgateway/nginx-gateway-crd.yaml | 133 +++++++++++ .../manifests/nginxgateway/nginx-gateway.yaml | 17 ++ tests/suite/nginxgateway_test.go | 207 ++++++++++++++++++ 3 files changed, 357 insertions(+) create mode 100644 tests/suite/manifests/nginxgateway/nginx-gateway-crd.yaml create mode 100644 tests/suite/manifests/nginxgateway/nginx-gateway.yaml create mode 100644 tests/suite/nginxgateway_test.go diff --git a/tests/suite/manifests/nginxgateway/nginx-gateway-crd.yaml b/tests/suite/manifests/nginxgateway/nginx-gateway-crd.yaml new file mode 100644 index 0000000000..90974538ee --- /dev/null +++ b/tests/suite/manifests/nginxgateway/nginx-gateway-crd.yaml @@ -0,0 +1,133 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.1 + name: nginxgateways.gateway.nginx.org +spec: + group: gateway.nginx.org + names: + categories: + - nginx-gateway-fabric + kind: NginxGateway + listKind: NginxGatewayList + plural: nginxgateways + singular: nginxgateway + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: NginxGateway represents the dynamic configuration for an NGINX Gateway Fabric control plane. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: NginxGatewaySpec defines the desired state of the NginxGateway. + properties: + logging: + description: Logging defines logging related settings for the control plane. + properties: + level: + default: info + description: Level defines the logging level. + enum: + - info + - debug + - error + type: string + type: object + type: object + status: + description: NginxGatewayStatus defines the state of the NginxGateway. + properties: + conditions: + items: + description: Condition contains details for one aspect of the current state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the + time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but + the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's + last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + maxItems: 8 + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} diff --git a/tests/suite/manifests/nginxgateway/nginx-gateway.yaml b/tests/suite/manifests/nginxgateway/nginx-gateway.yaml new file mode 100644 index 0000000000..8ebb7ad480 --- /dev/null +++ b/tests/suite/manifests/nginxgateway/nginx-gateway.yaml @@ -0,0 +1,17 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxGateway +metadata: + annotations: + meta.helm.sh/release-name: ngf-test + meta.helm.sh/release-namespace: nginx-gateway + name: ngf-test-config + namespace: nginx-gateway + labels: + helm.sh/chart: nginx-gateway-fabric-1.4.0 + app.kubernetes.io/name: nginx-gateway-fabric + app.kubernetes.io/instance: ngf-test + app.kubernetes.io/version: "edge" + app.kubernetes.io/managed-by: Helm +spec: + logging: + level: info diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go new file mode 100644 index 0000000000..bec32af788 --- /dev/null +++ b/tests/suite/nginxgateway_test.go @@ -0,0 +1,207 @@ +package main + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + core "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/tests/framework" +) + +var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), func() { + var ( + ngfPodName string + + namespace = "nginx-gateway" + nginxGatewayNsname = types.NamespacedName{Name: releaseName + "-config", Namespace: namespace} + + files = []string{ + "nginxgateway/nginx-gateway.yaml", + } + ) + + AfterAll(func() { + // cleanup and restore NGF instance + teardown(releaseName) + setup(getDefaultSetupCfg()) + }) + + getNginxGateway := func(nsname types.NamespacedName) (ngfAPI.NginxGateway, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout) + defer cancel() + + var nginxGateway ngfAPI.NginxGateway + + if err := k8sClient.Get(ctx, nsname, &nginxGateway); err != nil { + return nginxGateway, err + } + + return nginxGateway, nil + } + + verifyAndReturnNginxGateway := func(nsname types.NamespacedName) ngfAPI.NginxGateway { + nginxGateway, err := getNginxGateway(nsname) + Expect(nginxGateway).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + + Expect(nginxGateway.Status.Conditions).To(HaveLen(1)) + condition := nginxGateway.Status.Conditions[0] + + Expect(condition.Type).To(Equal("Valid")) + Expect(condition.Reason).To(Equal("Valid")) + + return nginxGateway + } + + When("testing NGF on startup", func() { + BeforeEach(func() { + teardown(releaseName) + + ns := &core.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed()) + }) + + When("no NginxGateway resource exists", func() { + It("creates one, uses its values, and the status is accepted and true", func() { + setup(getDefaultSetupCfg()) + + podNames, err := framework.GetReadyNGFPodNames( + k8sClient, + ngfNamespace, + releaseName, + timeoutConfig.GetTimeout, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(podNames).To(HaveLen(1)) + ngfPodName = podNames[0] + + _ = verifyAndReturnNginxGateway(nginxGatewayNsname) + + logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + Expect(err).ToNot(HaveOccurred()) + + Expect(logs).To(ContainSubstring("\"level\":\"debug\"")) + }) + }) + + When("an NginxGateway resource already exists", func() { + It("uses its values and the status is accepted and true", func() { + Expect(resourceManager.ApplyFromFiles([]string{"nginxgateway/nginx-gateway-crd.yaml"}, namespace)). + To(Succeed()) + // need to wait until files are applied, no current function because this is a crd file + time.Sleep(2 * time.Second) + Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) + + setup(getDefaultSetupCfg()) + + podNames, err := framework.GetReadyNGFPodNames( + k8sClient, + ngfNamespace, + releaseName, + timeoutConfig.GetTimeout, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(podNames).To(HaveLen(1)) + ngfPodName = podNames[0] + + nginxGateway := verifyAndReturnNginxGateway(nginxGatewayNsname) + Expect(nginxGateway.Status.Conditions[0].ObservedGeneration).To(Equal(int64(1))) + + logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + Expect(err).ToNot(HaveOccurred()) + + Expect(logs).ToNot(ContainSubstring("\"level\":\"debug\"")) + }) + }) + }) + + When("testing on an existing NGF instance", Ordered, func() { + BeforeAll(func() { + teardown(releaseName) + + ns := &core.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed()) + + setup(getDefaultSetupCfg()) + + podNames, err := framework.GetReadyNGFPodNames( + k8sClient, + ngfNamespace, + releaseName, + timeoutConfig.GetTimeout, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(podNames).To(HaveLen(1)) + ngfPodName = podNames[0] + }) + + When("NginxGateway is updated", func() { + It("captures the change, the status is accepted and true,"+ + " and the observed generation is incremented", func() { + var observedGeneration int64 + + nginxGateway := verifyAndReturnNginxGateway(nginxGatewayNsname) + observedGeneration = nginxGateway.Status.Conditions[0].ObservedGeneration + + logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + Expect(err).ToNot(HaveOccurred()) + + Expect(logs).To(ContainSubstring("\"level\":\"debug\"")) + + Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) + // need to wait until files are applied, no current function because this is a nginx-gateway crd + time.Sleep(2 * time.Second) + + nginxGateway = verifyAndReturnNginxGateway(nginxGatewayNsname) + Expect(nginxGateway.Status.Conditions[0].ObservedGeneration).To(Equal(observedGeneration + 1)) + + logs, err = resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + Expect(err).ToNot(HaveOccurred()) + + Expect(logs).To(ContainSubstring( + "\"current\":\"info\",\"msg\":\"Log level changed\",\"prev\":\"debug\"", + )) + }) + }) + + When("NginxGateway is deleted", func() { + It("captures the deletion and default values are used", func() { + Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed()) + time.Sleep(2 * time.Second) // need to wait until deletion is fully processed + + _, err := getNginxGateway(nginxGatewayNsname) + Expect(err).Should(HaveOccurred()) + + logs, err = resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + Expect(err).ToNot(HaveOccurred()) + + Expect(logs).To(ContainSubstring("NginxGateway configuration was deleted; using defaults")) + }) + }) + }) +}) From da61dfb8082c4c403490f31a6a9eb16706660896 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Fri, 30 Aug 2024 10:03:22 -0700 Subject: [PATCH 02/21] Add feedback --- tests/suite/nginxgateway_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index bec32af788..61f84ff996 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -34,7 +34,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f }) getNginxGateway := func(nsname types.NamespacedName) (ngfAPI.NginxGateway, error) { - ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout) + ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetTimeout) defer cancel() var nginxGateway ngfAPI.NginxGateway @@ -48,8 +48,8 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f verifyAndReturnNginxGateway := func(nsname types.NamespacedName) ngfAPI.NginxGateway { nginxGateway, err := getNginxGateway(nsname) - Expect(nginxGateway).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) + Expect(nginxGateway).ToNot(BeNil()) Expect(nginxGateway.Status.Conditions).To(HaveLen(1)) condition := nginxGateway.Status.Conditions[0] From cd306b27c9bc7562982b31de892502ac7d0fc218 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Fri, 30 Aug 2024 14:26:09 -0700 Subject: [PATCH 03/21] Add NGF Pod Name function --- tests/suite/nginxgateway_test.go | 47 ++++++++++++++++---------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 61f84ff996..62c2d7c358 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "fmt" "time" . "github.com/onsi/ginkgo/v2" @@ -60,6 +61,24 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f return nginxGateway } + getNGFPodName := func() (string, error) { + podNames, err := framework.GetReadyNGFPodNames( + k8sClient, + ngfNamespace, + releaseName, + timeoutConfig.GetTimeout, + ) + if err != nil { + return "", err + } + + if len(podNames) != 1 { + return "", fmt.Errorf("expected 1 pod name, got %d", len(podNames)) + } + + return podNames[0], nil + } + When("testing NGF on startup", func() { BeforeEach(func() { teardown(releaseName) @@ -76,15 +95,8 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f It("creates one, uses its values, and the status is accepted and true", func() { setup(getDefaultSetupCfg()) - podNames, err := framework.GetReadyNGFPodNames( - k8sClient, - ngfNamespace, - releaseName, - timeoutConfig.GetTimeout, - ) + ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - Expect(podNames).To(HaveLen(1)) - ngfPodName = podNames[0] _ = verifyAndReturnNginxGateway(nginxGatewayNsname) @@ -107,15 +119,8 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f setup(getDefaultSetupCfg()) - podNames, err := framework.GetReadyNGFPodNames( - k8sClient, - ngfNamespace, - releaseName, - timeoutConfig.GetTimeout, - ) + ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - Expect(podNames).To(HaveLen(1)) - ngfPodName = podNames[0] nginxGateway := verifyAndReturnNginxGateway(nginxGatewayNsname) Expect(nginxGateway.Status.Conditions[0].ObservedGeneration).To(Equal(int64(1))) @@ -143,15 +148,9 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f setup(getDefaultSetupCfg()) - podNames, err := framework.GetReadyNGFPodNames( - k8sClient, - ngfNamespace, - releaseName, - timeoutConfig.GetTimeout, - ) + var err error + ngfPodName, err = getNGFPodName() Expect(err).ToNot(HaveOccurred()) - Expect(podNames).To(HaveLen(1)) - ngfPodName = podNames[0] }) When("NginxGateway is updated", func() { From 165b345c82a62036ad4d64ea0b154453ba1f3950 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 2 Sep 2024 13:52:25 -0700 Subject: [PATCH 04/21] Refactor testing NGF on startup by removing unnecessary teardowns --- tests/suite/nginxgateway_test.go | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 62c2d7c358..625ad56f7b 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -80,21 +80,8 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f } When("testing NGF on startup", func() { - BeforeEach(func() { - teardown(releaseName) - - ns := &core.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespace, - }, - } - Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed()) - }) - When("no NginxGateway resource exists", func() { It("creates one, uses its values, and the status is accepted and true", func() { - setup(getDefaultSetupCfg()) - ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) @@ -111,6 +98,15 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f When("an NginxGateway resource already exists", func() { It("uses its values and the status is accepted and true", func() { + teardown(releaseName) + + ns := &core.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed()) + Expect(resourceManager.ApplyFromFiles([]string{"nginxgateway/nginx-gateway-crd.yaml"}, namespace)). To(Succeed()) // need to wait until files are applied, no current function because this is a crd file From 5978e68c6385ac3177bcaa4dc77a976ad0cf6e1a Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 2 Sep 2024 14:25:43 -0700 Subject: [PATCH 05/21] Refactor tests to test log level at installation --- tests/framework/ngf.go | 26 ++++++++++- .../manifests/nginxgateway/nginx-gateway.yaml | 2 +- tests/suite/nginxgateway_test.go | 44 +++++-------------- tests/suite/system_suite_test.go | 11 ++++- 4 files changed, 46 insertions(+), 37 deletions(-) diff --git a/tests/framework/ngf.go b/tests/framework/ngf.go index 7708172696..4f754fb894 100644 --- a/tests/framework/ngf.go +++ b/tests/framework/ngf.go @@ -56,6 +56,31 @@ func UninstallGatewayAPI(apiVersion string) ([]byte, error) { return nil, nil } +// InstallNGFDebugLevel installs NGF with debug log level. +func InstallNGFDebugLevel(cfg InstallationConfig, extraArgs ...string) ([]byte, error) { + args := []string{ + "install", + "--debug", + cfg.ReleaseName, + cfg.ChartPath, + "--create-namespace", + "--namespace", cfg.Namespace, + "--wait", + "--set", "nginxGateway.productTelemetry.enable=false", + "--set", "nginxGateway.config.logging.level=debug", + } + if cfg.ChartVersion != "" { + args = append(args, "--version", cfg.ChartVersion) + } + + args = append(args, setImageArgs(cfg)...) + fullArgs := append(args, extraArgs...) + + GinkgoWriter.Printf("Installing NGF with command: helm %v\n", strings.Join(fullArgs, " ")) + + return exec.Command("helm", fullArgs...).CombinedOutput() +} + // InstallNGF installs NGF. func InstallNGF(cfg InstallationConfig, extraArgs ...string) ([]byte, error) { args := []string{ @@ -67,7 +92,6 @@ func InstallNGF(cfg InstallationConfig, extraArgs ...string) ([]byte, error) { "--namespace", cfg.Namespace, "--wait", "--set", "nginxGateway.productTelemetry.enable=false", - "--set", "nginxGateway.config.logging.level=debug", } if cfg.ChartVersion != "" { args = append(args, "--version", cfg.ChartVersion) diff --git a/tests/suite/manifests/nginxgateway/nginx-gateway.yaml b/tests/suite/manifests/nginxgateway/nginx-gateway.yaml index 8ebb7ad480..e4d3d850f5 100644 --- a/tests/suite/manifests/nginxgateway/nginx-gateway.yaml +++ b/tests/suite/manifests/nginxgateway/nginx-gateway.yaml @@ -14,4 +14,4 @@ metadata: app.kubernetes.io/managed-by: Helm spec: logging: - level: info + level: debug diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 625ad56f7b..de74538d9d 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -8,9 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" core "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/tests/framework" @@ -80,8 +78,8 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f } When("testing NGF on startup", func() { - When("no NginxGateway resource exists", func() { - It("creates one, uses its values, and the status is accepted and true", func() { + When("log level is set to debug", func() { + It("outputs debug logs and the status is accepted and true", func() { ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) @@ -96,24 +94,13 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f }) }) - When("an NginxGateway resource already exists", func() { - It("uses its values and the status is accepted and true", func() { + When("default log level is used", func() { + It("only outputs info logs and the status is accepted and true", func() { teardown(releaseName) - ns := &core.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespace, - }, - } - Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed()) - - Expect(resourceManager.ApplyFromFiles([]string{"nginxgateway/nginx-gateway-crd.yaml"}, namespace)). - To(Succeed()) - // need to wait until files are applied, no current function because this is a crd file - time.Sleep(2 * time.Second) - Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) - - setup(getDefaultSetupCfg()) + cfg := getDefaultSetupCfg() + cfg.infoLogLevel = true + setup(cfg) ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) @@ -133,17 +120,6 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f When("testing on an existing NGF instance", Ordered, func() { BeforeAll(func() { - teardown(releaseName) - - ns := &core.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespace, - }, - } - Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed()) - - setup(getDefaultSetupCfg()) - var err error ngfPodName, err = getNGFPodName() Expect(err).ToNot(HaveOccurred()) @@ -152,6 +128,8 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f When("NginxGateway is updated", func() { It("captures the change, the status is accepted and true,"+ " and the observed generation is incremented", func() { + // previous test has left the log level at info, this test will change the log level to debug + var observedGeneration int64 nginxGateway := verifyAndReturnNginxGateway(nginxGatewayNsname) @@ -162,7 +140,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f }) Expect(err).ToNot(HaveOccurred()) - Expect(logs).To(ContainSubstring("\"level\":\"debug\"")) + Expect(logs).ToNot(ContainSubstring("\"level\":\"debug\"")) Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) // need to wait until files are applied, no current function because this is a nginx-gateway crd @@ -177,7 +155,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Expect(err).ToNot(HaveOccurred()) Expect(logs).To(ContainSubstring( - "\"current\":\"info\",\"msg\":\"Log level changed\",\"prev\":\"debug\"", + "\"current\":\"debug\",\"msg\":\"Log level changed\",\"prev\":\"info\"", )) }) }) diff --git a/tests/suite/system_suite_test.go b/tests/suite/system_suite_test.go index bb5168d551..3c80ebfa6c 100644 --- a/tests/suite/system_suite_test.go +++ b/tests/suite/system_suite_test.go @@ -96,8 +96,10 @@ type setupConfig struct { gwAPIVersion string deploy bool nfr bool + infoLogLevel bool } +//nolint:gocyclo func setup(cfg setupConfig, extraInstallArgs ...string) { log.SetLogger(GinkgoLogr) @@ -187,8 +189,13 @@ func setup(cfg setupConfig, extraInstallArgs ...string) { output, err := framework.InstallGatewayAPI(cfg.gwAPIVersion) Expect(err).ToNot(HaveOccurred(), string(output)) - output, err = framework.InstallNGF(installCfg, extraInstallArgs...) - Expect(err).ToNot(HaveOccurred(), string(output)) + if cfg.infoLogLevel { + output, err = framework.InstallNGF(installCfg, extraInstallArgs...) + Expect(err).ToNot(HaveOccurred(), string(output)) + } else { + output, err = framework.InstallNGFDebugLevel(installCfg, extraInstallArgs...) + Expect(err).ToNot(HaveOccurred(), string(output)) + } podNames, err := framework.GetReadyNGFPodNames( k8sClient, From 341afd9de8b68d0cf08b7bb59eb6cb885367c129 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 2 Sep 2024 14:29:13 -0700 Subject: [PATCH 06/21] Remove unnecessary crd --- .../nginxgateway/nginx-gateway-crd.yaml | 133 ------------------ 1 file changed, 133 deletions(-) delete mode 100644 tests/suite/manifests/nginxgateway/nginx-gateway-crd.yaml diff --git a/tests/suite/manifests/nginxgateway/nginx-gateway-crd.yaml b/tests/suite/manifests/nginxgateway/nginx-gateway-crd.yaml deleted file mode 100644 index 90974538ee..0000000000 --- a/tests/suite/manifests/nginxgateway/nginx-gateway-crd.yaml +++ /dev/null @@ -1,133 +0,0 @@ ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.16.1 - name: nginxgateways.gateway.nginx.org -spec: - group: gateway.nginx.org - names: - categories: - - nginx-gateway-fabric - kind: NginxGateway - listKind: NginxGatewayList - plural: nginxgateways - singular: nginxgateway - scope: Namespaced - versions: - - additionalPrinterColumns: - - jsonPath: .metadata.creationTimestamp - name: Age - type: date - name: v1alpha1 - schema: - openAPIV3Schema: - description: NginxGateway represents the dynamic configuration for an NGINX Gateway Fabric control plane. - properties: - apiVersion: - description: |- - APIVersion defines the versioned schema of this representation of an object. - Servers should convert recognized schemas to the latest internal value, and - may reject unrecognized values. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources - type: string - kind: - description: |- - Kind is a string value representing the REST resource this object represents. - Servers may infer this from the endpoint the client submits requests to. - Cannot be updated. - In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds - type: string - metadata: - type: object - spec: - description: NginxGatewaySpec defines the desired state of the NginxGateway. - properties: - logging: - description: Logging defines logging related settings for the control plane. - properties: - level: - default: info - description: Level defines the logging level. - enum: - - info - - debug - - error - type: string - type: object - type: object - status: - description: NginxGatewayStatus defines the state of the NginxGateway. - properties: - conditions: - items: - description: Condition contains details for one aspect of the current state of this API Resource. - properties: - lastTransitionTime: - description: |- - lastTransitionTime is the last time the condition transitioned from one status to another. - This should be when the underlying condition changed. If that is not known, then using the - time when the API field changed is acceptable. - format: date-time - type: string - message: - description: |- - message is a human readable message indicating details about the transition. - This may be an empty string. - maxLength: 32768 - type: string - observedGeneration: - description: |- - observedGeneration represents the .metadata.generation that the condition was set based upon. - For instance, if .metadata.generation is currently 12, but - the .status.conditions[x].observedGeneration is 9, the condition is out of date - with respect to the current state of the instance. - format: int64 - minimum: 0 - type: integer - reason: - description: |- - reason contains a programmatic identifier indicating the reason for the condition's - last transition. - Producers of specific condition types may define expected values and meanings for this field, - and whether the values are considered a guaranteed API. - The value should be a CamelCase string. - This field may not be empty. - maxLength: 1024 - minLength: 1 - pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ - type: string - status: - description: status of the condition, one of True, False, Unknown. - enum: - - "True" - - "False" - - Unknown - type: string - type: - description: type of condition in CamelCase or in foo.example.com/CamelCase. - maxLength: 316 - pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ - type: string - required: - - lastTransitionTime - - message - - reason - - status - - type - type: object - maxItems: 8 - type: array - x-kubernetes-list-map-keys: - - type - x-kubernetes-list-type: map - type: object - required: - - spec - type: object - served: true - storage: true - subresources: - status: {} From 6c61bdbfa2eb829c9c863a8a7c09057fa6eb819a Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 2 Sep 2024 14:34:13 -0700 Subject: [PATCH 07/21] Remove unnecessary fields in crd --- tests/suite/manifests/nginxgateway/nginx-gateway.yaml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/suite/manifests/nginxgateway/nginx-gateway.yaml b/tests/suite/manifests/nginxgateway/nginx-gateway.yaml index e4d3d850f5..e7b6f1011f 100644 --- a/tests/suite/manifests/nginxgateway/nginx-gateway.yaml +++ b/tests/suite/manifests/nginxgateway/nginx-gateway.yaml @@ -1,17 +1,8 @@ apiVersion: gateway.nginx.org/v1alpha1 kind: NginxGateway metadata: - annotations: - meta.helm.sh/release-name: ngf-test - meta.helm.sh/release-namespace: nginx-gateway name: ngf-test-config namespace: nginx-gateway - labels: - helm.sh/chart: nginx-gateway-fabric-1.4.0 - app.kubernetes.io/name: nginx-gateway-fabric - app.kubernetes.io/instance: ngf-test - app.kubernetes.io/version: "edge" - app.kubernetes.io/managed-by: Helm spec: logging: level: debug From 2d9e04da3795776501a7f368dff5a559487f3919 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 11:33:20 -0700 Subject: [PATCH 08/21] Break up setup function --- tests/suite/system_suite_test.go | 47 ++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/tests/suite/system_suite_test.go b/tests/suite/system_suite_test.go index 3c80ebfa6c..0d8db5211c 100644 --- a/tests/suite/system_suite_test.go +++ b/tests/suite/system_suite_test.go @@ -99,7 +99,6 @@ type setupConfig struct { infoLogLevel bool } -//nolint:gocyclo func setup(cfg setupConfig, extraInstallArgs ...string) { log.SetLogger(GinkgoLogr) @@ -161,6 +160,31 @@ func setup(cfg setupConfig, extraInstallArgs ...string) { return } + installCfg := setupNGF(cfg, extraInstallArgs...) + + podNames, err := framework.GetReadyNGFPodNames( + k8sClient, + installCfg.Namespace, + installCfg.ReleaseName, + timeoutConfig.CreateTimeout, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(podNames).ToNot(BeEmpty()) + + if *serviceType != "LoadBalancer" { + ports := []string{fmt.Sprintf("%d:80", ngfHTTPForwardedPort), fmt.Sprintf("%d:443", ngfHTTPSForwardedPort)} + portForwardStopCh = make(chan struct{}) + err = framework.PortForward(k8sConfig, installCfg.Namespace, podNames[0], ports, portForwardStopCh) + address = "127.0.0.1" + portFwdPort = ngfHTTPForwardedPort + portFwdHTTPSPort = ngfHTTPSForwardedPort + } else { + address, err = resourceManager.GetLBIPAddress(installCfg.Namespace) + } + Expect(err).ToNot(HaveOccurred()) +} + +func setupNGF(cfg setupConfig, extraInstallArgs ...string) framework.InstallationConfig { installCfg := framework.InstallationConfig{ ReleaseName: cfg.releaseName, Namespace: ngfNamespace, @@ -197,26 +221,7 @@ func setup(cfg setupConfig, extraInstallArgs ...string) { Expect(err).ToNot(HaveOccurred(), string(output)) } - podNames, err := framework.GetReadyNGFPodNames( - k8sClient, - installCfg.Namespace, - installCfg.ReleaseName, - timeoutConfig.CreateTimeout, - ) - Expect(err).ToNot(HaveOccurred()) - Expect(podNames).ToNot(BeEmpty()) - - if *serviceType != "LoadBalancer" { - ports := []string{fmt.Sprintf("%d:80", ngfHTTPForwardedPort), fmt.Sprintf("%d:443", ngfHTTPSForwardedPort)} - portForwardStopCh = make(chan struct{}) - err = framework.PortForward(k8sConfig, installCfg.Namespace, podNames[0], ports, portForwardStopCh) - address = "127.0.0.1" - portFwdPort = ngfHTTPForwardedPort - portFwdHTTPSPort = ngfHTTPSForwardedPort - } else { - address, err = resourceManager.GetLBIPAddress(installCfg.Namespace) - } - Expect(err).ToNot(HaveOccurred()) + return installCfg } func teardown(relName string) { From f694adbe195116736768186359e80453f8a5d2da Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 12:49:01 -0700 Subject: [PATCH 09/21] Remove sleeps and add eventually polling checks for nginxGateway --- tests/suite/nginxgateway_test.go | 72 +++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 19 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index de74538d9d..4de48f7874 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "fmt" "time" @@ -39,24 +40,41 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f var nginxGateway ngfAPI.NginxGateway if err := k8sClient.Get(ctx, nsname, &nginxGateway); err != nil { - return nginxGateway, err + return nginxGateway, errors.New("failed to get nginxGateway") } return nginxGateway, nil } - verifyAndReturnNginxGateway := func(nsname types.NamespacedName) ngfAPI.NginxGateway { + verifyAndReturnNginxGateway := func(nsname types.NamespacedName) (ngfAPI.NginxGateway, error) { nginxGateway, err := getNginxGateway(nsname) - Expect(err).ToNot(HaveOccurred()) - Expect(nginxGateway).ToNot(BeNil()) + if err != nil { + return nginxGateway, err + } + + if nginxGateway.Status.Conditions == nil { + return nginxGateway, errors.New("nginxGateway is has no conditions") + } + + if len(nginxGateway.Status.Conditions) != 1 { + return nginxGateway, + fmt.Errorf("expected nginxGateway to have only one condition, instead has %d conditions", + len(nginxGateway.Status.Conditions)) + } - Expect(nginxGateway.Status.Conditions).To(HaveLen(1)) condition := nginxGateway.Status.Conditions[0] - Expect(condition.Type).To(Equal("Valid")) - Expect(condition.Reason).To(Equal("Valid")) + if condition.Type != "Valid" { + return nginxGateway, fmt.Errorf("expected nginxGateway condition type to be Valid,"+ + " instead has type %s", condition.Type) + } + + if condition.Reason != "Valid" { + return nginxGateway, fmt.Errorf("expected nginxGateway reason to be Valid,"+ + " instead is %s", condition.Reason) + } - return nginxGateway + return nginxGateway, nil } getNGFPodName := func() (string, error) { @@ -83,7 +101,8 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - _ = verifyAndReturnNginxGateway(nginxGatewayNsname) + _, err = verifyAndReturnNginxGateway(nginxGatewayNsname) + Expect(err).ToNot(HaveOccurred()) logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ Container: "nginx-gateway", @@ -105,7 +124,9 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - nginxGateway := verifyAndReturnNginxGateway(nginxGatewayNsname) + nginxGateway, err := verifyAndReturnNginxGateway(nginxGatewayNsname) + Expect(err).ToNot(HaveOccurred()) + Expect(nginxGateway.Status.Conditions[0].ObservedGeneration).To(Equal(int64(1))) logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ @@ -132,7 +153,9 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f var observedGeneration int64 - nginxGateway := verifyAndReturnNginxGateway(nginxGatewayNsname) + nginxGateway, err := verifyAndReturnNginxGateway(nginxGatewayNsname) + Expect(err).ToNot(HaveOccurred()) + observedGeneration = nginxGateway.Status.Conditions[0].ObservedGeneration logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ @@ -143,11 +166,18 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Expect(logs).ToNot(ContainSubstring("\"level\":\"debug\"")) Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) - // need to wait until files are applied, no current function because this is a nginx-gateway crd - time.Sleep(2 * time.Second) - nginxGateway = verifyAndReturnNginxGateway(nginxGatewayNsname) - Expect(nginxGateway.Status.Conditions[0].ObservedGeneration).To(Equal(observedGeneration + 1)) + Eventually( + func() bool { + nginxGateway, err := verifyAndReturnNginxGateway(nginxGatewayNsname) + if err != nil { + return false + } + + return nginxGateway.Status.Conditions[0].ObservedGeneration == observedGeneration+1 + }).WithTimeout(timeoutConfig.UpdateTimeout). + WithPolling(500 * time.Millisecond). + Should(BeTrue()) logs, err = resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ Container: "nginx-gateway", @@ -163,12 +193,16 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f When("NginxGateway is deleted", func() { It("captures the deletion and default values are used", func() { Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed()) - time.Sleep(2 * time.Second) // need to wait until deletion is fully processed - _, err := getNginxGateway(nginxGatewayNsname) - Expect(err).Should(HaveOccurred()) + Eventually( + func() error { + _, err := verifyAndReturnNginxGateway(nginxGatewayNsname) + return err + }).WithTimeout(timeoutConfig.DeleteTimeout). + WithPolling(500 * time.Millisecond). + Should(MatchError("failed to get nginxGateway")) - logs, err = resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ Container: "nginx-gateway", }) Expect(err).ToNot(HaveOccurred()) From 3dd53b85050f7a0f3a2c44687238d79b27121765 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 14:20:00 -0700 Subject: [PATCH 10/21] Add retries to NGF Pod log assertions --- tests/suite/nginxgateway_test.go | 72 ++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 4de48f7874..3311ab3a12 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "time" . "github.com/onsi/ginkgo/v2" @@ -104,12 +105,19 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f _, err = verifyAndReturnNginxGateway(nginxGatewayNsname) Expect(err).ToNot(HaveOccurred()) - logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ - Container: "nginx-gateway", - }) - Expect(err).ToNot(HaveOccurred()) + Eventually( + func() bool { + logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + if err != nil { + return false + } - Expect(logs).To(ContainSubstring("\"level\":\"debug\"")) + return strings.Contains(logs, "\"level\":\"debug\"") + }).WithTimeout(timeoutConfig.GetTimeout). + WithPolling(500 * time.Millisecond). + Should(BeTrue()) }) }) @@ -129,12 +137,19 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Expect(nginxGateway.Status.Conditions[0].ObservedGeneration).To(Equal(int64(1))) - logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ - Container: "nginx-gateway", - }) - Expect(err).ToNot(HaveOccurred()) + Eventually( + func() bool { + logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + if err != nil { + return false + } - Expect(logs).ToNot(ContainSubstring("\"level\":\"debug\"")) + return !strings.Contains(logs, "\"level\":\"debug\"") + }).WithTimeout(timeoutConfig.GetTimeout). + WithPolling(500 * time.Millisecond). + Should(BeTrue()) }) }) }) @@ -179,14 +194,20 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f WithPolling(500 * time.Millisecond). Should(BeTrue()) - logs, err = resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ - Container: "nginx-gateway", - }) - Expect(err).ToNot(HaveOccurred()) + Eventually( + func() bool { + logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + if err != nil { + return false + } - Expect(logs).To(ContainSubstring( - "\"current\":\"debug\",\"msg\":\"Log level changed\",\"prev\":\"info\"", - )) + return strings.Contains(logs, + "\"current\":\"debug\",\"msg\":\"Log level changed\",\"prev\":\"info\"") + }).WithTimeout(timeoutConfig.GetTimeout). + WithPolling(500 * time.Millisecond). + Should(BeTrue()) }) }) @@ -202,12 +223,19 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f WithPolling(500 * time.Millisecond). Should(MatchError("failed to get nginxGateway")) - logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ - Container: "nginx-gateway", - }) - Expect(err).ToNot(HaveOccurred()) + Eventually( + func() bool { + logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ + Container: "nginx-gateway", + }) + if err != nil { + return false + } - Expect(logs).To(ContainSubstring("NginxGateway configuration was deleted; using defaults")) + return strings.Contains(logs, "NginxGateway configuration was deleted; using defaults") + }).WithTimeout(timeoutConfig.GetTimeout). + WithPolling(500 * time.Millisecond). + Should(BeTrue()) }) }) }) From 8dc28fb78a199fde6d7f5651f6a47ac723db5c31 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 14:26:54 -0700 Subject: [PATCH 11/21] Add debug log level as extra install argument --- tests/framework/ngf.go | 25 ----------------------- tests/suite/nginxgateway_test.go | 2 +- tests/suite/system_suite_test.go | 35 +++++++++++++++++--------------- tests/suite/upgrade_test.go | 11 +++++----- 4 files changed, 26 insertions(+), 47 deletions(-) diff --git a/tests/framework/ngf.go b/tests/framework/ngf.go index 4f754fb894..64eb23be25 100644 --- a/tests/framework/ngf.go +++ b/tests/framework/ngf.go @@ -56,31 +56,6 @@ func UninstallGatewayAPI(apiVersion string) ([]byte, error) { return nil, nil } -// InstallNGFDebugLevel installs NGF with debug log level. -func InstallNGFDebugLevel(cfg InstallationConfig, extraArgs ...string) ([]byte, error) { - args := []string{ - "install", - "--debug", - cfg.ReleaseName, - cfg.ChartPath, - "--create-namespace", - "--namespace", cfg.Namespace, - "--wait", - "--set", "nginxGateway.productTelemetry.enable=false", - "--set", "nginxGateway.config.logging.level=debug", - } - if cfg.ChartVersion != "" { - args = append(args, "--version", cfg.ChartVersion) - } - - args = append(args, setImageArgs(cfg)...) - fullArgs := append(args, extraArgs...) - - GinkgoWriter.Printf("Installing NGF with command: helm %v\n", strings.Join(fullArgs, " ")) - - return exec.Command("helm", fullArgs...).CombinedOutput() -} - // InstallNGF installs NGF. func InstallNGF(cfg InstallationConfig, extraArgs ...string) ([]byte, error) { args := []string{ diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 3311ab3a12..d75ae06877 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -126,7 +126,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f teardown(releaseName) cfg := getDefaultSetupCfg() - cfg.infoLogLevel = true + cfg.debugLogLevel = false setup(cfg) ngfPodName, err := getNGFPodName() diff --git a/tests/suite/system_suite_test.go b/tests/suite/system_suite_test.go index 0d8db5211c..e70d6ab9db 100644 --- a/tests/suite/system_suite_test.go +++ b/tests/suite/system_suite_test.go @@ -91,12 +91,12 @@ const ( ) type setupConfig struct { - releaseName string - chartPath string - gwAPIVersion string - deploy bool - nfr bool - infoLogLevel bool + releaseName string + chartPath string + gwAPIVersion string + deploy bool + nfr bool + debugLogLevel bool } func setup(cfg setupConfig, extraInstallArgs ...string) { @@ -213,14 +213,16 @@ func setupNGF(cfg setupConfig, extraInstallArgs ...string) framework.Installatio output, err := framework.InstallGatewayAPI(cfg.gwAPIVersion) Expect(err).ToNot(HaveOccurred(), string(output)) - if cfg.infoLogLevel { - output, err = framework.InstallNGF(installCfg, extraInstallArgs...) - Expect(err).ToNot(HaveOccurred(), string(output)) - } else { - output, err = framework.InstallNGFDebugLevel(installCfg, extraInstallArgs...) - Expect(err).ToNot(HaveOccurred(), string(output)) + if cfg.debugLogLevel { + extraInstallArgs = append( + extraInstallArgs, + "--set", "nginxGateway.config.logging.level=debug", + ) } + output, err = framework.InstallNGF(installCfg, extraInstallArgs...) + Expect(err).ToNot(HaveOccurred(), string(output)) + return installCfg } @@ -267,10 +269,11 @@ func getDefaultSetupCfg() setupConfig { localChartPath = filepath.Join(basepath, "charts/nginx-gateway-fabric") return setupConfig{ - releaseName: releaseName, - chartPath: localChartPath, - gwAPIVersion: *gatewayAPIVersion, - deploy: true, + releaseName: releaseName, + chartPath: localChartPath, + gwAPIVersion: *gatewayAPIVersion, + deploy: true, + debugLogLevel: true, } } diff --git a/tests/suite/upgrade_test.go b/tests/suite/upgrade_test.go index e4b1f1aa7f..0ca7923872 100644 --- a/tests/suite/upgrade_test.go +++ b/tests/suite/upgrade_test.go @@ -48,11 +48,12 @@ var _ = Describe("Upgrade testing", Label("nfr", "upgrade"), func() { teardown(releaseName) cfg := setupConfig{ - releaseName: releaseName, - chartPath: "oci://ghcr.io/nginxinc/charts/nginx-gateway-fabric", - gwAPIVersion: *gatewayAPIPrevVersion, - deploy: true, - nfr: true, + releaseName: releaseName, + chartPath: "oci://ghcr.io/nginxinc/charts/nginx-gateway-fabric", + gwAPIVersion: *gatewayAPIPrevVersion, + deploy: true, + nfr: true, + debugLogLevel: true, } setup(cfg, "--values", valuesFile) From 168c2fbe48dea2c85e7356694ea8be5764d893a8 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 14:37:54 -0700 Subject: [PATCH 12/21] Add expected generation to verifyNginxGateway --- tests/suite/nginxgateway_test.go | 50 ++++++++++++-------------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index d75ae06877..d65d5a09a0 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -47,35 +47,38 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f return nginxGateway, nil } - verifyAndReturnNginxGateway := func(nsname types.NamespacedName) (ngfAPI.NginxGateway, error) { + verifyNginxGateway := func(nsname types.NamespacedName, expObservedGen int64) error { nginxGateway, err := getNginxGateway(nsname) if err != nil { - return nginxGateway, err + return err } if nginxGateway.Status.Conditions == nil { - return nginxGateway, errors.New("nginxGateway is has no conditions") + return errors.New("nginxGateway is has no conditions") } if len(nginxGateway.Status.Conditions) != 1 { - return nginxGateway, - fmt.Errorf("expected nginxGateway to have only one condition, instead has %d conditions", - len(nginxGateway.Status.Conditions)) + return fmt.Errorf("expected nginxGateway to have only one condition, instead has %d conditions", + len(nginxGateway.Status.Conditions)) } condition := nginxGateway.Status.Conditions[0] if condition.Type != "Valid" { - return nginxGateway, fmt.Errorf("expected nginxGateway condition type to be Valid,"+ - " instead has type %s", condition.Type) + return fmt.Errorf("expected nginxGateway condition type to be Valid, instead has type %s", + condition.Type) } if condition.Reason != "Valid" { - return nginxGateway, fmt.Errorf("expected nginxGateway reason to be Valid,"+ - " instead is %s", condition.Reason) + return fmt.Errorf("expected nginxGateway reason to be Valid, instead is %s", condition.Reason) } - return nginxGateway, nil + if condition.ObservedGeneration != expObservedGen { + return fmt.Errorf("expected nginxGateway observed generation to be %d, instead is %d", + expObservedGen, condition.ObservedGeneration) + } + + return nil } getNGFPodName := func() (string, error) { @@ -102,8 +105,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - _, err = verifyAndReturnNginxGateway(nginxGatewayNsname) - Expect(err).ToNot(HaveOccurred()) + Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).ToNot(HaveOccurred()) Eventually( func() bool { @@ -132,10 +134,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - nginxGateway, err := verifyAndReturnNginxGateway(nginxGatewayNsname) - Expect(err).ToNot(HaveOccurred()) - - Expect(nginxGateway.Status.Conditions[0].ObservedGeneration).To(Equal(int64(1))) + Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).ToNot(HaveOccurred()) Eventually( func() bool { @@ -166,12 +165,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f " and the observed generation is incremented", func() { // previous test has left the log level at info, this test will change the log level to debug - var observedGeneration int64 - - nginxGateway, err := verifyAndReturnNginxGateway(nginxGatewayNsname) - Expect(err).ToNot(HaveOccurred()) - - observedGeneration = nginxGateway.Status.Conditions[0].ObservedGeneration + Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).ToNot(HaveOccurred()) logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ Container: "nginx-gateway", @@ -184,12 +178,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Eventually( func() bool { - nginxGateway, err := verifyAndReturnNginxGateway(nginxGatewayNsname) - if err != nil { - return false - } - - return nginxGateway.Status.Conditions[0].ObservedGeneration == observedGeneration+1 + return verifyNginxGateway(nginxGatewayNsname, int64(2)) != nil }).WithTimeout(timeoutConfig.UpdateTimeout). WithPolling(500 * time.Millisecond). Should(BeTrue()) @@ -217,8 +206,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Eventually( func() error { - _, err := verifyAndReturnNginxGateway(nginxGatewayNsname) - return err + return verifyNginxGateway(nginxGatewayNsname, int64(0)) }).WithTimeout(timeoutConfig.DeleteTimeout). WithPolling(500 * time.Millisecond). Should(MatchError("failed to get nginxGateway")) From 512ae4397ad8a269e306ccbc50e670d370f42383 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 14:44:09 -0700 Subject: [PATCH 13/21] Refactor gomega assertions --- tests/suite/nginxgateway_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index d65d5a09a0..3f216b1a1f 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -105,7 +105,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).ToNot(HaveOccurred()) + Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).To(Succeed()) Eventually( func() bool { @@ -134,7 +134,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).ToNot(HaveOccurred()) + Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).To(Succeed()) Eventually( func() bool { @@ -165,7 +165,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f " and the observed generation is incremented", func() { // previous test has left the log level at info, this test will change the log level to debug - Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).ToNot(HaveOccurred()) + Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).To(Succeed()) logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ Container: "nginx-gateway", From 835adf12158089586128208b9f3008c9db716dda Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 14:50:34 -0700 Subject: [PATCH 14/21] Remove teardown and setup after tests are done --- tests/suite/nginxgateway_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 3f216b1a1f..2ad9f8e7d3 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -28,12 +28,6 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f } ) - AfterAll(func() { - // cleanup and restore NGF instance - teardown(releaseName) - setup(getDefaultSetupCfg()) - }) - getNginxGateway := func(nsname types.NamespacedName) (ngfAPI.NginxGateway, error) { ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetTimeout) defer cancel() @@ -99,6 +93,18 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f return podNames[0], nil } + AfterAll(func() { + // re-apply NginxGateway crd to restore NGF instance for following functional tests + Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) + + Eventually( + func() bool { + return verifyNginxGateway(nginxGatewayNsname, int64(1)) != nil + }).WithTimeout(timeoutConfig.UpdateTimeout). + WithPolling(500 * time.Millisecond). + Should(BeTrue()) + }) + When("testing NGF on startup", func() { When("log level is set to debug", func() { It("outputs debug logs and the status is accepted and true", func() { From 4281970ba199650ac3c14267d7cb7e7ba1afaabd Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 15:02:43 -0700 Subject: [PATCH 15/21] Add event check --- tests/suite/nginxgateway_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 2ad9f8e7d3..b81a878e58 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -230,6 +230,20 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f }).WithTimeout(timeoutConfig.GetTimeout). WithPolling(500 * time.Millisecond). Should(BeTrue()) + + events, err := resourceManager.GetEvents(namespace) + Expect(err).ToNot(HaveOccurred()) + + var eventFound bool + for _, item := range events.Items { + if item.Message == "NginxGateway configuration was deleted; using defaults" && + item.Type == "Warning" && + item.Reason == "ResourceDeleted" { + eventFound = true + break + } + } + Expect(eventFound).To(BeTrue()) }) }) }) From 5df3672559a2222382cf2354dff3998fe6124726 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 3 Sep 2024 15:07:18 -0700 Subject: [PATCH 16/21] Rename variable --- tests/suite/nginxgateway_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index b81a878e58..16cf9fc043 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -234,16 +234,16 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f events, err := resourceManager.GetEvents(namespace) Expect(err).ToNot(HaveOccurred()) - var eventFound bool + var foundNginxGatewayDeletionEvent bool for _, item := range events.Items { if item.Message == "NginxGateway configuration was deleted; using defaults" && item.Type == "Warning" && item.Reason == "ResourceDeleted" { - eventFound = true + foundNginxGatewayDeletionEvent = true break } } - Expect(eventFound).To(BeTrue()) + Expect(foundNginxGatewayDeletionEvent).To(BeTrue()) }) }) }) From bcbe86dc16e2edfa11dfda3fcaf07842c153cbdb Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 4 Sep 2024 09:58:54 -0700 Subject: [PATCH 17/21] Add nit feedback review --- tests/suite/nginxgateway_test.go | 19 +++++++++++++------ tests/suite/system_suite_test.go | 4 ++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 16cf9fc043..dc56660ada 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -52,15 +52,19 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f } if len(nginxGateway.Status.Conditions) != 1 { - return fmt.Errorf("expected nginxGateway to have only one condition, instead has %d conditions", - len(nginxGateway.Status.Conditions)) + return fmt.Errorf( + "expected nginxGateway to have only one condition, instead has %d conditions", + len(nginxGateway.Status.Conditions), + ) } condition := nginxGateway.Status.Conditions[0] if condition.Type != "Valid" { - return fmt.Errorf("expected nginxGateway condition type to be Valid, instead has type %s", - condition.Type) + return fmt.Errorf( + "expected nginxGateway condition type to be Valid, instead has type %s", + condition.Type, + ) } if condition.Reason != "Valid" { @@ -68,8 +72,11 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f } if condition.ObservedGeneration != expObservedGen { - return fmt.Errorf("expected nginxGateway observed generation to be %d, instead is %d", - expObservedGen, condition.ObservedGeneration) + return fmt.Errorf( + "expected nginxGateway observed generation to be %d, instead is %d", + expObservedGen, + condition.ObservedGeneration, + ) } return nil diff --git a/tests/suite/system_suite_test.go b/tests/suite/system_suite_test.go index e70d6ab9db..524898ffd0 100644 --- a/tests/suite/system_suite_test.go +++ b/tests/suite/system_suite_test.go @@ -160,7 +160,7 @@ func setup(cfg setupConfig, extraInstallArgs ...string) { return } - installCfg := setupNGF(cfg, extraInstallArgs...) + installCfg := createNGFInstallConfig(cfg, extraInstallArgs...) podNames, err := framework.GetReadyNGFPodNames( k8sClient, @@ -184,7 +184,7 @@ func setup(cfg setupConfig, extraInstallArgs ...string) { Expect(err).ToNot(HaveOccurred()) } -func setupNGF(cfg setupConfig, extraInstallArgs ...string) framework.InstallationConfig { +func createNGFInstallConfig(cfg setupConfig, extraInstallArgs ...string) framework.InstallationConfig { installCfg := framework.InstallationConfig{ ReleaseName: cfg.releaseName, Namespace: ngfNamespace, From 6f55ed18ec2f3c71f898bbd3b5d42c91fb347fa5 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 4 Sep 2024 10:48:57 -0700 Subject: [PATCH 18/21] Refactor nginx gateway verification --- tests/suite/nginxgateway_test.go | 72 +++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index dc56660ada..9a9c51750b 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -41,24 +41,35 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f return nginxGateway, nil } - verifyNginxGateway := func(nsname types.NamespacedName, expObservedGen int64) error { - nginxGateway, err := getNginxGateway(nsname) - if err != nil { - return err - } - - if nginxGateway.Status.Conditions == nil { + verifyNginxGatewayConditions := func(ng ngfAPI.NginxGateway) error { + if ng.Status.Conditions == nil { return errors.New("nginxGateway is has no conditions") } - if len(nginxGateway.Status.Conditions) != 1 { + if len(ng.Status.Conditions) != 1 { return fmt.Errorf( "expected nginxGateway to have only one condition, instead has %d conditions", - len(nginxGateway.Status.Conditions), + len(ng.Status.Conditions), ) } - condition := nginxGateway.Status.Conditions[0] + return nil + } + + getNginxGatewayCurrentObservedGeneration := func(ng ngfAPI.NginxGateway) (int64, error) { + if err := verifyNginxGatewayConditions(ng); err != nil { + return 0, err + } + + return ng.Status.Conditions[0].ObservedGeneration, nil + } + + verifyNginxGatewayStatus := func(ng ngfAPI.NginxGateway, expObservedGen int64) error { + if err := verifyNginxGatewayConditions(ng); err != nil { + return err + } + + condition := ng.Status.Conditions[0] if condition.Type != "Valid" { return fmt.Errorf( @@ -106,7 +117,12 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Eventually( func() bool { - return verifyNginxGateway(nginxGatewayNsname, int64(1)) != nil + ng, err := getNginxGateway(nginxGatewayNsname) + if err != nil { + return false + } + + return verifyNginxGatewayStatus(ng, int64(1)) != nil }).WithTimeout(timeoutConfig.UpdateTimeout). WithPolling(500 * time.Millisecond). Should(BeTrue()) @@ -118,7 +134,13 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).To(Succeed()) + ng, err := getNginxGateway(nginxGatewayNsname) + Expect(err).ToNot(HaveOccurred()) + + gen, err := getNginxGatewayCurrentObservedGeneration(ng) + Expect(err).ToNot(HaveOccurred()) + + Expect(verifyNginxGatewayStatus(ng, gen)).To(Succeed()) Eventually( func() bool { @@ -147,7 +169,10 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).To(Succeed()) + ng, err := getNginxGateway(nginxGatewayNsname) + Expect(err).ToNot(HaveOccurred()) + + Expect(verifyNginxGatewayStatus(ng, int64(1))).To(Succeed()) Eventually( func() bool { @@ -177,8 +202,13 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f It("captures the change, the status is accepted and true,"+ " and the observed generation is incremented", func() { // previous test has left the log level at info, this test will change the log level to debug + ng, err := getNginxGateway(nginxGatewayNsname) + Expect(err).ToNot(HaveOccurred()) + + gen, err := getNginxGatewayCurrentObservedGeneration(ng) + Expect(err).ToNot(HaveOccurred()) - Expect(verifyNginxGateway(nginxGatewayNsname, int64(1))).To(Succeed()) + Expect(verifyNginxGatewayStatus(ng, gen)).To(Succeed()) logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ Container: "nginx-gateway", @@ -191,7 +221,12 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Eventually( func() bool { - return verifyNginxGateway(nginxGatewayNsname, int64(2)) != nil + ng, err := getNginxGateway(nginxGatewayNsname) + if err != nil { + return false + } + + return verifyNginxGatewayStatus(ng, gen+1) != nil }).WithTimeout(timeoutConfig.UpdateTimeout). WithPolling(500 * time.Millisecond). Should(BeTrue()) @@ -219,7 +254,12 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Eventually( func() error { - return verifyNginxGateway(nginxGatewayNsname, int64(0)) + _, err := getNginxGateway(nginxGatewayNsname) + if err != nil { + return err + } + + return nil }).WithTimeout(timeoutConfig.DeleteTimeout). WithPolling(500 * time.Millisecond). Should(MatchError("failed to get nginxGateway")) From 599b42a04a1fa50be290df7d7f7eb888a0d61408 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 4 Sep 2024 14:56:50 -0700 Subject: [PATCH 19/21] Wrap error and nit error messages --- tests/suite/nginxgateway_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 9a9c51750b..1e7fc8a49a 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -35,7 +35,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f var nginxGateway ngfAPI.NginxGateway if err := k8sClient.Get(ctx, nsname, &nginxGateway); err != nil { - return nginxGateway, errors.New("failed to get nginxGateway") + return nginxGateway, fmt.Errorf("failed to get nginxGateway: %w", err) } return nginxGateway, nil @@ -43,7 +43,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f verifyNginxGatewayConditions := func(ng ngfAPI.NginxGateway) error { if ng.Status.Conditions == nil { - return errors.New("nginxGateway is has no conditions") + return errors.New("nginxGateway has no conditions") } if len(ng.Status.Conditions) != 1 { @@ -262,7 +262,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f return nil }).WithTimeout(timeoutConfig.DeleteTimeout). WithPolling(500 * time.Millisecond). - Should(MatchError("failed to get nginxGateway")) + Should(MatchError(ContainSubstring("failed to get nginxGateway"))) Eventually( func() bool { From 9c5e517903755ea1cb68138d7c92838780473a42 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 4 Sep 2024 15:13:29 -0700 Subject: [PATCH 20/21] Add round of feedback --- tests/suite/nginxgateway_test.go | 37 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index 1e7fc8a49a..fbc5ecbf67 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -122,7 +122,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f return false } - return verifyNginxGatewayStatus(ng, int64(1)) != nil + return verifyNginxGatewayStatus(ng, int64(1)) == nil }).WithTimeout(timeoutConfig.UpdateTimeout). WithPolling(500 * time.Millisecond). Should(BeTrue()) @@ -130,17 +130,14 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f When("testing NGF on startup", func() { When("log level is set to debug", func() { - It("outputs debug logs and the status is accepted and true", func() { + It("outputs debug logs and the status is valid", func() { ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) ng, err := getNginxGateway(nginxGatewayNsname) Expect(err).ToNot(HaveOccurred()) - gen, err := getNginxGatewayCurrentObservedGeneration(ng) - Expect(err).ToNot(HaveOccurred()) - - Expect(verifyNginxGatewayStatus(ng, gen)).To(Succeed()) + Expect(verifyNginxGatewayStatus(ng, int64(1))).To(Succeed()) Eventually( func() bool { @@ -159,7 +156,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f }) When("default log level is used", func() { - It("only outputs info logs and the status is accepted and true", func() { + It("only outputs info logs and the status is valid", func() { teardown(releaseName) cfg := getDefaultSetupCfg() @@ -169,12 +166,19 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f ngfPodName, err := getNGFPodName() Expect(err).ToNot(HaveOccurred()) - ng, err := getNginxGateway(nginxGatewayNsname) - Expect(err).ToNot(HaveOccurred()) + Eventually( + func() bool { + ng, err := getNginxGateway(nginxGatewayNsname) + if err != nil { + return false + } - Expect(verifyNginxGatewayStatus(ng, int64(1))).To(Succeed()) + return verifyNginxGatewayStatus(ng, int64(1)) == nil + }).WithTimeout(timeoutConfig.UpdateTimeout). + WithPolling(500 * time.Millisecond). + Should(BeTrue()) - Eventually( + Consistently( func() bool { logs, err := resourceManager.GetPodLogs(ngfNamespace, ngfPodName, &core.PodLogOptions{ Container: "nginx-gateway", @@ -199,8 +203,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f }) When("NginxGateway is updated", func() { - It("captures the change, the status is accepted and true,"+ - " and the observed generation is incremented", func() { + It("captures the change, the status is valid, and the observed generation is incremented", func() { // previous test has left the log level at info, this test will change the log level to debug ng, err := getNginxGateway(nginxGatewayNsname) Expect(err).ToNot(HaveOccurred()) @@ -226,7 +229,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f return false } - return verifyNginxGatewayStatus(ng, gen+1) != nil + return verifyNginxGatewayStatus(ng, gen+1) == nil }).WithTimeout(timeoutConfig.UpdateTimeout). WithPolling(500 * time.Millisecond). Should(BeTrue()) @@ -255,11 +258,7 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f Eventually( func() error { _, err := getNginxGateway(nginxGatewayNsname) - if err != nil { - return err - } - - return nil + return err }).WithTimeout(timeoutConfig.DeleteTimeout). WithPolling(500 * time.Millisecond). Should(MatchError(ContainSubstring("failed to get nginxGateway"))) From da027f8f4768be75283543835f46b863bb47bf6b Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 4 Sep 2024 15:17:21 -0700 Subject: [PATCH 21/21] Add small format change --- tests/suite/nginxgateway_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/suite/nginxgateway_test.go b/tests/suite/nginxgateway_test.go index fbc5ecbf67..5e99b3774c 100644 --- a/tests/suite/nginxgateway_test.go +++ b/tests/suite/nginxgateway_test.go @@ -243,8 +243,10 @@ var _ = Describe("NginxGateway", Ordered, Label("functional", "nginxGateway"), f return false } - return strings.Contains(logs, - "\"current\":\"debug\",\"msg\":\"Log level changed\",\"prev\":\"info\"") + return strings.Contains( + logs, + "\"current\":\"debug\",\"msg\":\"Log level changed\",\"prev\":\"info\"", + ) }).WithTimeout(timeoutConfig.GetTimeout). WithPolling(500 * time.Millisecond). Should(BeTrue())