feat: make pseudo-TTY allocation opt-in (#961)

Fixes #956.

Pseudo-TTY allocation is now an explicit, runner-wide opt-in via `runner.allocate_pty`, applied to both host and docker backends. Default is off, matching GitHub `actions/runner`.

```yaml
runner:
  allocate_pty: false  # default
```

**Before:** the host backend hardcoded `if true /* allocate Terminal */` and the docker backend used `term.IsTerminal(os.Stdout.Fd())`. As a result, `docker build` (and other TTY-aware tools) saw a TTY and emitted cursor-control redraw frames that flooded captured logs with thousands of duplicate-looking progress lines — only on host-mode runners in production, and on docker-mode runners when the daemon happened to be launched from a shell rather than a service.

**After:** both backends consult `Config.AllocatePTY`. The `term.IsTerminal` heuristic is gone, so behavior no longer depends on whether the daemon has a controlling terminal.

**Reproduction:** running `docker build` through `HostEnvironment.Exec` with output captured to a buffer:

| | Before (`if true`) | After (`AllocatePTY=false`) |
|---|---:|---:|
| bytes captured | 18,167 | 1,048 |
| ANSI CSI sequences | 556 | 0 |
| cursor-up `\e[1A` | 181 | 0 |

**Side fix:** `ptyWriter.AutoStop` is now `atomic.Bool`. The field is written from the exec goroutine after `cmd.Wait()` and read from the `copyPtyOutput` goroutine via `ptyWriter.Write`; existing tests never tripped the race detector because their commands produced no output before exit. The new host-mode test does.

---
This PR was written with the help of Claude Opus 4.7

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Nicolas <bircni@icloud.com>
Reviewed-on: https://gitea.com/gitea/runner/pulls/961
Reviewed-by: Nicolas <bircni@icloud.com>
Co-authored-by: silverwind <2021+silverwind@noreply.gitea.com>
Co-committed-by: silverwind <2021+silverwind@noreply.gitea.com>
This commit is contained in:
silverwind
2026-05-15 18:11:39 +00:00
committed by Lunny Xiao
parent 880e9755d9
commit 3c5f03ff8f
12 changed files with 120 additions and 20 deletions

View File

@@ -47,6 +47,7 @@ type NewContainerInput struct {
// Gitea specific // Gitea specific
AutoRemove bool AutoRemove bool
ValidVolumes []string ValidVolumes []string
AllocatePTY bool // allocate a pseudo-TTY for the container's exec processes
} }
// FileEntry is a file to copy to a container // FileEntry is a file to copy to a container

View File

