Merge pull request #3589 from ntnn/dont-panic-outside-of-kcp

 Fix sdk/testing panicking when used outside of a kcp clone

Author: kcp-ci-bot

cmd/sharded-test-server/cache.go

@@ -52,7 +52,7 @@ func startCacheServer(ctx context.Context, logDirPath, workingDir, hostIP string
 	)
 	cacheWorkingDir := filepath.Join(workingDir, ".kcp-cache")
 	cachePort := 8012
-	commandLine := kcptestingserver.Command("cache-server", "cache")
+	workdir, commandLine := kcptestingserver.Command("cache-server", "cache")
 	commandLine = append(
 		commandLine,
 		fmt.Sprintf("--root-directory=%s", cacheWorkingDir),
@@ -64,6 +64,7 @@ func startCacheServer(ctx context.Context, logDirPath, workingDir, hostIP string
 	)
 	fmt.Fprintf(out, "running: %v\n", strings.Join(commandLine, " "))
 	cmd := exec.CommandContext(ctx, commandLine[0], commandLine[1:]...) //nolint:gosec
+	cmd.Dir = workdir
 
 	logFilePath := filepath.Join(".kcp-cache", "kcp.log")
 	if logDirPath != "" {

cmd/sharded-test-server/frontproxy.go

@@ -131,7 +131,8 @@ func startFrontProxy(
 	}
 
 	// run front-proxy command
-	commandLine := append(kcptestingserver.Command("kcp-front-proxy", "front-proxy"),
+	workdir, commandLine := kcptestingserver.Command("kcp-front-proxy", "front-proxy")
+	commandLine = append(commandLine,
 		"--bind-address="+hostIP,
 		fmt.Sprintf("--mapping-file=%s", filepath.Join(workDirPath, ".kcp-front-proxy", "mapping.yaml")),
 		fmt.Sprintf("--root-directory=%s", filepath.Join(workDirPath, ".kcp-front-proxy")),
@@ -147,6 +148,7 @@ func startFrontProxy(
 	fmt.Fprintf(out, "running: %v\n", strings.Join(commandLine, " "))
 
 	cmd := exec.CommandContext(ctx, commandLine[0], commandLine[1:]...) //nolint:gosec
+	cmd.Dir = workdir
 
 	logFilePath := filepath.Join(workDirPath, ".kcp-front-proxy", "proxy.log")
 	if logDirPath != "" {

cmd/sharded-test-server/virtual.go

@@ -188,7 +188,7 @@ func (v *VirtualWorkspace) start(ctx context.Context) error {
 		auditFilePath = filepath.Join(v.logDirPath, fmt.Sprintf("kcp-virtual-workspaces-%d-audit.log", v.index))
 	}
 
-	commandLine := kcptestingserver.Command("virtual-workspaces", strings.ToLower(prefix))
+	workdir, commandLine := kcptestingserver.Command("virtual-workspaces", strings.ToLower(prefix))
 	commandLine = append(commandLine, v.args...)
 	commandLine = append(
 		commandLine,
@@ -202,6 +202,7 @@ func (v *VirtualWorkspace) start(ctx context.Context) error {
 	fmt.Fprintf(out, "running: %v\n", strings.Join(commandLine, " "))
 
 	cmd := exec.CommandContext(ctx, commandLine[0], commandLine[1:]...) //nolint:gosec
+	cmd.Dir = workdir
 
 	if err := os.MkdirAll(filepath.Dir(logFilePath), 0755); err != nil {
 		return err

cmd/test-server/kcp/shard.go

@@ -94,8 +94,7 @@ func (s *Shard) Start(ctx context.Context, quiet bool) error {
 	}
 
 	// setup command
-	var commandLine []string
-	commandLine = append(commandLine, kcptestingserver.StartKcpCommand(s.name)...)
+	workdir, commandLine := kcptestingserver.StartKcpCommand(s.name)
 	commandLine = append(commandLine, s.args...)
 	commandLine = append(commandLine,
 		"--root-directory", s.runtimeDir,
@@ -114,6 +113,7 @@ func (s *Shard) Start(ctx context.Context, quiet bool) error {
 	fmt.Fprintf(out, "running: %v\n", strings.Join(commandLine, " "))
 
 	cmd := exec.CommandContext(ctx, commandLine[0], commandLine[1:]...) //nolint:gosec
+	cmd.Dir = workdir
 	if err := os.MkdirAll(filepath.Dir(s.logFilePath), 0755); err != nil {
 		return err
 	}

sdk/testing/helpers/repo.go

@@ -24,7 +24,7 @@ import (
 )
 
 // RepositoryDir returns the absolute path of kcp-dev/kcp repository on disk.
-func RepositoryDir() string {
+func RepositoryDir() (string, error) {
 	// Caller(0) returns the path to the calling test file rather than the path to this framework file. That
 	// precludes assuming how many directories are between the file and the repo root. It's therefore necessary
 	// to search in the hierarchy for an indication of a path that looks like the repo root.
@@ -35,18 +35,23 @@ func RepositoryDir() string {
 		if _, err := os.Stat(filepath.Join(currentDir, ".git")); err == nil {
 			break
 		} else if errors.Is(err, os.ErrNotExist) {
+			preDir := currentDir
 			currentDir, err = filepath.Abs(filepath.Join(currentDir, ".."))
 			if err != nil {
-				panic(err)
+				return "", err
+			}
+			if preDir == currentDir {
+				return "", errors.New("could not find kcp repository root")
 			}
 		} else {
-			panic(err)
+			return "", err
 		}
 	}
-	return currentDir
+	return currentDir, nil
 }
 
 // RepositoryBinDir returns the absolute path of <repo-dir>/bin. That's where `make build` produces our binaries.
-func RepositoryBinDir() string {
-	return filepath.Join(RepositoryDir(), "bin")
+func RepositoryBinDir() (string, error) {
+	repoDir, err := RepositoryDir()
+	return filepath.Join(repoDir, "bin"), err
 }

sdk/testing/server/fixture.go

@@ -191,42 +191,43 @@ func newKcpServer(t TestingT, cfg Config) (*kcpServer, error) {
 	return s, nil
 }
 
-// StartKcpCommand returns the string tokens required to start kcp in
-// the currently configured mode (direct or via `go run`).
-func StartKcpCommand(identity string) []string {
-	command := Command("kcp", identity)
-	return append(command, "start")
+// StartKcpCommand returns the work dir and string tokens required to
+// start kcp in the currently configured mode (direct or via `go run`).
+func StartKcpCommand(identity string) (string, []string) {
+	workdir, command := Command("kcp", identity)
+	return workdir, append(command, "start")
 }
 
-// Command returns the string tokens required to start
-// the given executable in the currently configured mode (direct or
-// via `go run`).
-func Command(executableName, identity string) []string {
-	if env.RunDelveEnvSet() {
-		cmdPath := filepath.Join(kcptestinghelpers.RepositoryDir(), "cmd", executableName)
-		return []string{"dlv", "debug", "--api-version=2", "--headless", fmt.Sprintf("--listen=unix:dlv-%s.sock", identity), cmdPath, "--"}
+// Command returns the work dir and string tokens required to start the
+// given executable in the currently configured mode (direct or via `go
+// run`).
+func Command(executableName, identity string) (string, []string) {
+	if env.NoGoRunEnvSet() {
+		return "", []string{executableName}
 	}
 
-	// are we inside of the kcp repository?
-	repo := kcptestinghelpers.RepositoryDir()
-	wd, err := os.Getwd()
+	// Check if this is a clone of the kcp repository. If not return the
+	// executable as is, expecting the user to have it in PATH.
+	repo, err := kcptestinghelpers.RepositoryDir()
 	if err != nil {
-		panic(err)
+		return "", []string{executableName}
 	}
-	inKcp := strings.HasPrefix(wd, repo+"/")
 
-	binary := executableName
-	if binDir := os.Getenv(kcpBinariesDirEnvDir); binDir == "" && inKcp {
-		binary = filepath.Join(kcptestinghelpers.RepositoryBinDir(), executableName)
-	} else if binDir != "" {
-		binary = filepath.Join(binDir, executableName)
+	cmdDir := filepath.Join("cmd", executableName)
+	fullPath := filepath.Join(repo, cmdDir)
+	cmdDir = "./" + cmdDir
+
+	if _, err := os.Stat(fullPath); os.IsNotExist(err) {
+		// The command dir (e.g. $repo/cmd/kcp) does not exist, default
+		// to the executable name and expect it to be in PATH.
+		return "", []string{executableName}
 	}
 
-	if env.NoGoRunEnvSet() || !inKcp {
-		return []string{binary}
+	if env.RunDelveEnvSet() {
+		return repo, []string{"dlv", "debug", "--api-version=2", "--headless", fmt.Sprintf("--listen=unix:dlv-%s.sock", identity), cmdDir, "--"}
 	}
 
-	return []string{"go", "run", filepath.Join(repo, "cmd", executableName)}
+	return repo, []string{"go", "run", cmdDir}
 }
 
 // Run runs the kcp server while the parent context is active. This call is not blocking,
@@ -301,13 +302,17 @@ func runExternal(ctx context.Context, t TestingT, cfg Config) (<-chan struct{},
 		return nil, fmt.Errorf("failed to build kcp args: %w", err)
 	}
 
-	commandLine := append(StartKcpCommand("KCP"), args...)
+	workdir, commandLine := StartKcpCommand("KCP")
+	commandLine = append(commandLine, args...)
 
 	t.Logf("running: %v", strings.Join(commandLine, " "))
 
 	// NOTE: do not use exec.CommandContext here. That method issues a SIGKILL when the context is done, and we
 	// want to issue SIGTERM instead, to give the server a chance to shut down cleanly.
 	cmd := exec.Command(commandLine[0], commandLine[1:]...) //nolint:gosec // G204: This is a test utility with controlled inputs
+	if workdir != "" {
+		cmd.Dir = workdir
+	}
 
 	// Create a new process group for the child/forked process (which is either 'go run ...' or just 'kcp
 	// ...'). This is necessary so the SIGTERM we send to terminate the kcp server works even with the

sdk/testing/server/metrics.go

@@ -74,7 +74,9 @@ func scrapeMetricsForServer(t TestingT, srv RunningServer) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), wait.ForeverTestTimeout)
 	defer cancel()
-	require.NoError(t, ScrapeMetrics(ctx, srv.RootShardSystemMasterBaseConfig(t), promUrl, kcptestinghelpers.RepositoryDir(), jobName, filepath.Join(srv.CADirectory(), "apiserver.crt"), labels))
+	repoDir, err := kcptestinghelpers.RepositoryDir()
+	require.NoError(t, err)
+	require.NoError(t, ScrapeMetrics(ctx, srv.RootShardSystemMasterBaseConfig(t), promUrl, repoDir, jobName, filepath.Join(srv.CADirectory(), "apiserver.crt"), labels))
 }
 
 func ScrapeMetrics(ctx context.Context, cfg *rest.Config, promUrl, promCfgDir, jobName, caFile string, labels map[string]string) error {

test/e2e/framework/flags.go

@@ -41,7 +41,11 @@ func complete() {
 		panic(errors.New("only one of --use-default-kcp-server and --kcp-kubeconfig should be set"))
 	}
 	if testConfig.useDefaultKCPServer {
-		testConfig.kcpKubeconfig = filepath.Join(kcptestinghelpers.RepositoryDir(), ".kcp", "admin.kubeconfig")
+		repo, err := kcptestinghelpers.RepositoryDir()
+		if err != nil {
+			panic(err)
+		}
+		testConfig.kcpKubeconfig = filepath.Join(repo, ".kcp", "admin.kubeconfig")
 	}
 	if len(testConfig.kcpKubeconfig) > 0 && len(testConfig.shardKubeconfigs) == 0 {
 		testConfig.shardKubeconfigs = map[string]string{corev1alpha1.RootShard: testConfig.kcpKubeconfig}

test/e2e/framework/kcp.go

@@ -25,9 +25,16 @@ import (
 )
 
 // DefaultTokenAuthFile returns the default auth tokens file.
-var DefaultTokenAuthFile = filepath.Join(kcptestinghelpers.RepositoryDir(), "test", "e2e", "framework", "auth-tokens.csv")
+var DefaultTokenAuthFile string
 
 func init() {
+	repo, err := kcptestinghelpers.RepositoryDir()
+	if err != nil {
+		panic(err)
+	}
+
+	DefaultTokenAuthFile = filepath.Join(repo, "test", "e2e", "framework", "auth-tokens.csv")
+
 	var args []string
 	args = append(args, "--token-auth-file", DefaultTokenAuthFile) //nolint:gocritic // no.
 	args = append(args, "--feature-gates=WorkspaceMounts=true,CacheAPIs=true,WorkspaceAuthentication=true")

test/e2e/framework/kubectl.go

@@ -40,7 +40,11 @@ func KcpCliPluginCommand() []string {
 	if env.NoGoRunEnvSet() {
 		return []string{"kubectl", "kcp"}
 	} else {
-		cmdPath := filepath.Join(kcptestinghelpers.RepositoryDir(), "cmd", "kubectl-kcp")
+		repo, err := kcptestinghelpers.RepositoryDir()
+		if err != nil {
+			panic(err)
+		}
+		cmdPath := filepath.Join(repo, "cmd", "kubectl-kcp")
 		return []string{"go", "run", cmdPath}
 	}
 }