chore: enable lint for benchmark (envoyproxy#3754)

* chore: enable lint for benchmark Signed-off-by: zirain <[email protected]> * update Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]>
Xunzhuo · Jul 5, 2024 · df5c265 · df5c265
1 parent d6b5415
commit df5c265
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 18 deletions.
diff --git a/test/benchmark/suite/render.go b/test/benchmark/suite/render.go
@@ -110,7 +110,7 @@ func newMarkdownStyleTableWriter(writer io.Writer) *tabwriter.Writer {
 }
 
 func renderEnvSettingsTable(writer io.Writer) {
-	_, _ = fmt.Fprintln(writer, "Benchmark test settings:", "\n")
+	_, _ = fmt.Fprintln(writer, "Benchmark test settings:")
 
 	table := newMarkdownStyleTableWriter(writer)
 
@@ -144,7 +144,7 @@ func renderEnvSettingsTable(writer io.Writer) {
 	renderMetricsTableHeader(table, headers)
 
 	writeTableRow(table, headers, func(_ int, h ReportTableHeader) string {
-		env := strings.Replace(strings.ToUpper(h.Name), " ", "_", -1)
+		env := strings.ReplaceAll(strings.ToUpper(h.Name), " ", "_")
 		if v, ok := os.LookupEnv(benchmarkEnvPrefix + env); ok {
 			return v
 		}
@@ -260,13 +260,15 @@ func writeSection(writer io.Writer, title string, level int, content string) {
 
 // writeCollapsibleSection writes one collapsible section in Markdown style.
 func writeCollapsibleSection(writer io.Writer, title string, content []byte) {
-	_, _ = fmt.Fprintln(writer, fmt.Sprintf(`
+	summary := fmt.Sprintf("```plaintext\n%s\n```", content)
+	_, _ = fmt.Fprintf(writer, `
 <details>
 <summary>%s</summary>
 
 %s
 
-</details>`, title, fmt.Sprintf("```plaintext\n%s\n```", content)))
+</details>
+`, title, summary)
 }
 
 // writeTableRow writes one row in Markdown table style.

diff --git a/test/benchmark/suite/report.go b/test/benchmark/suite/report.go
@@ -81,6 +81,9 @@ func (r *BenchmarkReport) Collect(t *testing.T, ctx context.Context, job *types.
 
 func (r *BenchmarkReport) GetBenchmarkResult(t *testing.T, ctx context.Context, job *types.NamespacedName) error {
 	pods, err := r.kubeClient.Kube().CoreV1().Pods(job.Namespace).List(ctx, metav1.ListOptions{LabelSelector: "job-name=" + job.Name})
+	if err != nil {
+		return err
+	}
 
 	if len(pods.Items) < 1 {
 		return fmt.Errorf("failed to get any pods for job %s", job.String())
@@ -183,26 +186,28 @@ func (r *BenchmarkReport) getLogsFromPod(ctx context.Context, pod *types.Namespa
 func (r *BenchmarkReport) getMetricsFromPortForwarder(t *testing.T, pod *types.NamespacedName, url string) ([]byte, error) {
 	fw, err := kube.NewLocalPortForwarder(r.kubeClient, *pod, localMetricsPort, controlPlaneMetricsPort)
 	if err != nil {
-		return nil, fmt.Errorf("failed to build port forwarder for pod %s: %v", pod.String(), err)
+		return nil, fmt.Errorf("failed to build port forwarder for pod %s: %w", pod.String(), err)
 	}
 
 	if err = fw.Start(); err != nil {
 		fw.Stop()
 
-		return nil, fmt.Errorf("failed to start port forwarder for pod %s: %v", pod.String(), err)
+		return nil, fmt.Errorf("failed to start port forwarder for pod %s: %w", pod.String(), err)
 	}
+	requestURL := fmt.Sprintf("http://%s%s", fw.Address(), url)
 
 	var out []byte
 	// Retrieving metrics from Pod.
-	go func() {
+	go func(requestURL string) {
 		defer fw.Stop()
 
-		url := fmt.Sprintf("http://%s%s", fw.Address(), url)
-		resp, err := http.Get(url)
+		//nolint: gosec
+		resp, err := http.Get(requestURL)
 		if err != nil {
-			t.Errorf("failed to request %s: %v", url, err)
+			t.Errorf("failed to request %s: %v", requestURL, err)
 			return
 		}
+		defer resp.Body.Close()
 
 		metrics, err := io.ReadAll(resp.Body)
 		if err != nil {
@@ -211,7 +216,7 @@ func (r *BenchmarkReport) getMetricsFromPortForwarder(t *testing.T, pod *types.N
 		}
 
 		out = metrics
-	}()
+	}(requestURL)
 
 	fw.WaitForStop()
 

diff --git a/test/benchmark/suite/suite.go b/test/benchmark/suite/suite.go
@@ -50,7 +50,8 @@ type BenchmarkTestSuite struct {
 }
 
 func NewBenchmarkTestSuite(client client.Client, options BenchmarkOptions,
-	gatewayManifest, httpRouteManifest, benchmarkClientManifest, reportPath string) (*BenchmarkTestSuite, error) {
+	gatewayManifest, httpRouteManifest, benchmarkClientManifest, reportPath string,
+) (*BenchmarkTestSuite, error) {
 	var (
 		gateway         = new(gwapiv1.Gateway)
 		httproute       = new(gwapiv1.HTTPRoute)
@@ -132,13 +133,13 @@ func (b *BenchmarkTestSuite) Run(t *testing.T, tests []BenchmarkTest) {
 	}
 
 	if len(b.ReportSavePath) > 0 {
-		if err := os.WriteFile(b.ReportSavePath, writer.Bytes(), 0644); err != nil {
+		if err := os.WriteFile(b.ReportSavePath, writer.Bytes(), 0o600); err != nil {
 			t.Errorf("Error writing report to path '%s': %v", b.ReportSavePath, err)
 		} else {
 			t.Logf("Writing report to path '%s' successfully", b.ReportSavePath)
 		}
 	} else {
-		t.Log(fmt.Sprintf("%s", writer.Bytes()))
+		t.Logf("%s", writer.Bytes())
 	}
 }
 

diff --git a/test/benchmark/tests/scale_httproutes.go b/test/benchmark/tests/scale_httproutes.go
@@ -78,7 +78,7 @@ var ScaleHTTPRoutes = suite.BenchmarkTest{
 		})
 
 		t.Run("scaling down httproutes", func(t *testing.T) {
-			var start = routeScales[routeScalesN-1]
+			start := routeScales[routeScalesN-1]
 
 			for i := routeScalesN - 2; i >= 0; i-- {
 				scale := routeScales[i]
@@ -89,8 +89,7 @@ var ScaleHTTPRoutes = suite.BenchmarkTest{
 						routeNNs = routeNNs[:len(routeNNs)-1]
 
 						// Making sure we are deleting the right one route.
-						require.Equal(t, routeNN,
-							types.NamespacedName{Name: route.Name, Namespace: route.Namespace})
+						require.Equal(t, types.NamespacedName{Name: route.Name, Namespace: route.Namespace}, routeNN)
 
 						t.Logf("Delete HTTPRoute: %s", routeNN.String())
 					})

diff --git a/tools/make/lint.mk b/tools/make/lint.mk
@@ -5,6 +5,7 @@
 ##@ Lint
 
 GITHUB_ACTION ?=
+LINT_BUILD_TAGS ?= e2e,celvalidation,conformance,experimental,benchmark
 
 .PHONY: lint
 lint: ## Run all linter of code sources, including golint, yamllint, whitenoise lint and codespell.
@@ -20,7 +21,7 @@ lint: lint.golint
 lint-deps: $(tools/golangci-lint)
 lint.golint: $(tools/golangci-lint)
 	@$(LOG_TARGET)
-	$(tools/golangci-lint) run $(GOLANGCI_LINT_FLAGS) --build-tags=e2e,celvalidation,conformance,experimental --config=tools/linter/golangci-lint/.golangci.yml
+	$(tools/golangci-lint) run $(GOLANGCI_LINT_FLAGS) --build-tags=$(LINT_BUILD_TAGS) --config=tools/linter/golangci-lint/.golangci.yml
 
 .PHONY: lint.yamllint
 lint: lint.yamllint