@@ -41,7 +41,6 @@ import (
"github.com/moby/moby/client" "github.com/moby/moby/client"
specs "github.com/opencontainers/image-spec/specs-go/v1" specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/spf13/pflag" "github.com/spf13/pflag"
"golang.org/x/term"
) )
// NewContainer creates a reference to a container // NewContainer creates a reference to a container
@@ -450,7 +449,6 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor {
return nil return nil
} }
logger := common.Logger(ctx) logger := common.Logger(ctx)
isTerminal := term.IsTerminal(int(os.Stdout.Fd()))
input := cr.input input := cr.input
exposedPorts, err := convertPortSet(input.ExposedPorts) exposedPorts, err := convertPortSet(input.ExposedPorts)
if err != nil { if err != nil {
@@ -466,7 +464,7 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor {
WorkingDir: input.WorkingDir, WorkingDir: input.WorkingDir,
Env: input.Env, Env: input.Env,
ExposedPorts: exposedPorts, ExposedPorts: exposedPorts,
Tty: isTerminal, Tty: input.AllocatePTY,
} }
// For Gitea, reduce log noise // For Gitea, reduce log noise
// logger.Debugf("Common container.Config ==> %+v", config) // logger.Debugf("Common container.Config ==> %+v", config)
@@ -604,7 +602,7 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user, wo
} }
logger.Debugf("Exec command '%s'", cmd) logger.Debugf("Exec command '%s'", cmd)
isTerminal := term.IsTerminal(int(os.Stdout.Fd())) isTerminal := cr.input.AllocatePTY
envList := make([]string, 0) envList := make([]string, 0)
for k, v := range env { for k, v := range env {
envList = append(envList, fmt.Sprintf("%s=%s", k, v)) envList = append(envList, fmt.Sprintf("%s=%s", k, v))
@@ -899,7 +897,7 @@ func (cr *containerReference) attach() common.Executor {
if err != nil { if err != nil {
return fmt.Errorf("failed to attach to container: %w", err) return fmt.Errorf("failed to attach to container: %w", err)
} }
isTerminal := term.IsTerminal(int(os.Stdout.Fd())) isTerminal := cr.input.AllocatePTY
var outWriter io.Writer var outWriter io.Writer
outWriter = cr.input.Stdout outWriter = cr.input.Stdout

View File

@@ -19,6 +19,7 @@ import (
"strconv" "strconv"
"strings" "strings"
"sync" "sync"
"sync/atomic"
"time" "time"
"gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common"
@@ -42,6 +43,7 @@ type HostEnvironment struct {
ActPath string ActPath string
CleanUp func() CleanUp func()
StdOut io.Writer StdOut io.Writer
AllocatePTY bool // allocate a pseudo-TTY for each step's process
mu sync.Mutex mu sync.Mutex
runningPIDs map[int]struct{} runningPIDs map[int]struct{}
@@ -200,12 +202,12 @@ func (e *HostEnvironment) Start(_ bool) common.Executor {
type ptyWriter struct { type ptyWriter struct {
Out io.Writer Out io.Writer
AutoStop bool AutoStop atomic.Bool
dirtyLine bool dirtyLine bool
} }
func (w *ptyWriter) Write(buf []byte) (int, error) { func (w *ptyWriter) Write(buf []byte) (int, error) {
if w.AutoStop && len(buf) > 0 && buf[len(buf)-1] == 4 { if w.AutoStop.Load() && len(buf) > 0 && buf[len(buf)-1] == 4 {
n, err := w.Out.Write(buf[:len(buf)-1]) n, err := w.Out.Write(buf[:len(buf)-1])
if err != nil { if err != nil {
return n, err return n, err
@@ -335,21 +337,20 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st
tty.Close() tty.Close()
} }
}() }()
if true /* allocate Terminal */ { if e.AllocatePTY {
var err error var err error
ppty, tty, err = setupPty(cmd, cmdline) ppty, tty, err = setupPty(cmd, cmdline)
if err != nil { if err != nil {
common.Logger(ctx).Debugf("Failed to setup Pty %v\n", err.Error()) common.Logger(ctx).Debugf("Failed to setup Pty %v\n", err.Error())
} }
} }
writer := &ptyWriter{Out: e.StdOut} var writer *ptyWriter
logctx, finishLog := context.WithCancel(context.Background()) var logctx context.Context
if ppty != nil { if ppty != nil {
writer = &ptyWriter{Out: e.StdOut}
var finishLog context.CancelFunc
logctx, finishLog = context.WithCancel(context.Background())
go copyPtyOutput(writer, ppty, finishLog) go copyPtyOutput(writer, ppty, finishLog)
} else {
finishLog()
}
if ppty != nil {
go writeKeepAlive(ppty) go writeKeepAlive(ppty)
} }
// Split Start/Wait so the PID can be registered before the process can exit; // Split Start/Wait so the PID can be registered before the process can exit;
@@ -379,14 +380,11 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st
return err return err
} }
if tty != nil { if tty != nil {
writer.AutoStop = true writer.AutoStop.Store(true)
if _, err := tty.WriteString("\x04"); err != nil { if _, err := tty.WriteString("\x04"); err != nil {
common.Logger(ctx).Debug("Failed to write EOT") common.Logger(ctx).Debug("Failed to write EOT")
} }
}
<-logctx.Done() <-logctx.Done()
if ppty != nil {
ppty.Close() ppty.Close()
ppty = nil ppty = nil
} }

View File

@@ -6,12 +6,14 @@ package container
import ( import (
"archive/tar" "archive/tar"
"bytes"
"context" "context"
"io" "io"
"os" "os"
"path" "path"
"path/filepath" "path/filepath"
"runtime" "runtime"
"strings"
"testing" "testing"
"gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common"
@@ -100,6 +102,45 @@ func TestHostEnvironmentExecExitCode(t *testing.T) {
assert.Equal(t, "Process completed with exit code 3.", err.Error()) assert.Equal(t, "Process completed with exit code 3.", err.Error())
} }
func TestHostEnvironmentAllocatePTY(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("uses POSIX shell")
}
for _, tc := range []struct {
name string
allocPTY bool
expect string
}{
{name: "off", allocPTY: false, expect: "NOTTY"},
{name: "on", allocPTY: true, expect: "TTY"},
} {
t.Run(tc.name, func(t *testing.T) {
dir := t.TempDir()
buf := &bytes.Buffer{}
e := &HostEnvironment{
Path: filepath.Join(dir, "path"),
TmpDir: filepath.Join(dir, "tmp"),
ToolCache: filepath.Join(dir, "tool_cache"),
ActPath: filepath.Join(dir, "act_path"),
StdOut: buf,
Workdir: filepath.Join(dir, "path"),
AllocatePTY: tc.allocPTY,
}
for _, p := range []string{e.Path, e.TmpDir, e.ToolCache, e.ActPath} {
require.NoError(t, os.MkdirAll(p, 0o700))
}
err := e.Exec(
[]string{"sh", "-c", "[ -t 1 ] && printf TTY || printf NOTTY"},
map[string]string{"PATH": os.Getenv("PATH")}, "", "",
)(context.Background())
require.NoError(t, err)
got := strings.TrimSpace(strings.ReplaceAll(buf.String(), "\r", ""))
assert.Equal(t, tc.expect, got)
})
}
}
func TestHostEnvironmentRemoveCleansWorkdir(t *testing.T) { func TestHostEnvironmentRemoveCleansWorkdir(t *testing.T) {
logger := logrus.New() logger := logrus.New()
ctx := common.WithLogger(context.Background(), logrus.NewEntry(logger)) ctx := common.WithLogger(context.Background(), logrus.NewEntry(logger))

View File

@@ -456,6 +456,7 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo
Options: rc.Config.ContainerOptions, Options: rc.Config.ContainerOptions,
AutoRemove: rc.Config.AutoRemove, AutoRemove: rc.Config.AutoRemove,
ValidVolumes: rc.Config.ValidVolumes, ValidVolumes: rc.Config.ValidVolumes,
AllocatePTY: rc.Config.AllocatePTY,
}) })
return stepContainer return stepContainer
} }

View File

@@ -230,6 +230,7 @@ func (rc *RunContext) startHostEnvironment() common.Executor {
os.RemoveAll(miscpath) os.RemoveAll(miscpath)
}, },
StdOut: logWriter, StdOut: logWriter,
AllocatePTY: rc.Config.AllocatePTY,
} }
rc.cleanUpJobContainer = rc.JobContainer.Remove() rc.cleanUpJobContainer = rc.JobContainer.Remove()
for k, v := range rc.JobContainer.GetRunnerContext(ctx) { for k, v := range rc.JobContainer.GetRunnerContext(ctx) {
@@ -371,6 +372,7 @@ func (rc *RunContext) startJobContainer() common.Executor {
NetworkAliases: []string{serviceID}, NetworkAliases: []string{serviceID},
ExposedPorts: exposedPorts, ExposedPorts: exposedPorts,
PortBindings: portBindings, PortBindings: portBindings,
AllocatePTY: rc.Config.AllocatePTY,
}) })
rc.ServiceContainers = append(rc.ServiceContainers, c) rc.ServiceContainers = append(rc.ServiceContainers, c)
} }
@@ -431,6 +433,7 @@ func (rc *RunContext) startJobContainer() common.Executor {
Options: rc.options(ctx), Options: rc.options(ctx),
AutoRemove: rc.Config.AutoRemove, AutoRemove: rc.Config.AutoRemove,
ValidVolumes: rc.Config.ValidVolumes, ValidVolumes: rc.Config.ValidVolumes,
AllocatePTY: rc.Config.AllocatePTY,
}) })
if rc.JobContainer == nil { if rc.JobContainer == nil {
return errors.New("Failed to create job container") return errors.New("Failed to create job container")

View File

@@ -79,6 +79,7 @@ type Config struct {
ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers
InsecureSkipTLS bool // whether to skip verifying TLS certificate of the Gitea instance InsecureSkipTLS bool // whether to skip verifying TLS certificate of the Gitea instance
MaxParallel int // max parallel jobs to run across all workflows (0 = no limit, uses CPU count) MaxParallel int // max parallel jobs to run across all workflows (0 = no limit, uses CPU count)
AllocatePTY bool // allocate a pseudo-TTY for each step's process
} }
// GetToken: Adapt to Gitea // GetToken: Adapt to Gitea

View File

@@ -139,6 +139,7 @@ func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd, e
Platform: rc.Config.ContainerArchitecture, Platform: rc.Config.ContainerArchitecture,
AutoRemove: rc.Config.AutoRemove, AutoRemove: rc.Config.AutoRemove,
ValidVolumes: rc.Config.ValidVolumes, ValidVolumes: rc.Config.ValidVolumes,
AllocatePTY: rc.Config.AllocatePTY,
}) })
return stepContainer return stepContainer
} }

