Code review; remove results

Author: sjberman

tests/framework/load.go

@@ -31,22 +31,25 @@ func convertTargetToVegetaTarget(targets []Target) []vegeta.Target {
 	return vegTargets
 }
 
+// LoadTestConfig is the configuration to run a load test.
+type LoadTestConfig struct {
+	Description string
+	Proxy       string
+	ServerName  string
+	Targets     []Target
+	Rate        int
+	Duration    time.Duration
+}
+
 // Metrics is a wrapper around the vegeta Metrics.
 type Metrics struct {
 	vegeta.Metrics
 }
 
 // RunLoadTest uses Vegeta to send traffic to the provided Targets at the given rate for the given duration and writes
 // the results to the provided file
-func RunLoadTest(
-	targets []Target,
-	rate int,
-	duration time.Duration,
-	desc,
-	proxy,
-	serverName string,
-) (vegeta.Results, Metrics) {
-	vegTargets := convertTargetToVegetaTarget(targets)
+func RunLoadTest(cfg LoadTestConfig) (vegeta.Results, Metrics) {
+	vegTargets := convertTargetToVegetaTarget(cfg.Targets)
 	targeter := vegeta.NewStaticTargeter(vegTargets...)
 
 	dialer := &net.Dialer{
@@ -58,11 +61,11 @@ func RunLoadTest(
 		Timeout: vegeta.DefaultTimeout,
 		Transport: &http.Transport{
 			DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
-				return dialer.DialContext(ctx, network, proxy)
+				return dialer.DialContext(ctx, network, cfg.Proxy)
 			},
 			TLSClientConfig: &tls.Config{
 				InsecureSkipVerify: true, //nolint:gosec // self-signed cert for testing
-				ServerName:         serverName,
+				ServerName:         cfg.ServerName,
 			},
 			MaxIdleConnsPerHost: vegeta.DefaultConnections,
 			MaxConnsPerHost:     vegeta.DefaultMaxConnections,
@@ -71,10 +74,10 @@ func RunLoadTest(
 
 	attacker := vegeta.NewAttacker(vegeta.Client(&httpClient))
 
-	r := vegeta.Rate{Freq: rate, Per: time.Second}
+	r := vegeta.Rate{Freq: cfg.Rate, Per: time.Second}
 	var results vegeta.Results
 	var metrics vegeta.Metrics
-	for res := range attacker.Attack(targeter, r, duration, desc) {
+	for res := range attacker.Attack(targeter, r, cfg.Duration, cfg.Description) {
 		results = append(results, *res)
 		metrics.Add(res)
 	}

tests/framework/resourcemanager.go

@@ -409,8 +409,8 @@ func (rm *ResourceManager) GetClusterInfo() (ClusterInfo, error) {
 	return *ci, nil
 }
 
-// GetNGFPodNames returns the name(s) of the NGF Pod(s).
-func GetNGFPodNames(
+// GetReadyNGFPodNames returns the name(s) of the NGF Pod(s).
+func GetReadyNGFPodNames(
 	k8sClient client.Client,
 	namespace,
 	releaseName string,

tests/results/ngf-upgrade/s.berman/http.png

@@ -87,14 +87,15 @@ var _ = Describe("Dataplane performance", Ordered, Label("performance"), func()
 			_, err := fmt.Fprint(outFile, text)
 			Expect(err).ToNot(HaveOccurred())
 
-			_, metrics := framework.RunLoadTest(
-				[]framework.Target{target},
-				1000,
-				30*time.Second,
-				description,
-				addr,
-				"cafe.example.com",
-			)
+			cfg := framework.LoadTestConfig{
+				Targets:     []framework.Target{target},
+				Rate:        1000,
+				Duration:    30 * time.Second,
+				Description: description,
+				Proxy:       addr,
+				ServerName:  "cafe.example.com",
+			}
+			_, metrics := framework.RunLoadTest(cfg)
 
 			Expect(framework.WriteResults(outFile, &metrics)).To(Succeed())
 

tests/results/ngf-upgrade/s.berman/https.png

@@ -140,7 +140,7 @@ func setup(cfg setupConfig, extraInstallArgs ...string) {
 	output, err = framework.InstallNGF(installCfg, extraInstallArgs...)
 	Expect(err).ToNot(HaveOccurred(), string(output))
 
-	podNames, err := framework.GetNGFPodNames(
+	podNames, err := framework.GetReadyNGFPodNames(
 		k8sClient,
 		installCfg.Namespace,
 		installCfg.ReleaseName,

tests/results/ngf-upgrade/s.berman/s.berman.md

@@ -145,14 +145,16 @@ var _ = Describe("Upgrade testing", Label("upgrade"), func() {
 				defer GinkgoRecover()
 				defer wg.Done()
 
-				results, metrics := framework.RunLoadTest(
-					[]framework.Target{cfg.target},
-					100,
-					60*time.Second,
-					cfg.desc,
-					fmt.Sprintf("%s:%s", address, cfg.port),
-					"cafe.example.com",
-				)
+				loadTestCfg := framework.LoadTestConfig{
+					Targets:     []framework.Target{cfg.target},
+					Rate:        100,
+					Duration:    60 * time.Second,
+					Description: cfg.desc,
+					Proxy:       fmt.Sprintf("%s:%s", address, cfg.port),
+					ServerName:  "cafe.example.com",
+				}
+
+				results, metrics := framework.RunLoadTest(loadTestCfg)
 
 				scheme := strings.Split(cfg.target.URL, "://")[0]
 				metricsRes := metricsResults{
@@ -196,20 +198,34 @@ var _ = Describe("Upgrade testing", Label("upgrade"), func() {
 
 		Expect(resourceManager.ApplyFromFiles([]string{"ngf-upgrade/gateway-updated.yaml"}, ns.Name)).To(Succeed())
 
-		podNames, err := framework.GetNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
+		podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(podNames).ToNot(HaveLen(0))
 
 		// ensure that the leader election lease has been updated to the new pods
-		leaseCtx, leaseCancel := context.WithTimeout(context.Background(), timeoutConfig.GetTimeout)
+		leaseCtx, leaseCancel := context.WithTimeout(context.Background(), 1*time.Minute)
 		defer leaseCancel()
 
 		var lease coordination.Lease
 		key := types.NamespacedName{Name: "ngf-test-nginx-gateway-fabric-leader-election", Namespace: ngfNamespace}
-		Expect(k8sClient.Get(leaseCtx, key, &lease)).To(Succeed())
+		Expect(wait.PollUntilContextCancel(
+			leaseCtx,
+			500*time.Millisecond,
+			true, /* poll immediately */
+			func(ctx context.Context) (bool, error) {
+				Expect(k8sClient.Get(leaseCtx, key, &lease)).To(Succeed())
 
-		Expect(lease.Spec.HolderIdentity).ToNot(BeNil())
-		Expect(podNames).To(ContainElement(*lease.Spec.HolderIdentity))
+				if lease.Spec.HolderIdentity != nil {
+					for _, podName := range podNames {
+						if podName == *lease.Spec.HolderIdentity {
+							return true, nil
+						}
+					}
+				}
+
+				return false, nil
+			},
+		)).To(Succeed())
 
 		// ensure that the Gateway has been properly updated with a new listener
 		gwCtx, gwCancel := context.WithTimeout(context.Background(), 1*time.Minute)