Merge #65831

65831: roachtest: zip the artifacts for each test on teamcity r=stevendanna a=tbg

- roachtest: don't duplicate log files
- roachtest: zip the artifacts for each test on teamcity


Co-authored-by: Tobias Grieger <[email protected]>

pkg/cmd/roachtest/cluster.go

@@ -1698,9 +1698,12 @@ func (c *cluster) FetchDebugZip(ctx context.Context) error {
 		// waste our time.
 		for i := 1; i <= c.spec.NodeCount; i++ {
 			// `./cockroach debug zip` is noisy. Suppress the output unless it fails.
+			//
+			// Ignore the files in the the log directory; we pull the logs separately anyway
+			// so this would only cause duplication.
 			si := strconv.Itoa(i)
 			output, err := execCmdWithBuffer(ctx, c.l, roachprod, "ssh", c.name+":"+si, "--",
-				"./cockroach", "debug", "zip", "--url", "{pgurl:"+si+"}", zipName)
+				"./cockroach", "debug", "zip", "--exclude-files='*.log,*.txt,*.pprof'", "--url", "{pgurl:"+si+"}", zipName)
 			if err != nil {
 				c.l.Printf("./cockroach debug zip failed: %s", output)
 				if i < c.spec.NodeCount {

pkg/cmd/roachtest/test_runner.go

@@ -11,6 +11,7 @@
 package main
 
 import (
+	"archive/zip"
 	"context"
 	"fmt"
 	"html"
@@ -710,6 +711,14 @@ caffeinate ./roachstress.sh %s
 		if teamCity {
 			shout(ctx, l, stdout, "##teamcity[testFinished name='%s' flowId='%s']", t.Name(), t.Name())
 
+			// Zip the artifacts. This improves the TeamCity UX where we can navigate
+			// through zip files just fine, but we can't download subtrees of the
+			// artifacts storage. By zipping we get this capability as we can just
+			// download the zip file for the failing test instead.
+			if err := zipArtifacts(t.artifactsDir); err != nil {
+				l.Printf("unable to zip artifacts: %s", err)
+			}
+
 			if t.artifactsSpec != "" {
 				// Tell TeamCity to collect this test's artifacts now. The TC job
 				// also collects the artifacts directory wholesale at the end, but
@@ -1254,3 +1263,77 @@ func (we *workerErrors) Err() error {
 	// error...
 	return we.mu.errs[0]
 }
+
+func zipArtifacts(path string) error {
+	f, err := os.Create(filepath.Join(path, "artifacts.zip"))
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+	z := zip.NewWriter(f)
+	rel := func(targetpath string) string {
+		relpath, err := filepath.Rel(path, targetpath)
+		if err != nil {
+			return targetpath
+		}
+		return relpath
+	}
+
+	walk := func(visitor func(string, os.FileInfo) error) error {
+		return filepath.Walk(path, func(path string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+			if !info.IsDir() && strings.HasSuffix(path, ".zip") {
+				// Skip any top-level zip files, which notably includes itself
+				// and, if present, the debug.zip.
+				return nil
+			}
+			return visitor(path, info)
+		})
+	}
+
+	// Zip all of the files.
+	if err := walk(func(path string, info os.FileInfo) error {
+		if info.IsDir() {
+			return nil
+		}
+		w, err := z.Create(rel(path))
+		if err != nil {
+			return err
+		}
+		r, err := os.Open(path)
+		if err != nil {
+			return err
+		}
+		defer r.Close()
+		if _, err := io.Copy(w, r); err != nil {
+			return err
+		}
+		return nil
+	}); err != nil {
+		return err
+	}
+	if err := z.Close(); err != nil {
+		return err
+	}
+	if err := f.Sync(); err != nil {
+		return err
+	}
+
+	// Now that the zip file is there, remove all of the files that went into it.
+	// Note that 'walk' skips the debug.zip and our newly written zip file.
+	root := path
+	return walk(func(path string, info os.FileInfo) error {
+		if path == root {
+			return nil
+		}
+		if err := os.RemoveAll(path); err != nil {
+			return err
+		}
+		if info.IsDir() {
+			return filepath.SkipDir
+		}
+		return nil
+	})
+}