View File

@@ -109,6 +109,55 @@ func TestStepDockerMain(t *testing.T) {
cm.AssertExpectations(t) cm.AssertExpectations(t)
} }
func TestStepDockerNewStepContainerAllocatePTY(t *testing.T) {
for _, tc := range []struct {
name string
allocPTY bool
}{
{name: "off", allocPTY: false},
{name: "on", allocPTY: true},
} {
t.Run(tc.name, func(t *testing.T) {
cm := &containerMock{}
var captured *container.NewContainerInput
origContainerNewContainer := ContainerNewContainer
ContainerNewContainer = func(input *container.NewContainerInput) container.ExecutionsEnvironment {
captured = input
return cm
}
defer func() {
ContainerNewContainer = origContainerNewContainer
}()
ctx := context.Background()
sd := &stepDocker{
RunContext: &RunContext{
StepResults: map[string]*model.StepResult{},
Config: &Config{
AllocatePTY: tc.allocPTY,
PlatformPicker: func(_ []string) string {
return "node:14"
},
},
Run: &model.Run{
JobID: "1",
Workflow: &model.Workflow{
Jobs: map[string]*model.Job{"1": {}},
},
},
JobContainer: cm,
},
Step: &model.Step{ID: "1", Uses: "docker://node:14"},
}
sd.RunContext.ExprEval = sd.RunContext.NewExpressionEvaluator(ctx)
_ = sd.newStepContainer(ctx, "node:14", []string{"echo", "hi"}, nil)
assert.Equal(t, tc.allocPTY, captured.AllocatePTY)
})
}
}
func TestStepDockerPrePost(t *testing.T) { func TestStepDockerPrePost(t *testing.T) {
ctx := context.Background() ctx := context.Background()
sd := &stepDocker{} sd := &stepDocker{}

View File

@@ -347,6 +347,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
Workdir: workdir, Workdir: workdir,
BindWorkdir: r.cfg.Container.BindWorkdir, BindWorkdir: r.cfg.Container.BindWorkdir,
ActionCacheDir: filepath.FromSlash(r.cfg.Host.WorkdirParent), ActionCacheDir: filepath.FromSlash(r.cfg.Host.WorkdirParent),
AllocatePTY: r.cfg.Runner.AllocatePTY,
ReuseContainers: false, ReuseContainers: false,
ForcePull: r.cfg.Container.ForcePull, ForcePull: r.cfg.Container.ForcePull,

View File

@@ -74,6 +74,11 @@ runner:
- "ubuntu-latest:docker://docker.gitea.com/runner-images:ubuntu-latest" - "ubuntu-latest:docker://docker.gitea.com/runner-images:ubuntu-latest"
- "ubuntu-24.04:docker://docker.gitea.com/runner-images:ubuntu-24.04" - "ubuntu-24.04:docker://docker.gitea.com/runner-images:ubuntu-24.04"
- "ubuntu-22.04:docker://docker.gitea.com/runner-images:ubuntu-22.04" - "ubuntu-22.04:docker://docker.gitea.com/runner-images:ubuntu-22.04"
# Allocate a pseudo-TTY for each step's process. Applies to both host and docker backends.
# Default false matches GitHub actions/runner. Enable only for jobs that need an interactive
# terminal; tools like `docker build` emit redrawing progress frames into the captured log
# when a TTY is present.
allocate_pty: false
cache: cache:
# Enable cache server to use actions/cache. # Enable cache server to use actions/cache.

View File

@@ -41,6 +41,7 @@ type Runner struct {
StateReportInterval time.Duration `yaml:"state_report_interval"` // StateReportInterval specifies the interval for state reporting. StateReportInterval time.Duration `yaml:"state_report_interval"` // StateReportInterval specifies the interval for state reporting.
Labels []string `yaml:"labels"` // Labels specify the labels of the runner. Labels are declared on each startup Labels []string `yaml:"labels"` // Labels specify the labels of the runner. Labels are declared on each startup
GithubMirror string `yaml:"github_mirror"` // GithubMirror defines what mirrors should be used when using github GithubMirror string `yaml:"github_mirror"` // GithubMirror defines what mirrors should be used when using github
AllocatePTY bool `yaml:"allocate_pty"` // AllocatePTY allocates a pseudo-TTY for each step's process. Default is false, matching GitHub's actions/runner. Enable only for jobs that need an interactive terminal; tools like docker build emit redrawing progress frames into the captured log when a TTY is present. Applies to both host and docker backends.
} }
// Cache represents the configuration for caching. // Cache represents the configuration for caching.