diff --git a/docs/guides/executables.md b/docs/guides/executables.md index 9173ac00..37a6d97f 100644 --- a/docs/guides/executables.md +++ b/docs/guides/executables.md @@ -324,9 +324,28 @@ exec: network: host ``` +`interpreter: python` works with `container` too — combine them to get a pinned Python toolchain +without installing it locally: + +```yaml +exec: + interpreter: python + cmd: | + import sys + print(sys.version) + container: + image: python:3.13-alpine +``` + Notes and limitations: - By default flow overrides the image entrypoint with `sh` so `cmd` behaves as a shell command on - any image. Set `entrypoint: ""` to use the image's own `ENTRYPOINT`. + any image. With `interpreter: python` the default entrypoint becomes `python3` instead. Set + `entrypoint: ""` to use the image's own `ENTRYPOINT` — with Python that only works if the image's + `ENTRYPOINT` is itself an interpreter. +- Host interpreter discovery does not apply inside a container: the image's own `python3` is used, + and `VIRTUAL_ENV`, `PYTHONPATH`, `PYTHONHOME`, and `FLOW_PYTHON_BIN` are dropped from the + container environment because those host paths mean nothing inside it. Install dependencies in + the image, or mount them with `volumes`. - On Linux, flow runs as your host user by default so mounted files are not root-owned. Set `user: root` to opt out. - `.bat`, `.cmd`, and `.ps1` files are not supported with `container`. diff --git a/internal/runner/exec/container.go b/internal/runner/exec/container.go index 64e20b71..6d05ad65 100644 --- a/internal/runner/exec/container.go +++ b/internal/runner/exec/container.go @@ -27,25 +27,34 @@ const containerScriptMount = "/flow/script" // elsewhere); the directory is mounted here directly. const containerFallbackWorkdir = "/flow/workdir" +// containerPythonBin is the interpreter used inside a container. It is a bare +// name so it resolves on the image's PATH, which is what makes any python image +// work without further configuration. +const containerPythonBin = "python3" + // buildContainerSpec translates a host-side exec into a container.RunContainer // spec: resolving the runtime, building mounts, translating the working // directory and environment, and choosing between cmd and file execution. +// The returned cleanup func removes any temp script generated for the run and is +// never nil; callers should register it on the context so an abandoned run does +// not leak the file. func buildContainerSpec( e *executable.Executable, targetDir string, envMap map[string]string, -) (run.ContainerSpec, error) { +) (spec run.ContainerSpec, cleanup func(), err error) { + cleanup = func() {} c := e.Exec.Container rt, err := resolveRuntimeFn(string(c.Runtime)) if err != nil { - return run.ContainerSpec{}, err + return run.ContainerSpec{}, cleanup, err } if e.Exec.File != "" { switch strings.ToLower(filepath.Ext(e.Exec.File)) { case ".bat", ".cmd", ".ps1": - return run.ContainerSpec{}, errors.Errorf( + return run.ContainerSpec{}, cleanup, errors.Errorf( "container execution does not support %s files", filepath.Ext(e.Exec.File)) } } @@ -76,12 +85,12 @@ func buildContainerSpec( for _, v := range c.Volumes { m, err := parseVolume(string(v), wsRoot) if err != nil { - return run.ContainerSpec{}, err + return run.ContainerSpec{}, cleanup, err } mounts = append(mounts, m) } - spec := run.ContainerSpec{ + spec = run.ContainerSpec{ Runtime: rt, Image: c.Image, Name: containerName(e), @@ -93,7 +102,7 @@ func buildContainerSpec( Mounts: mounts, Network: c.Network, } - spec.Entrypoint, spec.OverrideEntry = c.ResolveEntrypoint() + spec.Entrypoint, spec.OverrideEntry = resolveContainerEntrypoint(e.Exec) spec.User = resolveUser(c) if c.EnvInherited() { @@ -101,6 +110,22 @@ func buildContainerSpec( } switch { + case e.Exec.Cmd != "" && e.Exec.ResolveInterpreter() == executable.InterpreterPython: + // Mount the script rather than setting spec.Cmd: buildRunArgs would turn a + // Cmd into `python3 -c `, putting the code in the process table and + // costing real traceback line numbers. + scriptHost, cleanupScript, err := run.WritePythonScript(e.Exec.Cmd) + if err != nil { + return run.ContainerSpec{}, nil, err + } + cleanup = cleanupScript + containerScript := path.Join(containerScriptMount, filepath.Base(scriptHost)) + spec.Mounts = append(spec.Mounts, run.Mount{ + HostPath: scriptHost, + ContainerPath: containerScript, + ReadOnly: true, + }) + spec.Script = containerScript case e.Exec.Cmd != "": spec.Cmd = e.Exec.Cmd case e.Exec.File != "": @@ -119,7 +144,7 @@ func buildContainerSpec( } } - return spec, nil + return spec, cleanup, nil } // containerPathUnder returns the container-side path for hostPath if it resolves @@ -182,12 +207,21 @@ func translateEnv(envMap map[string]string, mounts []run.Mount, wsRoot, mountPoi // else drop: the host path is meaningless inside the container. case "FLOW_CONFIG_PATH", "FLOW_CACHE_PATH": // Drop: these directories are not mounted. + case "VIRTUAL_ENV", "PYTHONHOME", "PYTHONPATH", run.PythonBinEnv: + // Drop: these point at host interpreters and site-packages. Leaking + // them in would make the container's python search paths that either + // do not exist or, worse, resolve to an unrelated mounted directory. + // A containerized run uses the image's own python. default: out[k] = v } } out["FLOW_IN_CONTAINER"] = "true" + if _, set := out["PYTHONUNBUFFERED"]; !set { + out["PYTHONUNBUFFERED"] = "1" + } + // Forward color preferences so containerized tools keep their coloring. for _, k := range []string{"TERM", "FORCE_COLOR", "CLICOLOR_FORCE", "NO_COLOR"} { if _, set := out[k]; !set { @@ -288,3 +322,23 @@ func randomSuffix() string { } return hex.EncodeToString(buf) } + +// resolveContainerEntrypoint picks the container entrypoint for an exec spec. +// +// It defers to an explicit container.entrypoint (including an empty one, which +// means "use the image's own ENTRYPOINT"), and otherwise defaults to the binary +// matching the interpreter: sh for a shell command, python3 for a python one. +// Resolution lives here rather than on ExecContainer so the types package stays +// unaware of how flow launches containers. +// +// Host interpreter discovery deliberately does not apply: a containerized run +// uses the image's python, never a venv from the host. +func resolveContainerEntrypoint(spec *executable.ExecExecutableType) (entrypoint string, override bool) { + if spec.Container != nil && spec.Container.Entrypoint != nil { + return spec.Container.ResolveEntrypoint() + } + if spec.InterpreterForFile() == executable.InterpreterPython { + return containerPythonBin, true + } + return spec.Container.ResolveEntrypoint() +} diff --git a/internal/runner/exec/exec.go b/internal/runner/exec/exec.go index 0a6a0670..48a2cc79 100644 --- a/internal/runner/exec/exec.go +++ b/internal/runner/exec/exec.go @@ -93,10 +93,16 @@ func (r *execRunner) Exec( } if execSpec.Container != nil { - spec, err := buildContainerSpec(e, targetDir, envMap) + spec, cleanupScript, err := buildContainerSpec(e, targetDir, envMap) if err != nil { return err } + // Registered like the container cleanup below: a run abandoned by the + // timeout goroutine must not leave the generated script behind. + ctx.AddCallback(func(*context.Context) error { + cleanupScript() + return nil + }) // Register cleanup before launch so an orphaned container is removed even // if the run is abandoned by the timeout goroutine in the runner. ctx.AddCallback(func(*context.Context) error { diff --git a/internal/runner/exec/exec_test.go b/internal/runner/exec/exec_test.go index 73019ef4..593e0685 100644 --- a/internal/runner/exec/exec_test.go +++ b/internal/runner/exec/exec_test.go @@ -257,6 +257,85 @@ var _ = Describe("Exec Runner", func() { DeferCleanup(restore) }) + newPythonContainerExec := func(c *executable.ExecContainer, dir executable.Directory) *executable.Executable { + interpreter := executable.InterpreterPython + e := &executable.Executable{Exec: &executable.ExecExecutableType{ + Cmd: "print('hi')", Dir: dir, Container: c, Interpreter: &interpreter, + }} + e.SetContext(ctx.Ctx.CurrentWorkspace.AssignedName(), wsPath, "", "") + e.SetDefaults() + return e + } + + It("mounts python code as a script instead of passing it as -c", func() { + // spec.Cmd would become `python3 -c `, putting the code in the + // process table and losing traceback line numbers. + e := newPythonContainerExec(&executable.ExecContainer{Image: "python:3.13"}, executable.Directory(wsPath)) + Expect(execRnr.Exec(ctx.Ctx, e, mockEngine, map[string]string{}, nil)).To(Succeed()) + Expect(containerSpecs).To(HaveLen(1)) + Expect(containerSpecs[0].Cmd).To(BeEmpty()) + Expect(containerSpecs[0].Script).To(HaveSuffix(".py")) + + var scriptMount *run.Mount + for i := range containerSpecs[0].Mounts { + if containerSpecs[0].Mounts[i].ContainerPath == containerSpecs[0].Script { + scriptMount = &containerSpecs[0].Mounts[i] + } + } + Expect(scriptMount).ToNot(BeNil(), "the generated script should be bind-mounted") + Expect(scriptMount.ReadOnly).To(BeTrue()) + }) + + It("defaults the entrypoint to python3 for a python interpreter", func() { + e := newPythonContainerExec(&executable.ExecContainer{Image: "python:3.13"}, executable.Directory(wsPath)) + Expect(execRnr.Exec(ctx.Ctx, e, mockEngine, map[string]string{}, nil)).To(Succeed()) + Expect(containerSpecs[0].Entrypoint).To(Equal("python3")) + Expect(containerSpecs[0].OverrideEntry).To(BeTrue()) + }) + + It("lets an explicit entrypoint override the python default", func() { + e := newPythonContainerExec(&executable.ExecContainer{ + Image: "python:3.13", Entrypoint: strPtrFor("python3.13"), + }, executable.Directory(wsPath)) + Expect(execRnr.Exec(ctx.Ctx, e, mockEngine, map[string]string{}, nil)).To(Succeed()) + Expect(containerSpecs[0].Entrypoint).To(Equal("python3.13")) + }) + + It("uses the image ENTRYPOINT when entrypoint is explicitly empty", func() { + e := newPythonContainerExec(&executable.ExecContainer{ + Image: "python:3.13", Entrypoint: strPtrFor(""), + }, executable.Directory(wsPath)) + Expect(execRnr.Exec(ctx.Ctx, e, mockEngine, map[string]string{}, nil)).To(Succeed()) + Expect(containerSpecs[0].OverrideEntry).To(BeFalse()) + }) + + It("keeps a shell command on the sh entrypoint", func() { + e := newContainerExec(&executable.ExecContainer{Image: "alpine:3"}, executable.Directory(wsPath)) + Expect(execRnr.Exec(ctx.Ctx, e, mockEngine, map[string]string{}, nil)).To(Succeed()) + Expect(containerSpecs[0].Entrypoint).To(Equal("sh")) + }) + + It("drops host python paths from the container environment", func() { + // A host venv path names nothing inside the container, and could even + // resolve to an unrelated mounted directory. + e := newPythonContainerExec(&executable.ExecContainer{Image: "python:3.13"}, executable.Directory(wsPath)) + Expect(execRnr.Exec(ctx.Ctx, e, mockEngine, map[string]string{ + "VIRTUAL_ENV": "/host/.venv", + "PYTHONPATH": "/host/site-packages", + "PYTHONHOME": "/host/python", + "FLOW_PYTHON_BIN": "/host/.venv/bin/python", + "KEEP_ME": "yes", + }, nil)).To(Succeed()) + + env := containerSpecs[0].Env + Expect(env).ToNot(HaveKey("VIRTUAL_ENV")) + Expect(env).ToNot(HaveKey("PYTHONPATH")) + Expect(env).ToNot(HaveKey("PYTHONHOME")) + Expect(env).ToNot(HaveKey("FLOW_PYTHON_BIN")) + Expect(env).To(HaveKeyWithValue("KEEP_ME", "yes")) + Expect(env).To(HaveKeyWithValue("PYTHONUNBUFFERED", "1")) + }) + It("routes to the container backend instead of runCmd", func() { e := newContainerExec(&executable.ExecContainer{Image: "alpine:3"}, executable.Directory(wsPath)) Expect(execRnr.Exec(ctx.Ctx, e, mockEngine, map[string]string{}, nil)).To(Succeed()) @@ -397,3 +476,6 @@ var _ = Describe("Exec Runner", func() { }) }) }) + +// strPtrFor is a local helper for building optional container fields in tests. +func strPtrFor(s string) *string { return &s } diff --git a/tests/container_exec_e2e_test.go b/tests/container_exec_e2e_test.go index da5cce87..acd28681 100644 --- a/tests/container_exec_e2e_test.go +++ b/tests/container_exec_e2e_test.go @@ -61,6 +61,19 @@ var _ = Describe("container exec e2e", func() { } }) + It("runs python inside the container using the image's interpreter", func() { + runner := utils.NewE2ECommandRunner() + stdOut := ctx.StdOut() + Expect(runner.Run(ctx.Context, "exec", "examples:with-python-container", + "--log-level", "debug")).To(Succeed()) + out, _ := readFileContent(stdOut) + Expect(out).To(ContainSubstring("hello from with-python-container")) + Expect(out).To(ContainSubstring("in-container=true")) + // A real interpreter ran, and it was the image's - the host venv, if + // any, is deliberately not carried in. + Expect(out).To(ContainSubstring("py-major=3")) + }) + It("runs the command inside the container", func() { runner := utils.NewE2ECommandRunner() stdOut := ctx.StdOut() diff --git a/tests/utils/builder/exec.go b/tests/utils/builder/exec.go index 99f985a9..8adbe2de 100644 --- a/tests/utils/builder/exec.go +++ b/tests/utils/builder/exec.go @@ -280,6 +280,32 @@ func ExecWithPython(opts ...Option) *executable.Executable { return e } +func ExecWithPythonContainer(opts ...Option) *executable.Executable { + name := "with-python-container" + interpreter := executable.InterpreterPython + e := &executable.Executable{ + Verb: "run", + Name: name, + Visibility: privateExecVisibility(), + Exec: &executable.ExecExecutableType{ + Interpreter: &interpreter, + Cmd: fmt.Sprintf( + "import os, sys\nprint('hello from %s')\nprint('in-container=' + os.environ.get('FLOW_IN_CONTAINER', ''))\n"+ + "print('py-major=%%d' %% sys.version_info[0])\n", + name, + ), + Container: &executable.ExecContainer{ + Image: "python:3.13-alpine", + }, + }, + } + if len(opts) > 0 { + vals := NewOptionValues(opts...) + e.SetContext(vals.WorkspaceName, vals.WorkspacePath, vals.NamespaceName, vals.FlowFilePath) + } + return e +} + func ExecWithWorkspaceEnv(opts ...Option) *executable.Executable { name := "with-workspace-env" e := &executable.Executable{ diff --git a/tests/utils/builder/flowfile.go b/tests/utils/builder/flowfile.go index 12519d42..49a2ec41 100644 --- a/tests/utils/builder/flowfile.go +++ b/tests/utils/builder/flowfile.go @@ -37,6 +37,7 @@ func ExamplesExecFlowFile(opts ...Option) *executable.FlowFile { ExecWithWorkspaceEnv(opts...), ExecWithContainer(opts...), ExecWithPython(opts...), + ExecWithPythonContainer(opts...), }, } if len(opts) > 0 {