test: e2e: improve artifact capture, run more tests

We now disinguish between the data directories for kcp servers we start
and the artifacts that a test produces, as we only want to capture the
latter in CI for review when a test fails. This commit also adds the
artifact publishing to CI.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

.github/workflows/ci.yaml

@@ -78,7 +78,7 @@ jobs:
     - name: Check codegen
       run: make verify-codegen
     - run: make build
-    - run: PATH="${PATH}:$(pwd)/bin/" go test -race -coverprofile=coverage.txt -covermode=atomic ./...
+    - run: ARTIFACT_DIR=/tmp/e2e PATH="${PATH}:$(pwd)/bin/" go test -race -coverprofile=coverage.txt -covermode=atomic ./...
     # Because verify-codegen updates imports under the covers, and it affects
     # everything in the working directory, we have to check out kubernetes
     # after verifying codegen; otherwise, kubernetes code will get modified,
@@ -92,4 +92,9 @@ jobs:
     - run: make WHAT=test/e2e/e2e.test
       working-directory: ./kubernetes
     - run: WORKDIR=/tmp/e2e hack/run-sharded-kcp.sh
-    - run: WORKDIR=/tmp/cluster hack/run-cluster-kcp.sh
+    - run: WORKDIR=/tmp/cluster hack/run-cluster-kcp.sh
+    - uses: actions/upload-artifact@v2
+      if: ${{ always() }}
+      with:
+        name: e2e-artifacts
+        path: /tmp/e2e/**/artifacts/*

test/e2e/framework/kcp.go

@@ -43,6 +43,7 @@ import (
 type kcpServer struct {
 	args        []string
 	ctx         context.Context
+	dataDir     string
 	artifactDir string
 
 	lock *sync.Mutex
@@ -51,11 +52,7 @@ type kcpServer struct {
 	t TestingTInterface
 }
 
-func (c *kcpServer) ArtifactDir() string {
-	return c.artifactDir
-}
-
-func newKcpServer(t *T, cfg KcpConfig) (*kcpServer, error) {
+func newKcpServer(t *T, cfg KcpConfig, artifactDir, dataDir string) (*kcpServer, error) {
 	t.Helper()
 	ctx := context.Background()
 	if deadline, ok := t.Deadline(); ok {
@@ -78,18 +75,23 @@ func newKcpServer(t *T, cfg KcpConfig) (*kcpServer, error) {
 	if err != nil {
 		return nil, err
 	}
-	artifactDir, err := ArtifactDir(t, "kcp", cfg.Name)
-	if err != nil {
-		return nil, err
+	artifactDir = filepath.Join(artifactDir, "kcp", cfg.Name)
+	if err := os.MkdirAll(artifactDir, 0755); err != nil {
+		return nil, fmt.Errorf("could not create artifact dir: %w", err)
+	}
+	dataDir = filepath.Join(dataDir, "kcp", cfg.Name)
+	if err := os.MkdirAll(dataDir, 0755); err != nil {
+		return nil, fmt.Errorf("could not create data dir: %w", err)
 	}
 	return &kcpServer{
 		args: append([]string{
-			"--root_directory=" + artifactDir,
+			"--root_directory=" + dataDir,
 			"--listen=:" + kcpListenPort,
 			"--etcd_client_port=" + etcdClientPort,
 			"--etcd_peer_port=" + etcdPeerPort,
 			"--kubeconfig_path=admin.kubeconfig"},
 			cfg.Args...),
+		dataDir:     dataDir,
 		artifactDir: artifactDir,
 		ctx:         ctx,
 		t:           t,
@@ -231,7 +233,7 @@ func (c *kcpServer) loadCfg() error {
 	var loadError error
 	loadCtx, cancel := context.WithTimeout(c.ctx, 15*time.Second)
 	wait.UntilWithContext(loadCtx, func(ctx context.Context) {
-		kubeconfigPath := filepath.Join(c.artifactDir, "admin.kubeconfig")
+		kubeconfigPath := filepath.Join(c.dataDir, "admin.kubeconfig")
 		if _, err := os.Stat(kubeconfigPath); os.IsNotExist(err) {
 			return // try again
 		} else if err != nil {

test/e2e/framework/test.go

@@ -73,10 +73,16 @@ func Run(top *testing.T, name string, f TestFunc, cfgs ...KcpConfig) {
 		mid.Parallel()
 		ctx, cancel := context.WithCancel(context.Background())
 		bottom := NewT(ctx, mid)
+		artifactDir, dataDir, err := ScratchDirs(bottom)
+		if err != nil {
+			bottom.Errorf("failed to create scratch dirs: %v", err)
+			cancel()
+			return
+		}
 		var servers []*kcpServer
 		var runningServers []RunningServer
 		for _, cfg := range cfgs {
-			server, err := newKcpServer(bottom, cfg)
+			server, err := newKcpServer(bottom, cfg, artifactDir, dataDir)
 			if err != nil {
 				mid.Fatal(err)
 			}

test/e2e/framework/util.go

@@ -26,28 +26,32 @@ import (
 	"strings"
 )
 
-// ArtifactDir determines where artifacts should live for a test case.
-func ArtifactDir(t TestingTInterface, segments ...string) (string, error) {
+// ScratchDirs determines where artifacts and data should live for a test case.
+func ScratchDirs(t TestingTInterface) (string, string, error) {
 	var baseDir string
 	if dir, set := os.LookupEnv("ARTIFACT_DIR"); set {
 		baseDir = dir
 	} else {
 		baseDir = t.TempDir()
 	}
-	segments = append([]string{
-		baseDir, strings.NewReplacer("/", "_", "\\", "_", ":", "_").Replace(t.Name()),
-	}, segments...)
-	artifactDir := filepath.Join(segments...)
-	if info, err := os.Stat(artifactDir); err == nil {
-		if info.IsDir() {
-			return "", fmt.Errorf("artifact dir %s already exists, ensure clean output directory", artifactDir)
-		}
+	baseDir = filepath.Join(baseDir, strings.NewReplacer("\\", "_", ":", "_").Replace(t.Name()))
+	if err := os.MkdirAll(baseDir, 0755); err != nil {
+		return "", "", fmt.Errorf("could not create base dir: %w", err)
+	}
+	baseTempDir, err := os.MkdirTemp(baseDir, "")
+	if err != nil {
+		return "", "", fmt.Errorf("could not create base temp dir: %w", err)
 	}
-	if err := os.MkdirAll(artifactDir, 0755); err != nil {
-		return "", fmt.Errorf("could not create artifact dir: %w", err)
+	var directories []string
+	for _, prefix := range []string{"artifacts", "data"} {
+		dir := filepath.Join(baseTempDir, prefix)
+		if err := os.MkdirAll(dir, 0755); err != nil {
+			return "", "", fmt.Errorf("could not create subdir: %w", err)
+		}
+		directories = append(directories, dir)
 	}
-	t.Logf("Saving artifacts to %s.", artifactDir)
-	return artifactDir, nil
+	t.Logf("Saving test artifacts and data under %s.", baseTempDir)
+	return directories[0], directories[1], nil
 }
 
 // GetFreePort asks the kernel for a free open port that is ready to use.