diff --git a/e2e/tests/up/provider_docker.go b/e2e/tests/up/provider_docker.go index 55e583817..1ab507f38 100644 --- a/e2e/tests/up/provider_docker.go +++ b/e2e/tests/up/provider_docker.go @@ -476,6 +476,27 @@ var _ = ginkgo.Describe( ginkgo.SpecTimeout(framework.GetTimeout()), ) + ginkgo.It( + "initializeCommand with object syntax runs named sub-commands in parallel", + func(ctx context.Context) { + tempDir, err := dtc.setupAndUp(ctx, "tests/up/testdata/docker-initcmd-parallel") + framework.ExpectNoError(err) + + // Both initializeCommand sub-commands run on the host and write + // marker files into the workspace folder. + // #nosec G304 -- test path from setupAndUp + one, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-one.out")) + framework.ExpectNoError(err) + gomega.Expect(string(one)).To(gomega.Equal("initCmdOne")) + + // #nosec G304 -- test path from setupAndUp + two, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-two.out")) + framework.ExpectNoError(err) + gomega.Expect(string(two)).To(gomega.Equal("initCmdTwo")) + }, + ginkgo.SpecTimeout(framework.GetTimeout()), + ) + ginkgo.It("IDE accessible before postAttachCommand completes", func(ctx context.Context) { tempDir, err := setupWorkspace( "tests/up/testdata/docker-post-attach-nonblocking", diff --git a/e2e/tests/up/testdata/docker-initcmd-parallel/.devcontainer.json b/e2e/tests/up/testdata/docker-initcmd-parallel/.devcontainer.json new file mode 100644 index 000000000..3ca4f59fa --- /dev/null +++ b/e2e/tests/up/testdata/docker-initcmd-parallel/.devcontainer.json @@ -0,0 +1,7 @@ +{ + "image": "ghcr.io/devsy-org/test-images/go:1", + "initializeCommand": { + "write-one": "echo -n initCmdOne > ./init-cmd-one.out", + "write-two": ["sh", "-c", "echo -n initCmdTwo > ./init-cmd-two.out"] + } +} diff --git a/pkg/devcontainer/run.go b/pkg/devcontainer/run.go index b962e9d67..60dd1e76f 100644 --- a/pkg/devcontainer/run.go +++ b/pkg/devcontainer/run.go @@ -2,12 +2,14 @@ package devcontainer import ( "context" + "errors" "fmt" "io" "os" "os/exec" "runtime" "strings" + "sync" "time" "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -201,6 +203,14 @@ func isDockerFileConfig(config *config.DevContainerConfig) bool { return config.GetDockerfile() != "" } +// initCmdContext groups the shared state for running initializeCommand +// sub-commands. +type initCmdContext struct { + shellArgs []string + workspaceFolder string + extraEnvVars []string +} + func runInitializeCommand( workspaceFolder string, config *config.DevContainerConfig, @@ -210,51 +220,102 @@ func runInitializeCommand( return nil } - shellArgs := []string{"sh", "-c"} + ctx := initCmdContext{ + shellArgs: []string{"sh", "-c"}, + workspaceFolder: workspaceFolder, + extraEnvVars: extraEnvVars, + } // According to the devcontainer spec, `initializeCommand` needs to be run on the host. // On Windows we can't assume everyone has `sh` added to their PATH so we need to use Windows default shell (usually cmd.exe) if runtime.GOOS == "windows" { comSpec := os.Getenv("COMSPEC") if comSpec != "" { - shellArgs = []string{comSpec, "/c"} + ctx.shellArgs = []string{comSpec, "/c"} } else { - shellArgs = []string{"cmd.exe", "/c"} + ctx.shellArgs = []string{"cmd.exe", "/c"} } } - for _, cmd := range config.InitializeCommand { - // should run in shell? - var args []string - if len(cmd) == 1 { - args = []string{shellArgs[0], shellArgs[1], cmd[0]} - } else { - args = cmd - } + // When the hook has multiple named keys (object syntax), run + // sub-commands concurrently per the devcontainer spec, matching + // executeLifecycleHook in lifecyclehooks.go. + if len(config.InitializeCommand) > 1 { + return ctx.runParallel(config.InitializeCommand) + } - // run the command - log.Infof("Running initializeCommand from devcontainer.json: '%s'", strings.Join(args, " ")) - writer := log.Writer(log.LevelInfo) - errwriter := log.Writer(log.LevelError) - defer func() { _ = writer.Close() }() - defer func() { _ = errwriter.Close() }() - - cmd := exec.Command(args[0], args[1:]...) - env := cmd.Environ() - env = append(env, extraEnvVars...) - - cmd.Stdout = writer - cmd.Stderr = errwriter - cmd.Dir = workspaceFolder - cmd.Env = env - err := cmd.Run() - if err != nil { - return err - } + for name, cmd := range config.InitializeCommand { + return ctx.runSingle(name, cmd) } return nil } +func (c *initCmdContext) runParallel( + commands map[string][]string, +) error { + var ( + wg sync.WaitGroup + mu sync.Mutex + errs []error + ) + + wg.Add(len(commands)) + for name, cmd := range commands { + go func() { + defer wg.Done() + if err := c.runSingle(name, cmd); err != nil { + mu.Lock() + errs = append( + errs, + fmt.Errorf("named command %q failed: %w", name, err), + ) + mu.Unlock() + } + }() + } + + wg.Wait() + return errors.Join(errs...) +} + +func (c *initCmdContext) runSingle( + name string, + cmd []string, +) error { + var args []string + if len(cmd) == 1 { + args = []string{ + c.shellArgs[0], + c.shellArgs[1], + cmd[0], + } + } else { + args = cmd + } + + log.Infof( + "Running initializeCommand from devcontainer.json: %s '%s'", + name, + strings.Join(args, " "), + ) + writer := log.Writer(log.LevelInfo) + errwriter := log.Writer(log.LevelError) + defer func() { _ = writer.Close() }() + defer func() { _ = errwriter.Close() }() + + // #nosec G204 -- args come from devcontainer.json initializeCommand + execCmd := exec.Command(args[0], args[1:]...) + env := execCmd.Environ() + env = append(env, c.extraEnvVars...) + + execCmd.Stdout = writer + execCmd.Stderr = errwriter + execCmd.Dir = c.workspaceFolder + execCmd.Env = env + + return execCmd.Run() +} + func getWorkspace( workspaceFolder, workspaceID string, conf *config.DevContainerConfig, diff --git a/pkg/devcontainer/run_test.go b/pkg/devcontainer/run_test.go new file mode 100644 index 000000000..f980d07a9 --- /dev/null +++ b/pkg/devcontainer/run_test.go @@ -0,0 +1,97 @@ +package devcontainer + +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newInitCmdConfig(hook types.LifecycleHook) *config.DevContainerConfig { + return &config.DevContainerConfig{ + DevContainerConfigBase: config.DevContainerConfigBase{ + InitializeCommand: hook, + }, + } +} + +func TestRunInitializeCommand_ParallelTiming(t *testing.T) { + // Two "sleep 0.5" commands that, if run in parallel, complete in ~0.5s. + cfg := newInitCmdConfig(types.LifecycleHook{ + "sleep-a": {"sleep", "0.5"}, + "sleep-b": {"sleep", "0.5"}, + }) + + start := time.Now() + err := runInitializeCommand(t.TempDir(), cfg, nil) + elapsed := time.Since(start) + + assert.NoError(t, err) + assert.Less(t, elapsed, 900*time.Millisecond, + "two 0.5s commands should complete in ~0.5s when parallel, not ~1s") +} + +func TestRunInitializeCommand_ParallelErrorCollection(t *testing.T) { + dir := t.TempDir() + markerFile := filepath.Join(dir, "ran.txt") + + cfg := newInitCmdConfig(types.LifecycleHook{ + "fail": {"sh", "-c", "exit 1"}, + "succeed": {"sh", "-c", fmt.Sprintf("sleep 0.1 && echo done > %s", markerFile)}, + }) + + err := runInitializeCommand(dir, cfg, nil) + + // The combined error should mention which named command failed. + assert.Error(t, err) + assert.Contains(t, err.Error(), `named command "fail" failed`) + + // The succeed command should have run to completion despite the failure. + _, statErr := os.Stat(markerFile) + assert.NoError(t, statErr, "succeed command should run even when fail command errors") +} + +func TestRunInitializeCommand_SingleKey(t *testing.T) { + dir := t.TempDir() + outFile := filepath.Join(dir, "out.txt") + + cfg := newInitCmdConfig(types.LifecycleHook{ + "": {"sh", "-c", fmt.Sprintf("echo hello > %s", outFile)}, + }) + + err := runInitializeCommand(dir, cfg, nil) + require.NoError(t, err) + + data, err := os.ReadFile(outFile) // #nosec G304 -- test path from t.TempDir + require.NoError(t, err) + assert.Contains(t, string(data), "hello") +} + +func TestRunInitializeCommand_StringFormat(t *testing.T) { + dir := t.TempDir() + outFile := filepath.Join(dir, "out.txt") + + // String format results in a single key with a shell command string. + cfg := newInitCmdConfig(types.LifecycleHook{ + "": {fmt.Sprintf("echo hello > %s", outFile)}, + }) + + err := runInitializeCommand(dir, cfg, nil) + require.NoError(t, err) + + data, err := os.ReadFile(outFile) // #nosec G304 -- test path from t.TempDir + require.NoError(t, err) + assert.Contains(t, string(data), "hello") +} + +func TestRunInitializeCommand_Empty(t *testing.T) { + cfg := &config.DevContainerConfig{} + err := runInitializeCommand(t.TempDir(), cfg, nil) + assert.NoError(t, err) +} diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go index a9d76b851..85460c711 100644 --- a/pkg/inject/inject.go +++ b/pkg/inject/inject.go @@ -325,9 +325,17 @@ func pipe( // Wait for the first copy to complete — its error is the meaningful one. first := <-errChan - // Close both endpoints so the remaining goroutine unblocks promptly. + // Close all closable endpoints so the remaining goroutine unblocks + // promptly instead of leaking. _ = toStdin.Close() _ = fromStdout.Close() + if c, ok := fromStdin.(io.Closer); ok { + _ = c.Close() + } + + // Drain the second goroutine so callers can safely read toStdout after + // pipe returns without a data race. + <-errChan return first }