mirror of
https://github.com/coder/coder.git
synced 2025-07-15 22:20:27 +00:00
feat: integrate new agentexec pkg (#15609)
- Integrates the `agentexec` pkg into the agent and removes the legacy system of iterating over the process tree. It adds some linting rules to hopefully catch future improper uses of `exec.Command` in the package.
This commit is contained in:
264
agent/agent.go
264
agent/agent.go
@ -12,8 +12,6 @@ import (
|
||||
"os"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"runtime/debug"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
@ -35,7 +33,6 @@ import (
|
||||
"tailscale.com/util/clientmetric"
|
||||
|
||||
"cdr.dev/slog"
|
||||
"github.com/coder/coder/v2/agent/agentproc"
|
||||
"github.com/coder/coder/v2/agent/agentscripts"
|
||||
"github.com/coder/coder/v2/agent/agentssh"
|
||||
"github.com/coder/coder/v2/agent/proto"
|
||||
@ -82,11 +79,6 @@ type Options struct {
|
||||
PrometheusRegistry *prometheus.Registry
|
||||
ReportMetadataInterval time.Duration
|
||||
ServiceBannerRefreshInterval time.Duration
|
||||
Syscaller agentproc.Syscaller
|
||||
// ModifiedProcesses is used for testing process priority management.
|
||||
ModifiedProcesses chan []*agentproc.Process
|
||||
// ProcessManagementTick is used for testing process priority management.
|
||||
ProcessManagementTick <-chan time.Time
|
||||
BlockFileTransfer bool
|
||||
}
|
||||
|
||||
@ -147,10 +139,6 @@ func New(options Options) Agent {
|
||||
prometheusRegistry = prometheus.NewRegistry()
|
||||
}
|
||||
|
||||
if options.Syscaller == nil {
|
||||
options.Syscaller = agentproc.NewSyscaller()
|
||||
}
|
||||
|
||||
hardCtx, hardCancel := context.WithCancel(context.Background())
|
||||
gracefulCtx, gracefulCancel := context.WithCancel(hardCtx)
|
||||
a := &agent{
|
||||
@ -178,9 +166,6 @@ func New(options Options) Agent {
|
||||
announcementBannersRefreshInterval: options.ServiceBannerRefreshInterval,
|
||||
sshMaxTimeout: options.SSHMaxTimeout,
|
||||
subsystems: options.Subsystems,
|
||||
syscaller: options.Syscaller,
|
||||
modifiedProcs: options.ModifiedProcesses,
|
||||
processManagementTick: options.ProcessManagementTick,
|
||||
logSender: agentsdk.NewLogSender(options.Logger),
|
||||
blockFileTransfer: options.BlockFileTransfer,
|
||||
|
||||
@ -254,12 +239,6 @@ type agent struct {
|
||||
// metrics are prometheus registered metrics that will be collected and
|
||||
// labeled in Coder with the agent + workspace.
|
||||
metrics *agentMetrics
|
||||
syscaller agentproc.Syscaller
|
||||
|
||||
// modifiedProcs is used for testing process priority management.
|
||||
modifiedProcs chan []*agentproc.Process
|
||||
// processManagementTick is used for testing process priority management.
|
||||
processManagementTick <-chan time.Time
|
||||
}
|
||||
|
||||
func (a *agent) TailnetConn() *tailnet.Conn {
|
||||
@ -308,8 +287,6 @@ func (a *agent) init() {
|
||||
// may be happening, but regardless after the intermittent
|
||||
// failure, you'll want the agent to reconnect.
|
||||
func (a *agent) runLoop() {
|
||||
go a.manageProcessPriorityUntilGracefulShutdown()
|
||||
|
||||
// need to keep retrying up to the hardCtx so that we can send graceful shutdown-related
|
||||
// messages.
|
||||
ctx := a.hardCtx
|
||||
@ -1443,162 +1420,6 @@ func (a *agent) Collect(ctx context.Context, networkStats map[netlogtype.Connect
|
||||
return stats
|
||||
}
|
||||
|
||||
var prioritizedProcs = []string{"coder agent"}
|
||||
|
||||
func (a *agent) manageProcessPriorityUntilGracefulShutdown() {
|
||||
// process priority can stop as soon as we are gracefully shutting down
|
||||
ctx := a.gracefulCtx
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
a.logger.Critical(ctx, "recovered from panic",
|
||||
slog.F("panic", r),
|
||||
slog.F("stack", string(debug.Stack())),
|
||||
)
|
||||
}
|
||||
}()
|
||||
|
||||
if val := a.environmentVariables[EnvProcPrioMgmt]; val == "" || runtime.GOOS != "linux" {
|
||||
a.logger.Debug(ctx, "process priority not enabled, agent will not manage process niceness/oom_score_adj ",
|
||||
slog.F("env_var", EnvProcPrioMgmt),
|
||||
slog.F("value", val),
|
||||
slog.F("goos", runtime.GOOS),
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
if a.processManagementTick == nil {
|
||||
ticker := time.NewTicker(time.Second)
|
||||
defer ticker.Stop()
|
||||
a.processManagementTick = ticker.C
|
||||
}
|
||||
|
||||
oomScore := unsetOOMScore
|
||||
if scoreStr, ok := a.environmentVariables[EnvProcOOMScore]; ok {
|
||||
score, err := strconv.Atoi(strings.TrimSpace(scoreStr))
|
||||
if err == nil && score >= -1000 && score <= 1000 {
|
||||
oomScore = score
|
||||
} else {
|
||||
a.logger.Error(ctx, "invalid oom score",
|
||||
slog.F("min_value", -1000),
|
||||
slog.F("max_value", 1000),
|
||||
slog.F("value", scoreStr),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
debouncer := &logDebouncer{
|
||||
logger: a.logger,
|
||||
messages: map[string]time.Time{},
|
||||
interval: time.Minute,
|
||||
}
|
||||
|
||||
for {
|
||||
procs, err := a.manageProcessPriority(ctx, debouncer, oomScore)
|
||||
// Avoid spamming the logs too often.
|
||||
if err != nil {
|
||||
debouncer.Error(ctx, "manage process priority",
|
||||
slog.Error(err),
|
||||
)
|
||||
}
|
||||
if a.modifiedProcs != nil {
|
||||
a.modifiedProcs <- procs
|
||||
}
|
||||
|
||||
select {
|
||||
case <-a.processManagementTick:
|
||||
case <-ctx.Done():
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// unsetOOMScore is set to an invalid OOM score to imply an unset value.
|
||||
const unsetOOMScore = 1001
|
||||
|
||||
func (a *agent) manageProcessPriority(ctx context.Context, debouncer *logDebouncer, oomScore int) ([]*agentproc.Process, error) {
|
||||
const (
|
||||
niceness = 10
|
||||
)
|
||||
|
||||
// We fetch the agent score each time because it's possible someone updates the
|
||||
// value after it is started.
|
||||
agentScore, err := a.getAgentOOMScore()
|
||||
if err != nil {
|
||||
agentScore = unsetOOMScore
|
||||
}
|
||||
if oomScore == unsetOOMScore && agentScore != unsetOOMScore {
|
||||
// If the child score has not been explicitly specified we should
|
||||
// set it to a score relative to the agent score.
|
||||
oomScore = childOOMScore(agentScore)
|
||||
}
|
||||
|
||||
procs, err := agentproc.List(a.filesystem, a.syscaller)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("list: %w", err)
|
||||
}
|
||||
|
||||
modProcs := []*agentproc.Process{}
|
||||
|
||||
for _, proc := range procs {
|
||||
containsFn := func(e string) bool {
|
||||
contains := strings.Contains(proc.Cmd(), e)
|
||||
return contains
|
||||
}
|
||||
|
||||
// If the process is prioritized we should adjust
|
||||
// it's oom_score_adj and avoid lowering its niceness.
|
||||
if slices.ContainsFunc(prioritizedProcs, containsFn) {
|
||||
continue
|
||||
}
|
||||
|
||||
score, niceErr := proc.Niceness(a.syscaller)
|
||||
if niceErr != nil && !isBenignProcessErr(niceErr) {
|
||||
debouncer.Warn(ctx, "unable to get proc niceness",
|
||||
slog.F("cmd", proc.Cmd()),
|
||||
slog.F("pid", proc.PID),
|
||||
slog.Error(niceErr),
|
||||
)
|
||||
}
|
||||
|
||||
// We only want processes that don't have a nice value set
|
||||
// so we don't override user nice values.
|
||||
// Getpriority actually returns priority for the nice value
|
||||
// which is niceness + 20, so here 20 = a niceness of 0 (aka unset).
|
||||
if score != 20 {
|
||||
// We don't log here since it can get spammy
|
||||
continue
|
||||
}
|
||||
|
||||
if niceErr == nil {
|
||||
err := proc.SetNiceness(a.syscaller, niceness)
|
||||
if err != nil && !isBenignProcessErr(err) {
|
||||
debouncer.Warn(ctx, "unable to set proc niceness",
|
||||
slog.F("cmd", proc.Cmd()),
|
||||
slog.F("pid", proc.PID),
|
||||
slog.F("niceness", niceness),
|
||||
slog.Error(err),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// If the oom score is valid and it's not already set and isn't a custom value set by another process then it's ok to update it.
|
||||
if oomScore != unsetOOMScore && oomScore != proc.OOMScoreAdj && !isCustomOOMScore(agentScore, proc) {
|
||||
oomScoreStr := strconv.Itoa(oomScore)
|
||||
err := afero.WriteFile(a.filesystem, fmt.Sprintf("/proc/%d/oom_score_adj", proc.PID), []byte(oomScoreStr), 0o644)
|
||||
if err != nil && !isBenignProcessErr(err) {
|
||||
debouncer.Warn(ctx, "unable to set oom_score_adj",
|
||||
slog.F("cmd", proc.Cmd()),
|
||||
slog.F("pid", proc.PID),
|
||||
slog.F("score", oomScoreStr),
|
||||
slog.Error(err),
|
||||
)
|
||||
}
|
||||
}
|
||||
modProcs = append(modProcs, proc)
|
||||
}
|
||||
return modProcs, nil
|
||||
}
|
||||
|
||||
// isClosed returns whether the API is closed or not.
|
||||
func (a *agent) isClosed() bool {
|
||||
return a.hardCtx.Err() != nil
|
||||
@ -1992,88 +1813,3 @@ func PrometheusMetricsHandler(prometheusRegistry *prometheus.Registry, logger sl
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// childOOMScore returns the oom_score_adj for a child process. It is based
|
||||
// on the oom_score_adj of the agent process.
|
||||
func childOOMScore(agentScore int) int {
|
||||
// If the agent has a negative oom_score_adj, we set the child to 0
|
||||
// so it's treated like every other process.
|
||||
if agentScore < 0 {
|
||||
return 0
|
||||
}
|
||||
|
||||
// If the agent is already almost at the maximum then set it to the max.
|
||||
if agentScore >= 998 {
|
||||
return 1000
|
||||
}
|
||||
|
||||
// If the agent oom_score_adj is >=0, we set the child to slightly
|
||||
// less than the maximum. If users want a different score they set it
|
||||
// directly.
|
||||
return 998
|
||||
}
|
||||
|
||||
func (a *agent) getAgentOOMScore() (int, error) {
|
||||
scoreStr, err := afero.ReadFile(a.filesystem, "/proc/self/oom_score_adj")
|
||||
if err != nil {
|
||||
return 0, xerrors.Errorf("read file: %w", err)
|
||||
}
|
||||
|
||||
score, err := strconv.Atoi(strings.TrimSpace(string(scoreStr)))
|
||||
if err != nil {
|
||||
return 0, xerrors.Errorf("parse int: %w", err)
|
||||
}
|
||||
|
||||
return score, nil
|
||||
}
|
||||
|
||||
// isCustomOOMScore checks to see if the oom_score_adj is not a value that would
|
||||
// originate from an agent-spawned process.
|
||||
func isCustomOOMScore(agentScore int, process *agentproc.Process) bool {
|
||||
score := process.OOMScoreAdj
|
||||
return agentScore != score && score != 1000 && score != 0 && score != 998
|
||||
}
|
||||
|
||||
// logDebouncer skips writing a log for a particular message if
|
||||
// it's been emitted within the given interval duration.
|
||||
// It's a shoddy implementation used in one spot that should be replaced at
|
||||
// some point.
|
||||
type logDebouncer struct {
|
||||
logger slog.Logger
|
||||
messages map[string]time.Time
|
||||
interval time.Duration
|
||||
}
|
||||
|
||||
func (l *logDebouncer) Warn(ctx context.Context, msg string, fields ...any) {
|
||||
l.log(ctx, slog.LevelWarn, msg, fields...)
|
||||
}
|
||||
|
||||
func (l *logDebouncer) Error(ctx context.Context, msg string, fields ...any) {
|
||||
l.log(ctx, slog.LevelError, msg, fields...)
|
||||
}
|
||||
|
||||
func (l *logDebouncer) log(ctx context.Context, level slog.Level, msg string, fields ...any) {
|
||||
// This (bad) implementation assumes you wouldn't reuse the same msg
|
||||
// for different levels.
|
||||
if last, ok := l.messages[msg]; ok && time.Since(last) < l.interval {
|
||||
return
|
||||
}
|
||||
switch level {
|
||||
case slog.LevelWarn:
|
||||
l.logger.Warn(ctx, msg, fields...)
|
||||
case slog.LevelError:
|
||||
l.logger.Error(ctx, msg, fields...)
|
||||
}
|
||||
l.messages[msg] = time.Now()
|
||||
}
|
||||
|
||||
func isBenignProcessErr(err error) bool {
|
||||
return err != nil &&
|
||||
(xerrors.Is(err, os.ErrNotExist) ||
|
||||
xerrors.Is(err, os.ErrPermission) ||
|
||||
isNoSuchProcessErr(err))
|
||||
}
|
||||
|
||||
func isNoSuchProcessErr(err error) bool {
|
||||
return err != nil && strings.Contains(err.Error(), "no such process")
|
||||
}
|
||||
|
@ -21,9 +21,7 @@ import (
|
||||
"runtime"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@ -37,7 +35,6 @@ import (
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/goleak"
|
||||
"go.uber.org/mock/gomock"
|
||||
"golang.org/x/crypto/ssh"
|
||||
"golang.org/x/exp/slices"
|
||||
"golang.org/x/xerrors"
|
||||
@ -45,11 +42,8 @@ import (
|
||||
"tailscale.com/tailcfg"
|
||||
|
||||
"cdr.dev/slog"
|
||||
"cdr.dev/slog/sloggers/sloghuman"
|
||||
"cdr.dev/slog/sloggers/slogtest"
|
||||
"github.com/coder/coder/v2/agent"
|
||||
"github.com/coder/coder/v2/agent/agentproc"
|
||||
"github.com/coder/coder/v2/agent/agentproc/agentproctest"
|
||||
"github.com/coder/coder/v2/agent/agentssh"
|
||||
"github.com/coder/coder/v2/agent/agenttest"
|
||||
"github.com/coder/coder/v2/agent/proto"
|
||||
@ -2668,242 +2662,6 @@ func TestAgent_Metrics_SSH(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestAgent_ManageProcessPriority(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("OK", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if runtime.GOOS != "linux" {
|
||||
t.Skip("Skipping non-linux environment")
|
||||
}
|
||||
|
||||
var (
|
||||
expectedProcs = map[int32]agentproc.Process{}
|
||||
fs = afero.NewMemMapFs()
|
||||
syscaller = agentproctest.NewMockSyscaller(gomock.NewController(t))
|
||||
ticker = make(chan time.Time)
|
||||
modProcs = make(chan []*agentproc.Process)
|
||||
logger = slog.Make(sloghuman.Sink(io.Discard))
|
||||
)
|
||||
|
||||
requireFileWrite(t, fs, "/proc/self/oom_score_adj", "-500")
|
||||
|
||||
// Create some processes.
|
||||
for i := 0; i < 4; i++ {
|
||||
// Create a prioritized process.
|
||||
var proc agentproc.Process
|
||||
if i == 0 {
|
||||
proc = agentproctest.GenerateProcess(t, fs,
|
||||
func(p *agentproc.Process) {
|
||||
p.CmdLine = "./coder\x00agent\x00--no-reap"
|
||||
p.PID = int32(i)
|
||||
},
|
||||
)
|
||||
} else {
|
||||
proc = agentproctest.GenerateProcess(t, fs,
|
||||
func(p *agentproc.Process) {
|
||||
// Make the cmd something similar to a prioritized
|
||||
// process but differentiate the arguments.
|
||||
p.CmdLine = "./coder\x00stat"
|
||||
},
|
||||
)
|
||||
|
||||
syscaller.EXPECT().GetPriority(proc.PID).Return(20, nil)
|
||||
syscaller.EXPECT().SetPriority(proc.PID, 10).Return(nil)
|
||||
}
|
||||
syscaller.EXPECT().
|
||||
Kill(proc.PID, syscall.Signal(0)).
|
||||
Return(nil)
|
||||
|
||||
expectedProcs[proc.PID] = proc
|
||||
}
|
||||
|
||||
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
|
||||
o.Syscaller = syscaller
|
||||
o.ModifiedProcesses = modProcs
|
||||
o.EnvironmentVariables = map[string]string{agent.EnvProcPrioMgmt: "1"}
|
||||
o.Filesystem = fs
|
||||
o.Logger = logger
|
||||
o.ProcessManagementTick = ticker
|
||||
})
|
||||
actualProcs := <-modProcs
|
||||
require.Len(t, actualProcs, len(expectedProcs)-1)
|
||||
for _, proc := range actualProcs {
|
||||
requireFileEquals(t, fs, fmt.Sprintf("/proc/%d/oom_score_adj", proc.PID), "0")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("IgnoreCustomNice", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if runtime.GOOS != "linux" {
|
||||
t.Skip("Skipping non-linux environment")
|
||||
}
|
||||
|
||||
var (
|
||||
expectedProcs = map[int32]agentproc.Process{}
|
||||
fs = afero.NewMemMapFs()
|
||||
ticker = make(chan time.Time)
|
||||
syscaller = agentproctest.NewMockSyscaller(gomock.NewController(t))
|
||||
modProcs = make(chan []*agentproc.Process)
|
||||
logger = slog.Make(sloghuman.Sink(io.Discard))
|
||||
)
|
||||
|
||||
err := afero.WriteFile(fs, "/proc/self/oom_score_adj", []byte("0"), 0o644)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create some processes.
|
||||
for i := 0; i < 3; i++ {
|
||||
proc := agentproctest.GenerateProcess(t, fs)
|
||||
syscaller.EXPECT().
|
||||
Kill(proc.PID, syscall.Signal(0)).
|
||||
Return(nil)
|
||||
|
||||
if i == 0 {
|
||||
// Set a random nice score. This one should not be adjusted by
|
||||
// our management loop.
|
||||
syscaller.EXPECT().GetPriority(proc.PID).Return(25, nil)
|
||||
} else {
|
||||
syscaller.EXPECT().GetPriority(proc.PID).Return(20, nil)
|
||||
syscaller.EXPECT().SetPriority(proc.PID, 10).Return(nil)
|
||||
}
|
||||
|
||||
expectedProcs[proc.PID] = proc
|
||||
}
|
||||
|
||||
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
|
||||
o.Syscaller = syscaller
|
||||
o.ModifiedProcesses = modProcs
|
||||
o.EnvironmentVariables = map[string]string{agent.EnvProcPrioMgmt: "1"}
|
||||
o.Filesystem = fs
|
||||
o.Logger = logger
|
||||
o.ProcessManagementTick = ticker
|
||||
})
|
||||
actualProcs := <-modProcs
|
||||
// We should ignore the process with a custom nice score.
|
||||
require.Len(t, actualProcs, 2)
|
||||
for _, proc := range actualProcs {
|
||||
_, ok := expectedProcs[proc.PID]
|
||||
require.True(t, ok)
|
||||
requireFileEquals(t, fs, fmt.Sprintf("/proc/%d/oom_score_adj", proc.PID), "998")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("CustomOOMScore", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if runtime.GOOS != "linux" {
|
||||
t.Skip("Skipping non-linux environment")
|
||||
}
|
||||
|
||||
var (
|
||||
fs = afero.NewMemMapFs()
|
||||
ticker = make(chan time.Time)
|
||||
syscaller = agentproctest.NewMockSyscaller(gomock.NewController(t))
|
||||
modProcs = make(chan []*agentproc.Process)
|
||||
logger = slog.Make(sloghuman.Sink(io.Discard))
|
||||
)
|
||||
|
||||
err := afero.WriteFile(fs, "/proc/self/oom_score_adj", []byte("0"), 0o644)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create some processes.
|
||||
for i := 0; i < 3; i++ {
|
||||
proc := agentproctest.GenerateProcess(t, fs)
|
||||
syscaller.EXPECT().
|
||||
Kill(proc.PID, syscall.Signal(0)).
|
||||
Return(nil)
|
||||
syscaller.EXPECT().GetPriority(proc.PID).Return(20, nil)
|
||||
syscaller.EXPECT().SetPriority(proc.PID, 10).Return(nil)
|
||||
}
|
||||
|
||||
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
|
||||
o.Syscaller = syscaller
|
||||
o.ModifiedProcesses = modProcs
|
||||
o.EnvironmentVariables = map[string]string{
|
||||
agent.EnvProcPrioMgmt: "1",
|
||||
agent.EnvProcOOMScore: "-567",
|
||||
}
|
||||
o.Filesystem = fs
|
||||
o.Logger = logger
|
||||
o.ProcessManagementTick = ticker
|
||||
})
|
||||
actualProcs := <-modProcs
|
||||
// We should ignore the process with a custom nice score.
|
||||
require.Len(t, actualProcs, 3)
|
||||
for _, proc := range actualProcs {
|
||||
requireFileEquals(t, fs, fmt.Sprintf("/proc/%d/oom_score_adj", proc.PID), "-567")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("DisabledByDefault", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if runtime.GOOS != "linux" {
|
||||
t.Skip("Skipping non-linux environment")
|
||||
}
|
||||
|
||||
var (
|
||||
buf bytes.Buffer
|
||||
wr = &syncWriter{
|
||||
w: &buf,
|
||||
}
|
||||
)
|
||||
log := slog.Make(sloghuman.Sink(wr)).Leveled(slog.LevelDebug)
|
||||
|
||||
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
|
||||
o.Logger = log
|
||||
})
|
||||
|
||||
require.Eventually(t, func() bool {
|
||||
wr.mu.Lock()
|
||||
defer wr.mu.Unlock()
|
||||
return strings.Contains(buf.String(), "process priority not enabled")
|
||||
}, testutil.WaitLong, testutil.IntervalFast)
|
||||
})
|
||||
|
||||
t.Run("DisabledForNonLinux", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if runtime.GOOS == "linux" {
|
||||
t.Skip("Skipping linux environment")
|
||||
}
|
||||
|
||||
var (
|
||||
buf bytes.Buffer
|
||||
wr = &syncWriter{
|
||||
w: &buf,
|
||||
}
|
||||
)
|
||||
log := slog.Make(sloghuman.Sink(wr)).Leveled(slog.LevelDebug)
|
||||
|
||||
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
|
||||
o.Logger = log
|
||||
// Try to enable it so that we can assert that non-linux
|
||||
// environments are truly disabled.
|
||||
o.EnvironmentVariables = map[string]string{agent.EnvProcPrioMgmt: "1"}
|
||||
})
|
||||
require.Eventually(t, func() bool {
|
||||
wr.mu.Lock()
|
||||
defer wr.mu.Unlock()
|
||||
|
||||
return strings.Contains(buf.String(), "process priority not enabled")
|
||||
}, testutil.WaitLong, testutil.IntervalFast)
|
||||
})
|
||||
}
|
||||
|
||||
type syncWriter struct {
|
||||
mu sync.Mutex
|
||||
w io.Writer
|
||||
}
|
||||
|
||||
func (s *syncWriter) Write(p []byte) (int, error) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
return s.w.Write(p)
|
||||
}
|
||||
|
||||
// echoOnce accepts a single connection, reads 4 bytes and echos them back
|
||||
func echoOnce(t *testing.T, ll net.Listener) {
|
||||
t.Helper()
|
||||
@ -2933,17 +2691,3 @@ func requireEcho(t *testing.T, conn net.Conn) {
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "test", string(b))
|
||||
}
|
||||
|
||||
func requireFileWrite(t testing.TB, fs afero.Fs, fp, data string) {
|
||||
t.Helper()
|
||||
err := afero.WriteFile(fs, fp, []byte(data), 0o600)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func requireFileEquals(t testing.TB, fs afero.Fs, fp, expect string) {
|
||||
t.Helper()
|
||||
actual, err := afero.ReadFile(fs, fp)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Equal(t, expect, string(actual))
|
||||
}
|
||||
|
@ -23,7 +23,8 @@ const unset = -2000
|
||||
// CLI runs the agent-exec command. It should only be called by the cli package.
|
||||
func CLI() error {
|
||||
// We lock the OS thread here to avoid a race condition where the nice priority
|
||||
// we get is on a different thread from the one we set it on.
|
||||
// we set gets applied to a different thread than the one we exec the provided
|
||||
// command on.
|
||||
runtime.LockOSThread()
|
||||
// Nop on success but we do it anyway in case of an error.
|
||||
defer runtime.UnlockOSThread()
|
||||
@ -68,12 +69,18 @@ func CLI() error {
|
||||
|
||||
err = unix.Setpriority(unix.PRIO_PROCESS, 0, *nice)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("set nice score: %w", err)
|
||||
// We alert the user instead of failing the command since it can be difficult to debug
|
||||
// for a template admin otherwise. It's quite possible (and easy) to set an
|
||||
// inappriopriate value for niceness.
|
||||
printfStdErr("failed to adjust niceness to %q: %v", *nice, err)
|
||||
}
|
||||
|
||||
err = writeOOMScoreAdj(*oom)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("set oom score: %w", err)
|
||||
// We alert the user instead of failing the command since it can be difficult to debug
|
||||
// for a template admin otherwise. It's quite possible (and easy) to set an
|
||||
// inappriopriate value for oom_score_adj.
|
||||
printfStdErr("failed to adjust oom score to %q: %v", *nice, err)
|
||||
}
|
||||
|
||||
path, err := exec.LookPath(args[0])
|
||||
@ -143,3 +150,7 @@ func execArgs(args []string) []string {
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func printfStdErr(format string, a ...any) {
|
||||
_, _ = fmt.Fprintf(os.Stderr, "coder-agent: %s\n", fmt.Sprintf(format, a...))
|
||||
}
|
||||
|
@ -10,6 +10,8 @@ import (
|
||||
"strconv"
|
||||
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/pty"
|
||||
)
|
||||
|
||||
const (
|
||||
@ -25,19 +27,39 @@ const (
|
||||
// is returned. All instances of exec.Cmd should flow through this function to ensure
|
||||
// proper resource constraints are applied to the child process.
|
||||
func CommandContext(ctx context.Context, cmd string, args ...string) (*exec.Cmd, error) {
|
||||
cmd, args, err := agentExecCmd(cmd, args...)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("agent exec cmd: %w", err)
|
||||
}
|
||||
return exec.CommandContext(ctx, cmd, args...), nil
|
||||
}
|
||||
|
||||
// PTYCommandContext returns an pty.Cmd that calls "coder agent-exec" prior to exec'ing
|
||||
// the provided command if CODER_PROC_PRIO_MGMT is set, otherwise a normal pty.Cmd
|
||||
// is returned. All instances of pty.Cmd should flow through this function to ensure
|
||||
// proper resource constraints are applied to the child process.
|
||||
func PTYCommandContext(ctx context.Context, cmd string, args ...string) (*pty.Cmd, error) {
|
||||
cmd, args, err := agentExecCmd(cmd, args...)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("agent exec cmd: %w", err)
|
||||
}
|
||||
return pty.CommandContext(ctx, cmd, args...), nil
|
||||
}
|
||||
|
||||
func agentExecCmd(cmd string, args ...string) (string, []string, error) {
|
||||
_, enabled := os.LookupEnv(EnvProcPrioMgmt)
|
||||
if runtime.GOOS != "linux" || !enabled {
|
||||
return exec.CommandContext(ctx, cmd, args...), nil
|
||||
return cmd, args, nil
|
||||
}
|
||||
|
||||
executable, err := os.Executable()
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get executable: %w", err)
|
||||
return "", nil, xerrors.Errorf("get executable: %w", err)
|
||||
}
|
||||
|
||||
bin, err := filepath.EvalSymlinks(executable)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("eval symlinks: %w", err)
|
||||
return "", nil, xerrors.Errorf("eval symlinks: %w", err)
|
||||
}
|
||||
|
||||
execArgs := []string{"agent-exec"}
|
||||
@ -51,7 +73,7 @@ func CommandContext(ctx context.Context, cmd string, args ...string) (*exec.Cmd,
|
||||
execArgs = append(execArgs, "--", cmd)
|
||||
execArgs = append(execArgs, args...)
|
||||
|
||||
return exec.CommandContext(ctx, bin, execArgs...), nil
|
||||
return bin, execArgs, nil
|
||||
}
|
||||
|
||||
// envValInt searches for a key in a list of environment variables and parses it to an int.
|
||||
|
@ -1,5 +0,0 @@
|
||||
// Package agentproctest contains utility functions
|
||||
// for testing process management in the agent.
|
||||
package agentproctest
|
||||
|
||||
//go:generate mockgen -destination ./syscallermock.go -package agentproctest github.com/coder/coder/v2/agent/agentproc Syscaller
|
@ -1,55 +0,0 @@
|
||||
package agentproctest
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/agent/agentproc"
|
||||
"github.com/coder/coder/v2/cryptorand"
|
||||
)
|
||||
|
||||
func GenerateProcess(t *testing.T, fs afero.Fs, muts ...func(*agentproc.Process)) agentproc.Process {
|
||||
t.Helper()
|
||||
|
||||
pid, err := cryptorand.Intn(1<<31 - 1)
|
||||
require.NoError(t, err)
|
||||
|
||||
arg1, err := cryptorand.String(5)
|
||||
require.NoError(t, err)
|
||||
|
||||
arg2, err := cryptorand.String(5)
|
||||
require.NoError(t, err)
|
||||
|
||||
arg3, err := cryptorand.String(5)
|
||||
require.NoError(t, err)
|
||||
|
||||
cmdline := fmt.Sprintf("%s\x00%s\x00%s", arg1, arg2, arg3)
|
||||
|
||||
process := agentproc.Process{
|
||||
CmdLine: cmdline,
|
||||
PID: int32(pid),
|
||||
OOMScoreAdj: 0,
|
||||
}
|
||||
|
||||
for _, mut := range muts {
|
||||
mut(&process)
|
||||
}
|
||||
|
||||
process.Dir = fmt.Sprintf("%s/%d", "/proc", process.PID)
|
||||
|
||||
err = fs.MkdirAll(process.Dir, 0o555)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = afero.WriteFile(fs, fmt.Sprintf("%s/cmdline", process.Dir), []byte(process.CmdLine), 0o444)
|
||||
require.NoError(t, err)
|
||||
|
||||
score := strconv.Itoa(process.OOMScoreAdj)
|
||||
err = afero.WriteFile(fs, fmt.Sprintf("%s/oom_score_adj", process.Dir), []byte(score), 0o444)
|
||||
require.NoError(t, err)
|
||||
|
||||
return process
|
||||
}
|
@ -1,83 +0,0 @@
|
||||
// Code generated by MockGen. DO NOT EDIT.
|
||||
// Source: github.com/coder/coder/v2/agent/agentproc (interfaces: Syscaller)
|
||||
//
|
||||
// Generated by this command:
|
||||
//
|
||||
// mockgen -destination ./syscallermock.go -package agentproctest github.com/coder/coder/v2/agent/agentproc Syscaller
|
||||
//
|
||||
|
||||
// Package agentproctest is a generated GoMock package.
|
||||
package agentproctest
|
||||
|
||||
import (
|
||||
reflect "reflect"
|
||||
syscall "syscall"
|
||||
|
||||
gomock "go.uber.org/mock/gomock"
|
||||
)
|
||||
|
||||
// MockSyscaller is a mock of Syscaller interface.
|
||||
type MockSyscaller struct {
|
||||
ctrl *gomock.Controller
|
||||
recorder *MockSyscallerMockRecorder
|
||||
}
|
||||
|
||||
// MockSyscallerMockRecorder is the mock recorder for MockSyscaller.
|
||||
type MockSyscallerMockRecorder struct {
|
||||
mock *MockSyscaller
|
||||
}
|
||||
|
||||
// NewMockSyscaller creates a new mock instance.
|
||||
func NewMockSyscaller(ctrl *gomock.Controller) *MockSyscaller {
|
||||
mock := &MockSyscaller{ctrl: ctrl}
|
||||
mock.recorder = &MockSyscallerMockRecorder{mock}
|
||||
return mock
|
||||
}
|
||||
|
||||
// EXPECT returns an object that allows the caller to indicate expected use.
|
||||
func (m *MockSyscaller) EXPECT() *MockSyscallerMockRecorder {
|
||||
return m.recorder
|
||||
}
|
||||
|
||||
// GetPriority mocks base method.
|
||||
func (m *MockSyscaller) GetPriority(arg0 int32) (int, error) {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "GetPriority", arg0)
|
||||
ret0, _ := ret[0].(int)
|
||||
ret1, _ := ret[1].(error)
|
||||
return ret0, ret1
|
||||
}
|
||||
|
||||
// GetPriority indicates an expected call of GetPriority.
|
||||
func (mr *MockSyscallerMockRecorder) GetPriority(arg0 any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPriority", reflect.TypeOf((*MockSyscaller)(nil).GetPriority), arg0)
|
||||
}
|
||||
|
||||
// Kill mocks base method.
|
||||
func (m *MockSyscaller) Kill(arg0 int32, arg1 syscall.Signal) error {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "Kill", arg0, arg1)
|
||||
ret0, _ := ret[0].(error)
|
||||
return ret0
|
||||
}
|
||||
|
||||
// Kill indicates an expected call of Kill.
|
||||
func (mr *MockSyscallerMockRecorder) Kill(arg0, arg1 any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Kill", reflect.TypeOf((*MockSyscaller)(nil).Kill), arg0, arg1)
|
||||
}
|
||||
|
||||
// SetPriority mocks base method.
|
||||
func (m *MockSyscaller) SetPriority(arg0 int32, arg1 int) error {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "SetPriority", arg0, arg1)
|
||||
ret0, _ := ret[0].(error)
|
||||
return ret0
|
||||
}
|
||||
|
||||
// SetPriority indicates an expected call of SetPriority.
|
||||
func (mr *MockSyscallerMockRecorder) SetPriority(arg0, arg1 any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetPriority", reflect.TypeOf((*MockSyscaller)(nil).SetPriority), arg0, arg1)
|
||||
}
|
@ -1,3 +0,0 @@
|
||||
// Package agentproc contains logic for interfacing with local
|
||||
// processes running in the same context as the agent.
|
||||
package agentproc
|
@ -1,24 +0,0 @@
|
||||
//go:build !linux
|
||||
// +build !linux
|
||||
|
||||
package agentproc
|
||||
|
||||
import (
|
||||
"github.com/spf13/afero"
|
||||
)
|
||||
|
||||
func (*Process) Niceness(Syscaller) (int, error) {
|
||||
return 0, errUnimplemented
|
||||
}
|
||||
|
||||
func (*Process) SetNiceness(Syscaller, int) error {
|
||||
return errUnimplemented
|
||||
}
|
||||
|
||||
func (*Process) Cmd() string {
|
||||
return ""
|
||||
}
|
||||
|
||||
func List(afero.Fs, Syscaller) ([]*Process, error) {
|
||||
return nil, errUnimplemented
|
||||
}
|
@ -1,166 +0,0 @@
|
||||
package agentproc_test
|
||||
|
||||
import (
|
||||
"runtime"
|
||||
"syscall"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/mock/gomock"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/agent/agentproc"
|
||||
"github.com/coder/coder/v2/agent/agentproc/agentproctest"
|
||||
)
|
||||
|
||||
func TestList(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if runtime.GOOS != "linux" {
|
||||
t.Skipf("skipping non-linux environment")
|
||||
}
|
||||
|
||||
t.Run("OK", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
fs = afero.NewMemMapFs()
|
||||
sc = agentproctest.NewMockSyscaller(gomock.NewController(t))
|
||||
expectedProcs = make(map[int32]agentproc.Process)
|
||||
)
|
||||
|
||||
for i := 0; i < 4; i++ {
|
||||
proc := agentproctest.GenerateProcess(t, fs)
|
||||
expectedProcs[proc.PID] = proc
|
||||
|
||||
sc.EXPECT().
|
||||
Kill(proc.PID, syscall.Signal(0)).
|
||||
Return(nil)
|
||||
}
|
||||
|
||||
actualProcs, err := agentproc.List(fs, sc)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, actualProcs, len(expectedProcs))
|
||||
for _, proc := range actualProcs {
|
||||
expected, ok := expectedProcs[proc.PID]
|
||||
require.True(t, ok)
|
||||
require.Equal(t, expected.PID, proc.PID)
|
||||
require.Equal(t, expected.CmdLine, proc.CmdLine)
|
||||
require.Equal(t, expected.Dir, proc.Dir)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("FinishedProcess", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
fs = afero.NewMemMapFs()
|
||||
sc = agentproctest.NewMockSyscaller(gomock.NewController(t))
|
||||
expectedProcs = make(map[int32]agentproc.Process)
|
||||
)
|
||||
|
||||
for i := 0; i < 3; i++ {
|
||||
proc := agentproctest.GenerateProcess(t, fs)
|
||||
expectedProcs[proc.PID] = proc
|
||||
|
||||
sc.EXPECT().
|
||||
Kill(proc.PID, syscall.Signal(0)).
|
||||
Return(nil)
|
||||
}
|
||||
|
||||
// Create a process that's already finished. We're not adding
|
||||
// it to the map because it should be skipped over.
|
||||
proc := agentproctest.GenerateProcess(t, fs)
|
||||
sc.EXPECT().
|
||||
Kill(proc.PID, syscall.Signal(0)).
|
||||
Return(xerrors.New("os: process already finished"))
|
||||
|
||||
actualProcs, err := agentproc.List(fs, sc)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, actualProcs, len(expectedProcs))
|
||||
for _, proc := range actualProcs {
|
||||
expected, ok := expectedProcs[proc.PID]
|
||||
require.True(t, ok)
|
||||
require.Equal(t, expected.PID, proc.PID)
|
||||
require.Equal(t, expected.CmdLine, proc.CmdLine)
|
||||
require.Equal(t, expected.Dir, proc.Dir)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("NoSuchProcess", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
fs = afero.NewMemMapFs()
|
||||
sc = agentproctest.NewMockSyscaller(gomock.NewController(t))
|
||||
expectedProcs = make(map[int32]agentproc.Process)
|
||||
)
|
||||
|
||||
for i := 0; i < 3; i++ {
|
||||
proc := agentproctest.GenerateProcess(t, fs)
|
||||
expectedProcs[proc.PID] = proc
|
||||
|
||||
sc.EXPECT().
|
||||
Kill(proc.PID, syscall.Signal(0)).
|
||||
Return(nil)
|
||||
}
|
||||
|
||||
// Create a process that doesn't exist. We're not adding
|
||||
// it to the map because it should be skipped over.
|
||||
proc := agentproctest.GenerateProcess(t, fs)
|
||||
sc.EXPECT().
|
||||
Kill(proc.PID, syscall.Signal(0)).
|
||||
Return(syscall.ESRCH)
|
||||
|
||||
actualProcs, err := agentproc.List(fs, sc)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, actualProcs, len(expectedProcs))
|
||||
for _, proc := range actualProcs {
|
||||
expected, ok := expectedProcs[proc.PID]
|
||||
require.True(t, ok)
|
||||
require.Equal(t, expected.PID, proc.PID)
|
||||
require.Equal(t, expected.CmdLine, proc.CmdLine)
|
||||
require.Equal(t, expected.Dir, proc.Dir)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// These tests are not very interesting but they provide some modicum of
|
||||
// confidence.
|
||||
func TestProcess(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if runtime.GOOS != "linux" {
|
||||
t.Skipf("skipping non-linux environment")
|
||||
}
|
||||
|
||||
t.Run("SetNiceness", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
sc = agentproctest.NewMockSyscaller(gomock.NewController(t))
|
||||
proc = &agentproc.Process{
|
||||
PID: 32,
|
||||
}
|
||||
score = 20
|
||||
)
|
||||
|
||||
sc.EXPECT().SetPriority(proc.PID, score).Return(nil)
|
||||
err := proc.SetNiceness(sc, score)
|
||||
require.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("Cmd", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
proc = &agentproc.Process{
|
||||
CmdLine: "helloworld\x00--arg1\x00--arg2",
|
||||
}
|
||||
expectedName = "helloworld --arg1 --arg2"
|
||||
)
|
||||
|
||||
require.Equal(t, expectedName, proc.Cmd())
|
||||
})
|
||||
}
|
@ -1,134 +0,0 @@
|
||||
//go:build linux
|
||||
// +build linux
|
||||
|
||||
package agentproc
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"syscall"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
"golang.org/x/xerrors"
|
||||
)
|
||||
|
||||
func List(fs afero.Fs, syscaller Syscaller) ([]*Process, error) {
|
||||
d, err := fs.Open(defaultProcDir)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("open dir %q: %w", defaultProcDir, err)
|
||||
}
|
||||
defer d.Close()
|
||||
|
||||
entries, err := d.Readdirnames(0)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("readdirnames: %w", err)
|
||||
}
|
||||
|
||||
processes := make([]*Process, 0, len(entries))
|
||||
for _, entry := range entries {
|
||||
pid, err := strconv.ParseInt(entry, 10, 32)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
// Check that the process still exists.
|
||||
exists, err := isProcessExist(syscaller, int32(pid))
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("check process exists: %w", err)
|
||||
}
|
||||
if !exists {
|
||||
continue
|
||||
}
|
||||
|
||||
cmdline, err := afero.ReadFile(fs, filepath.Join(defaultProcDir, entry, "cmdline"))
|
||||
if err != nil {
|
||||
if isBenignError(err) {
|
||||
continue
|
||||
}
|
||||
return nil, xerrors.Errorf("read cmdline: %w", err)
|
||||
}
|
||||
|
||||
oomScore, err := afero.ReadFile(fs, filepath.Join(defaultProcDir, entry, "oom_score_adj"))
|
||||
if err != nil {
|
||||
if isBenignError(err) {
|
||||
continue
|
||||
}
|
||||
|
||||
return nil, xerrors.Errorf("read oom_score_adj: %w", err)
|
||||
}
|
||||
|
||||
oom, err := strconv.Atoi(strings.TrimSpace(string(oomScore)))
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("convert oom score: %w", err)
|
||||
}
|
||||
|
||||
processes = append(processes, &Process{
|
||||
PID: int32(pid),
|
||||
CmdLine: string(cmdline),
|
||||
Dir: filepath.Join(defaultProcDir, entry),
|
||||
OOMScoreAdj: oom,
|
||||
})
|
||||
}
|
||||
|
||||
return processes, nil
|
||||
}
|
||||
|
||||
func isProcessExist(syscaller Syscaller, pid int32) (bool, error) {
|
||||
err := syscaller.Kill(pid, syscall.Signal(0))
|
||||
if err == nil {
|
||||
return true, nil
|
||||
}
|
||||
if err.Error() == "os: process already finished" {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
var errno syscall.Errno
|
||||
if !errors.As(err, &errno) {
|
||||
return false, err
|
||||
}
|
||||
|
||||
switch errno {
|
||||
case syscall.ESRCH:
|
||||
return false, nil
|
||||
case syscall.EPERM:
|
||||
return true, nil
|
||||
}
|
||||
|
||||
return false, xerrors.Errorf("kill: %w", err)
|
||||
}
|
||||
|
||||
func (p *Process) Niceness(sc Syscaller) (int, error) {
|
||||
nice, err := sc.GetPriority(p.PID)
|
||||
if err != nil {
|
||||
return 0, xerrors.Errorf("get priority for %q: %w", p.CmdLine, err)
|
||||
}
|
||||
return nice, nil
|
||||
}
|
||||
|
||||
func (p *Process) SetNiceness(sc Syscaller, score int) error {
|
||||
err := sc.SetPriority(p.PID, score)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("set priority for %q: %w", p.CmdLine, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (p *Process) Cmd() string {
|
||||
return strings.Join(p.cmdLine(), " ")
|
||||
}
|
||||
|
||||
func (p *Process) cmdLine() []string {
|
||||
return strings.Split(p.CmdLine, "\x00")
|
||||
}
|
||||
|
||||
func isBenignError(err error) bool {
|
||||
var errno syscall.Errno
|
||||
if !xerrors.As(err, &errno) {
|
||||
return false
|
||||
}
|
||||
|
||||
return errno == syscall.ESRCH || errno == syscall.EPERM || xerrors.Is(err, os.ErrNotExist)
|
||||
}
|
@ -1,21 +0,0 @@
|
||||
package agentproc
|
||||
|
||||
import (
|
||||
"syscall"
|
||||
)
|
||||
|
||||
type Syscaller interface {
|
||||
SetPriority(pid int32, priority int) error
|
||||
GetPriority(pid int32) (int, error)
|
||||
Kill(pid int32, sig syscall.Signal) error
|
||||
}
|
||||
|
||||
// nolint: unused // used on some but no all platforms
|
||||
const defaultProcDir = "/proc"
|
||||
|
||||
type Process struct {
|
||||
Dir string
|
||||
CmdLine string
|
||||
PID int32
|
||||
OOMScoreAdj int
|
||||
}
|
@ -1,30 +0,0 @@
|
||||
//go:build !linux
|
||||
// +build !linux
|
||||
|
||||
package agentproc
|
||||
|
||||
import (
|
||||
"syscall"
|
||||
|
||||
"golang.org/x/xerrors"
|
||||
)
|
||||
|
||||
func NewSyscaller() Syscaller {
|
||||
return nopSyscaller{}
|
||||
}
|
||||
|
||||
var errUnimplemented = xerrors.New("unimplemented")
|
||||
|
||||
type nopSyscaller struct{}
|
||||
|
||||
func (nopSyscaller) SetPriority(int32, int) error {
|
||||
return errUnimplemented
|
||||
}
|
||||
|
||||
func (nopSyscaller) GetPriority(int32) (int, error) {
|
||||
return 0, errUnimplemented
|
||||
}
|
||||
|
||||
func (nopSyscaller) Kill(int32, syscall.Signal) error {
|
||||
return errUnimplemented
|
||||
}
|
@ -1,42 +0,0 @@
|
||||
//go:build linux
|
||||
// +build linux
|
||||
|
||||
package agentproc
|
||||
|
||||
import (
|
||||
"syscall"
|
||||
|
||||
"golang.org/x/sys/unix"
|
||||
"golang.org/x/xerrors"
|
||||
)
|
||||
|
||||
func NewSyscaller() Syscaller {
|
||||
return UnixSyscaller{}
|
||||
}
|
||||
|
||||
type UnixSyscaller struct{}
|
||||
|
||||
func (UnixSyscaller) SetPriority(pid int32, nice int) error {
|
||||
err := unix.Setpriority(unix.PRIO_PROCESS, int(pid), nice)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("set priority: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (UnixSyscaller) GetPriority(pid int32) (int, error) {
|
||||
nice, err := unix.Getpriority(0, int(pid))
|
||||
if err != nil {
|
||||
return 0, xerrors.Errorf("get priority: %w", err)
|
||||
}
|
||||
return nice, nil
|
||||
}
|
||||
|
||||
func (UnixSyscaller) Kill(pid int32, sig syscall.Signal) error {
|
||||
err := syscall.Kill(int(pid), sig)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("kill: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
@ -30,6 +30,7 @@ import (
|
||||
|
||||
"cdr.dev/slog"
|
||||
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/coder/v2/agent/usershell"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/pty"
|
||||
@ -725,7 +726,10 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string)
|
||||
}
|
||||
}
|
||||
|
||||
cmd := pty.CommandContext(ctx, name, args...)
|
||||
cmd, err := agentexec.PTYCommandContext(ctx, name, args...)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("pty command context: %w", err)
|
||||
}
|
||||
cmd.Dir = s.config.WorkingDirectory()
|
||||
|
||||
// If the metadata directory doesn't exist, we run the command
|
||||
|
@ -14,6 +14,7 @@ import (
|
||||
|
||||
"cdr.dev/slog"
|
||||
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/coder/v2/pty"
|
||||
)
|
||||
|
||||
@ -58,7 +59,11 @@ func newBuffered(ctx context.Context, cmd *pty.Cmd, options *Options, logger slo
|
||||
|
||||
// Add TERM then start the command with a pty. pty.Cmd duplicates Path as the
|
||||
// first argument so remove it.
|
||||
cmdWithEnv := pty.CommandContext(ctx, cmd.Path, cmd.Args[1:]...)
|
||||
cmdWithEnv, err := agentexec.PTYCommandContext(ctx, cmd.Path, cmd.Args[1:]...)
|
||||
if err != nil {
|
||||
rpty.state.setState(StateDone, xerrors.Errorf("pty command context: %w", err))
|
||||
return rpty
|
||||
}
|
||||
cmdWithEnv.Env = append(rpty.command.Env, "TERM=xterm-256color")
|
||||
cmdWithEnv.Dir = rpty.command.Dir
|
||||
ptty, process, err := pty.Start(cmdWithEnv)
|
||||
|
@ -9,7 +9,6 @@ import (
|
||||
"io"
|
||||
"net"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
@ -20,6 +19,7 @@ import (
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog"
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/coder/v2/pty"
|
||||
)
|
||||
|
||||
@ -210,7 +210,7 @@ func (rpty *screenReconnectingPTY) doAttach(ctx context.Context, conn net.Conn,
|
||||
logger.Debug(ctx, "spawning screen client", slog.F("screen_id", rpty.id))
|
||||
|
||||
// Wrap the command with screen and tie it to the connection's context.
|
||||
cmd := pty.CommandContext(ctx, "screen", append([]string{
|
||||
cmd, err := agentexec.PTYCommandContext(ctx, "screen", append([]string{
|
||||
// -S is for setting the session's name.
|
||||
"-S", rpty.id,
|
||||
// -U tells screen to use UTF-8 encoding.
|
||||
@ -223,6 +223,9 @@ func (rpty *screenReconnectingPTY) doAttach(ctx context.Context, conn net.Conn,
|
||||
rpty.command.Path,
|
||||
// pty.Cmd duplicates Path as the first argument so remove it.
|
||||
}, rpty.command.Args[1:]...)...)
|
||||
if err != nil {
|
||||
return nil, nil, xerrors.Errorf("pty command context: %w", err)
|
||||
}
|
||||
cmd.Env = append(rpty.command.Env, "TERM=xterm-256color")
|
||||
cmd.Dir = rpty.command.Dir
|
||||
ptty, process, err := pty.Start(cmd, pty.WithPTYOption(
|
||||
@ -327,10 +330,10 @@ func (rpty *screenReconnectingPTY) sendCommand(ctx context.Context, command stri
|
||||
defer cancel()
|
||||
|
||||
var lastErr error
|
||||
run := func() bool {
|
||||
run := func() (bool, error) {
|
||||
var stdout bytes.Buffer
|
||||
//nolint:gosec
|
||||
cmd := exec.CommandContext(ctx, "screen",
|
||||
cmd, err := agentexec.CommandContext(ctx, "screen",
|
||||
// -x targets an attached session.
|
||||
"-x", rpty.id,
|
||||
// -c is the flag for the config file.
|
||||
@ -338,18 +341,21 @@ func (rpty *screenReconnectingPTY) sendCommand(ctx context.Context, command stri
|
||||
// -X runs a command in the matching session.
|
||||
"-X", command,
|
||||
)
|
||||
if err != nil {
|
||||
return false, xerrors.Errorf("command context: %w", err)
|
||||
}
|
||||
cmd.Env = append(rpty.command.Env, "TERM=xterm-256color")
|
||||
cmd.Dir = rpty.command.Dir
|
||||
cmd.Stdout = &stdout
|
||||
err := cmd.Run()
|
||||
err = cmd.Run()
|
||||
if err == nil {
|
||||
return true
|
||||
return true, nil
|
||||
}
|
||||
|
||||
stdoutStr := stdout.String()
|
||||
for _, se := range successErrors {
|
||||
if strings.Contains(stdoutStr, se) {
|
||||
return true
|
||||
return true, nil
|
||||
}
|
||||
}
|
||||
|
||||
@ -359,11 +365,15 @@ func (rpty *screenReconnectingPTY) sendCommand(ctx context.Context, command stri
|
||||
lastErr = xerrors.Errorf("`screen -x %s -X %s`: %w: %s", rpty.id, command, err, stdoutStr)
|
||||
}
|
||||
|
||||
return false
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// Run immediately.
|
||||
if done := run(); done {
|
||||
done, err := run()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if done {
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -379,7 +389,11 @@ func (rpty *screenReconnectingPTY) sendCommand(ctx context.Context, command stri
|
||||
}
|
||||
return errors.Join(ctx.Err(), lastErr)
|
||||
case <-ticker.C:
|
||||
if done := run(); done {
|
||||
done, err := run()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if done {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
@ -1,10 +1,7 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/netip"
|
||||
"sync"
|
||||
"testing"
|
||||
@ -16,8 +13,6 @@ import (
|
||||
|
||||
"tailscale.com/types/netlogtype"
|
||||
|
||||
"cdr.dev/slog"
|
||||
"cdr.dev/slog/sloggers/slogjson"
|
||||
"github.com/coder/coder/v2/agent/proto"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
@ -213,58 +208,3 @@ func newFakeStatsDest() *fakeStatsDest {
|
||||
resps: make(chan *proto.UpdateStatsResponse),
|
||||
}
|
||||
}
|
||||
|
||||
func Test_logDebouncer(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
buf bytes.Buffer
|
||||
logger = slog.Make(slogjson.Sink(&buf))
|
||||
ctx = context.Background()
|
||||
)
|
||||
|
||||
debouncer := &logDebouncer{
|
||||
logger: logger,
|
||||
messages: map[string]time.Time{},
|
||||
interval: time.Minute,
|
||||
}
|
||||
|
||||
fields := map[string]interface{}{
|
||||
"field_1": float64(1),
|
||||
"field_2": "2",
|
||||
}
|
||||
|
||||
debouncer.Error(ctx, "my message", "field_1", 1, "field_2", "2")
|
||||
debouncer.Warn(ctx, "another message", "field_1", 1, "field_2", "2")
|
||||
// Shouldn't log this.
|
||||
debouncer.Warn(ctx, "another message", "field_1", 1, "field_2", "2")
|
||||
|
||||
require.Len(t, debouncer.messages, 2)
|
||||
|
||||
type entry struct {
|
||||
Msg string `json:"msg"`
|
||||
Level string `json:"level"`
|
||||
Fields map[string]interface{} `json:"fields"`
|
||||
}
|
||||
|
||||
assertLog := func(msg string, level string, fields map[string]interface{}) {
|
||||
line, err := buf.ReadString('\n')
|
||||
require.NoError(t, err)
|
||||
|
||||
var e entry
|
||||
err = json.Unmarshal([]byte(line), &e)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, msg, e.Msg)
|
||||
require.Equal(t, level, e.Level)
|
||||
require.Equal(t, fields, e.Fields)
|
||||
}
|
||||
assertLog("my message", "ERROR", fields)
|
||||
assertLog("another message", "WARN", fields)
|
||||
|
||||
debouncer.messages["another message"] = time.Now().Add(-2 * time.Minute)
|
||||
debouncer.Warn(ctx, "another message", "field_1", 1, "field_2", "2")
|
||||
assertLog("another message", "WARN", fields)
|
||||
// Assert nothing else was written.
|
||||
_, err := buf.ReadString('\n')
|
||||
require.ErrorIs(t, err, io.EOF)
|
||||
}
|
||||
|
27
cli/agent.go
27
cli/agent.go
@ -25,7 +25,7 @@ import (
|
||||
"cdr.dev/slog/sloggers/slogjson"
|
||||
"cdr.dev/slog/sloggers/slogstackdriver"
|
||||
"github.com/coder/coder/v2/agent"
|
||||
"github.com/coder/coder/v2/agent/agentproc"
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/coder/v2/agent/agentssh"
|
||||
"github.com/coder/coder/v2/agent/reaper"
|
||||
"github.com/coder/coder/v2/buildinfo"
|
||||
@ -171,6 +171,7 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
|
||||
slog.F("auth", auth),
|
||||
slog.F("version", version),
|
||||
)
|
||||
|
||||
client := agentsdk.New(r.agentURL)
|
||||
client.SDK.SetLogger(logger)
|
||||
// Set a reasonable timeout so requests can't hang forever!
|
||||
@ -292,11 +293,20 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
|
||||
environmentVariables := map[string]string{
|
||||
"GIT_ASKPASS": executablePath,
|
||||
}
|
||||
if v, ok := os.LookupEnv(agent.EnvProcPrioMgmt); ok {
|
||||
environmentVariables[agent.EnvProcPrioMgmt] = v
|
||||
}
|
||||
if v, ok := os.LookupEnv(agent.EnvProcOOMScore); ok {
|
||||
environmentVariables[agent.EnvProcOOMScore] = v
|
||||
|
||||
enabled := os.Getenv(agentexec.EnvProcPrioMgmt)
|
||||
if enabled != "" && runtime.GOOS == "linux" {
|
||||
logger.Info(ctx, "process priority management enabled",
|
||||
slog.F("env_var", agentexec.EnvProcPrioMgmt),
|
||||
slog.F("enabled", enabled),
|
||||
slog.F("os", runtime.GOOS),
|
||||
)
|
||||
} else {
|
||||
logger.Info(ctx, "process priority management not enabled (linux-only) ",
|
||||
slog.F("env_var", agentexec.EnvProcPrioMgmt),
|
||||
slog.F("enabled", enabled),
|
||||
slog.F("os", runtime.GOOS),
|
||||
)
|
||||
}
|
||||
|
||||
agnt := agent.New(agent.Options{
|
||||
@ -322,11 +332,6 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
|
||||
Subsystems: subsystems,
|
||||
|
||||
PrometheusRegistry: prometheusRegistry,
|
||||
Syscaller: agentproc.NewSyscaller(),
|
||||
// Intentionally set this to nil. It's mainly used
|
||||
// for testing.
|
||||
ModifiedProcesses: nil,
|
||||
|
||||
BlockFileTransfer: blockFileTransfer,
|
||||
})
|
||||
|
||||
|
@ -1,12 +1,20 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
_ "time/tzdata"
|
||||
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/coder/v2/cli"
|
||||
)
|
||||
|
||||
func main() {
|
||||
if len(os.Args) > 1 && os.Args[1] == "agent-exec" {
|
||||
err := agentexec.CLI()
|
||||
_, _ = fmt.Fprintln(os.Stderr, err)
|
||||
os.Exit(1)
|
||||
}
|
||||
var rootCmd cli.RootCmd
|
||||
rootCmd.RunWithSubcommands(rootCmd.AGPL())
|
||||
}
|
||||
|
@ -1,12 +1,21 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
_ "time/tzdata"
|
||||
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
entcli "github.com/coder/coder/v2/enterprise/cli"
|
||||
)
|
||||
|
||||
func main() {
|
||||
if len(os.Args) > 1 && os.Args[1] == "agent-exec" {
|
||||
err := agentexec.CLI()
|
||||
_, _ = fmt.Fprintln(os.Stderr, err)
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
var rootCmd entcli.RootCmd
|
||||
rootCmd.RunWithSubcommands(rootCmd.EnterpriseSubcommands())
|
||||
}
|
||||
|
@ -487,3 +487,39 @@ func workspaceActivity(m dsl.Matcher) {
|
||||
!m.File().Name.Matches(`_test\.go$`),
|
||||
).Report("Updating workspace activity should always be done in the workspacestats package.")
|
||||
}
|
||||
|
||||
// noExecInAgent ensures that packages under agent/ don't use exec.Command or
|
||||
// exec.CommandContext directly.
|
||||
//
|
||||
//nolint:unused,deadcode,varnamelen
|
||||
func noExecInAgent(m dsl.Matcher) {
|
||||
m.Import("os/exec")
|
||||
m.Match(
|
||||
`exec.Command($*_)`,
|
||||
`exec.CommandContext($*_)`,
|
||||
).
|
||||
Where(
|
||||
m.File().PkgPath.Matches("/agent/") &&
|
||||
!m.File().PkgPath.Matches("/agentexec") &&
|
||||
!m.File().Name.Matches(`_test\.go$`),
|
||||
).
|
||||
Report("The agent and its subpackages should not use exec.Command or exec.CommandContext directly. Consider using agentexec.CommandContext instead.")
|
||||
}
|
||||
|
||||
// noPTYInAgent ensures that packages under agent/ don't use pty.Command or
|
||||
// pty.CommandContext directly.
|
||||
//
|
||||
//nolint:unused,deadcode,varnamelen
|
||||
func noPTYInAgent(m dsl.Matcher) {
|
||||
m.Import("github.com/coder/coder/v2/pty")
|
||||
m.Match(
|
||||
`pty.Command($*_)`,
|
||||
`pty.CommandContext($*_)`,
|
||||
).
|
||||
Where(
|
||||
m.File().PkgPath.Matches(`/agent/`) &&
|
||||
!m.File().PkgPath.Matches(`/agentexec`) &&
|
||||
!m.File().Name.Matches(`_test\.go$`),
|
||||
).
|
||||
Report("The agent and its subpackages should not use pty.Command or pty.CommandContext directly. Consider using agentexec.PTYCommandContext instead.")
|
||||
}
|
||||
|
Reference in New Issue
Block a user