Add updates so functional tests pass

Author: bjee19

internal/mode/static/telemetry/collector.go

@@ -145,10 +145,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) {
 		return Data{}, fmt.Errorf("failed to collect cluster information: %w", err)
 	}
 
-	graphResourceCount, err := collectGraphResourceCount(g, c.cfg.ConfigurationGetter)
-	if err != nil {
-		return Data{}, fmt.Errorf("failed to collect NGF resource counts: %w", err)
-	}
+	graphResourceCount := collectGraphResourceCount(g, c.cfg.ConfigurationGetter)
 
 	replicaSet, err := getPodReplicaSet(ctx, c.cfg.K8sClientReader, c.cfg.PodNSName)
 	if err != nil {
@@ -193,13 +190,16 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) {
 func collectGraphResourceCount(
 	g *graph.Graph,
 	configurationGetter ConfigurationGetter,
-) (NGFResourceCounts, error) {
+) NGFResourceCounts {
 	ngfResourceCounts := NGFResourceCounts{}
 	cfg := configurationGetter.GetLatestConfiguration()
 
-	if cfg == nil {
-		return ngfResourceCounts, errors.New("latest configuration cannot be nil")
-	}
+	// might need to change this and make this optional, if cfg is nil then we can't report on upstream count but
+	// we can do everything else.
+
+	// if cfg == nil {
+	//	return ngfResourceCounts, errors.New("latest configuration cannot be nil")
+	//}
 
 	ngfResourceCounts.GatewayClassCount = int64(len(g.IgnoredGatewayClasses))
 	if g.GatewayClass != nil {
@@ -219,9 +219,11 @@ func collectGraphResourceCount(
 	ngfResourceCounts.SecretCount = int64(len(g.ReferencedSecrets))
 	ngfResourceCounts.ServiceCount = int64(len(g.ReferencedServices))
 
-	for _, upstream := range cfg.Upstreams {
-		if upstream.ErrorMsg == "" {
-			ngfResourceCounts.EndpointCount += int64(len(upstream.Endpoints))
+	if cfg != nil {
+		for _, upstream := range cfg.Upstreams {
+			if upstream.ErrorMsg == "" {
+				ngfResourceCounts.EndpointCount += int64(len(upstream.Endpoints))
+			}
 		}
 	}
 
@@ -249,7 +251,7 @@ func collectGraphResourceCount(
 	ngfResourceCounts.NginxProxyCount = int64(len(g.ReferencedNginxProxies))
 	ngfResourceCounts.SnippetsFilterCount = int64(len(g.SnippetsFilters))
 
-	return ngfResourceCounts, nil
+	return ngfResourceCounts
 }
 
 type RouteCounts struct {

internal/mode/static/telemetry/collector_test.go

@@ -728,14 +728,6 @@ var _ = Describe("Collector", Ordered, func() {
 					_, err := dataCollector.Collect(ctx)
 					Expect(err).To(MatchError(expectedError))
 				})
-
-				It("should error on nil latest configuration", func(ctx SpecContext) {
-					expectedError := errors.New("latest configuration cannot be nil")
-					fakeConfigurationGetter.GetLatestConfigurationReturns(nil)
-
-					_, err := dataCollector.Collect(ctx)
-					Expect(err).To(MatchError(expectedError))
-				})
 			})
 		})
 	})

tests/framework/collector.go

@@ -28,6 +28,14 @@ func InstallCollector() ([]byte, error) {
 		return output, err
 	}
 
+	if output, err := exec.Command(
+		"helm",
+		"repo",
+		"update",
+	).CombinedOutput(); err != nil {
+		return output, fmt.Errorf("failed to update helm repos: %w; output: %s", err, string(output))
+	}
+
 	args := []string{
 		"install",
 		collectorChartReleaseName,

tests/framework/crossplane.go

@@ -203,7 +203,7 @@ func injectCrossplaneContainer(
 func createCrossplaneExecutor(
 	k8sClient kubernetes.Interface,
 	k8sConfig *rest.Config,
-	ngfPodName,
+	nginxPodName,
 	namespace string,
 ) (remotecommand.Executor, error) {
 	cmd := []string{"./crossplane", "/etc/nginx/nginx.conf"}
@@ -217,7 +217,7 @@ func createCrossplaneExecutor(
 	req := k8sClient.CoreV1().RESTClient().Post().
 		Resource("pods").
 		SubResource("exec").
-		Name(ngfPodName).
+		Name(nginxPodName).
 		Namespace(namespace).
 		VersionedParams(opts, scheme.ParameterCodec)
 

tests/framework/portforward.go

@@ -52,13 +52,13 @@ func PortForward(config *rest.Config, namespace, podName string, ports []string,
 		for {
 			if err := forward(); err != nil {
 				slog.Error("error forwarding ports", "error", err)
-				slog.Info("retrying port forward in 100ms...")
+				slog.Info("retrying port forward in 1s...")
 			}
 
 			select {
 			case <-stopCh:
 				return
-			case <-time.After(100 * time.Millisecond):
+			case <-time.After(1 * time.Second):
 				// retrying
 			}
 		}

tests/framework/resourcemanager.go

@@ -719,6 +719,40 @@ func GetReadyNGFPodNames(
 	return nil, errors.New("unable to find NGF Pod(s)")
 }
 
+// GetReadyNginxPodNames returns the name(s) of the Nginx Pod(s).
+func GetReadyNginxPodNames(
+	k8sClient client.Client,
+	namespace string,
+	timeout time.Duration,
+) ([]string, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+
+	var podList core.PodList
+	if err := k8sClient.List(
+		ctx,
+		&podList,
+		client.InNamespace(namespace),
+		client.HasLabels{"gateway.networking.k8s.io/gateway-name"},
+	); err != nil {
+		return nil, fmt.Errorf("error getting list of Nginx Pods: %w", err)
+	}
+
+	if len(podList.Items) > 0 {
+		var names []string
+		for _, pod := range podList.Items {
+			for _, cond := range pod.Status.Conditions {
+				if cond.Type == core.PodReady && cond.Status == core.ConditionTrue {
+					names = append(names, pod.Name)
+				}
+			}
+		}
+		return names, nil
+	}
+
+	return nil, errors.New("unable to find NGF Pod(s)")
+}
+
 func countNumberOfReadyParents(parents []v1.RouteParentStatus) int {
 	readyCount := 0
 
@@ -760,7 +794,7 @@ func (rm *ResourceManager) WaitForAppsToBeReadyWithCtxWithPodCount(
 	return rm.waitForGatewaysToBeReady(ctx, namespace)
 }
 
-// WaitForPodsToBeReady waits for all Pods in the specified namespace to be ready or
+// WaitForPodsToBeReadyWithCount 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(
@@ -817,17 +851,17 @@ func (rm *ResourceManager) WaitForGatewayObservedGeneration(
 }
 
 // GetNginxConfig uses crossplane to get the nginx configuration and convert it to JSON.
-func (rm *ResourceManager) GetNginxConfig(ngfPodName, namespace string) (*Payload, error) {
+func (rm *ResourceManager) GetNginxConfig(nginxPodName, namespace string) (*Payload, error) {
 	if err := injectCrossplaneContainer(
 		rm.ClientGoClient,
 		rm.TimeoutConfig.UpdateTimeout,
-		ngfPodName,
+		nginxPodName,
 		namespace,
 	); err != nil {
 		return nil, err
 	}
 
-	exec, err := createCrossplaneExecutor(rm.ClientGoClient, rm.K8sConfig, ngfPodName, namespace)
+	exec, err := createCrossplaneExecutor(rm.ClientGoClient, rm.K8sConfig, nginxPodName, namespace)
 	if err != nil {
 		return nil, err
 	}

tests/suite/advanced_routing_test.go

@@ -39,9 +39,17 @@ var _ = Describe("AdvancedRouting", Ordered, Label("functional", "routing"), fun
 		Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed())
 		Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed())
 		Expect(resourceManager.WaitForAppsToBeReady(namespace)).To(Succeed())
+
+		nginxPodNames, err := framework.GetReadyNginxPodNames(k8sClient, namespace, timeoutConfig.GetTimeout)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(nginxPodNames).To(HaveLen(1))
+
+		setUpPortForward(nginxPodNames[0], namespace)
 	})
 
 	AfterAll(func() {
+		cleanUpPortForward()
+
 		Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed())
 		Expect(resourceManager.DeleteNamespace(namespace)).To(Succeed())
 	})

tests/suite/client_settings_test.go

@@ -32,6 +32,8 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy"
 		}
 
 		namespace = "clientsettings"
+
+		nginxPodName string
 	)
 
 	BeforeAll(func() {
@@ -44,9 +46,19 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy"
 		Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed())
 		Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed())
 		Expect(resourceManager.WaitForAppsToBeReady(namespace)).To(Succeed())
+
+		nginxPodNames, err := framework.GetReadyNginxPodNames(k8sClient, namespace, timeoutConfig.GetTimeout)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(nginxPodNames).To(HaveLen(1))
+
+		nginxPodName = nginxPodNames[0]
+
+		setUpPortForward(nginxPodName, namespace)
 	})
 
 	AfterAll(func() {
+		cleanUpPortForward()
+
 		Expect(resourceManager.DeleteNamespace(namespace)).To(Succeed())
 	})
 
@@ -96,13 +108,8 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy"
 			filePrefix := fmt.Sprintf("/etc/nginx/includes/ClientSettingsPolicy_%s", namespace)
 
 			BeforeAll(func() {
-				podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
-				Expect(err).ToNot(HaveOccurred())
-				Expect(podNames).To(HaveLen(1))
-
-				ngfPodName := podNames[0]
-
-				conf, err = resourceManager.GetNginxConfig(ngfPodName, ngfNamespace)
+				var err error
+				conf, err = resourceManager.GetNginxConfig(nginxPodName, namespace)
 				Expect(err).ToNot(HaveOccurred())
 			})
 

tests/suite/dataplane_perf_test.go

@@ -18,51 +18,60 @@ import (
 )
 
 var _ = Describe("Dataplane performance", Ordered, Label("nfr", "performance"), func() {
-	files := []string{
-		"dp-perf/coffee.yaml",
-		"dp-perf/gateway.yaml",
-		"dp-perf/cafe-routes.yaml",
-	}
-
-	var ns core.Namespace
-
-	var addr string
-	targetURL := "http://cafe.example.com"
-	var outFile *os.File
-
-	t1 := framework.Target{
-		Method: "GET",
-		URL:    fmt.Sprintf("%s%s", targetURL, "/latte"),
-	}
-	t2 := framework.Target{
-		Method: "GET",
-		URL:    fmt.Sprintf("%s%s", targetURL, "/coffee"),
-		Header: http.Header{"version": []string{"v2"}},
-	}
-	t3 := framework.Target{
-		Method: "GET",
-		URL:    fmt.Sprintf("%s%s", targetURL, "/coffee?TEST=v2"),
-	}
-	t4 := framework.Target{
-		Method: "GET",
-		URL:    fmt.Sprintf("%s%s", targetURL, "/tea"),
-	}
-	t5 := framework.Target{
-		Method: "POST",
-		URL:    fmt.Sprintf("%s%s", targetURL, "/tea"),
-	}
+	var (
+		files = []string{
+			"dp-perf/coffee.yaml",
+			"dp-perf/gateway.yaml",
+			"dp-perf/cafe-routes.yaml",
+		}
+
+		namespace = "dp-perf"
+
+		targetURL = "http://cafe.example.com"
+
+		t1 = framework.Target{
+			Method: "GET",
+			URL:    fmt.Sprintf("%s%s", targetURL, "/latte"),
+		}
+		t2 = framework.Target{
+			Method: "GET",
+			URL:    fmt.Sprintf("%s%s", targetURL, "/coffee"),
+			Header: http.Header{"version": []string{"v2"}},
+		}
+		t3 = framework.Target{
+			Method: "GET",
+			URL:    fmt.Sprintf("%s%s", targetURL, "/coffee?TEST=v2"),
+		}
+		t4 = framework.Target{
+			Method: "GET",
+			URL:    fmt.Sprintf("%s%s", targetURL, "/tea"),
+		}
+		t5 = framework.Target{
+			Method: "POST",
+			URL:    fmt.Sprintf("%s%s", targetURL, "/tea"),
+		}
+
+		outFile *os.File
+		addr    string
+	)
 
 	BeforeAll(func() {
-		ns = core.Namespace{
+		ns := core.Namespace{
 			ObjectMeta: metav1.ObjectMeta{
-				Name: "dp-perf",
+				Name: namespace,
 			},
 		}
 
 		Expect(resourceManager.Apply([]client.Object{&ns})).To(Succeed())
 		Expect(resourceManager.ApplyFromFiles(files, ns.Name)).To(Succeed())
 		Expect(resourceManager.WaitForAppsToBeReady(ns.Name)).To(Succeed())
 
+		nginxPodNames, err := framework.GetReadyNginxPodNames(k8sClient, namespace, timeoutConfig.GetTimeout)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(nginxPodNames).To(HaveLen(1))
+
+		setUpPortForward(nginxPodNames[0], namespace)
+
 		port := ":80"
 		if portFwdPort != 0 {
 			port = fmt.Sprintf(":%s", strconv.Itoa(portFwdPort))
@@ -79,8 +88,10 @@ var _ = Describe("Dataplane performance", Ordered, Label("nfr", "performance"),
 	})
 
 	AfterAll(func() {
-		Expect(resourceManager.DeleteFromFiles(files, ns.Name)).To(Succeed())
-		Expect(resourceManager.DeleteNamespace(ns.Name)).To(Succeed())
+		cleanUpPortForward()
+
+		Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed())
+		Expect(resourceManager.DeleteNamespace(namespace)).To(Succeed())
 		outFile.Close()
 	})