Skip to content

Commit 5570b0b

Browse files
sreyasreya
committed
Remove process priority management system
1 parent e0af8e1 commit 5570b0b

File tree

4 files changed

+19
-349
lines changed

4 files changed

+19
-349
lines changed

‎agent/agent.go

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import (
3535
"tailscale.com/util/clientmetric"
3636

3737
"cdr.dev/slog"
38-
"github.com/coder/coder/v2/agent/agentproc"
3938
"github.com/coder/coder/v2/agent/agentscripts"
4039
"github.com/coder/coder/v2/agent/agentssh"
4140
"github.com/coder/coder/v2/agent/proto"
@@ -82,12 +81,7 @@ type Options struct {
8281
PrometheusRegistry *prometheus.Registry
8382
ReportMetadataInterval time.Duration
8483
ServiceBannerRefreshInterval time.Duration
85-
Syscaller agentproc.Syscaller
86-
// ModifiedProcesses is used for testing process priority management.
87-
ModifiedProcesses chan []*agentproc.Process
88-
// ProcessManagementTick is used for testing process priority management.
89-
ProcessManagementTick <-chan time.Time
90-
BlockFileTransfer bool
84+
BlockFileTransfer bool
9185
}
9286

9387
type Client interface {
@@ -147,10 +141,6 @@ func New(options Options) Agent {
147141
prometheusRegistry = prometheus.NewRegistry()
148142
}
149143

150-
if options.Syscaller == nil {
151-
options.Syscaller = agentproc.NewSyscaller()
152-
}
153-
154144
hardCtx, hardCancel := context.WithCancel(context.Background())
155145
gracefulCtx, gracefulCancel := context.WithCancel(hardCtx)
156146
a := &agent{
@@ -178,9 +168,6 @@ func New(options Options) Agent {
178168
announcementBannersRefreshInterval: options.ServiceBannerRefreshInterval,
179169
sshMaxTimeout: options.SSHMaxTimeout,
180170
subsystems: options.Subsystems,
181-
syscaller: options.Syscaller,
182-
modifiedProcs: options.ModifiedProcesses,
183-
processManagementTick: options.ProcessManagementTick,
184171
logSender: agentsdk.NewLogSender(options.Logger),
185172
blockFileTransfer: options.BlockFileTransfer,
186173

@@ -255,13 +242,7 @@ type agent struct {
255242
prometheusRegistry *prometheus.Registry
256243
// metrics are prometheus registered metrics that will be collected and
257244
// labeled in Coder with the agent + workspace.
258-
metrics *agentMetrics
259-
syscaller agentproc.Syscaller
260-
261-
// modifiedProcs is used for testing process priority management.
262-
modifiedProcs chan []*agentproc.Process
263-
// processManagementTick is used for testing process priority management.
264-
processManagementTick <-chan time.Time
245+
metrics *agentMetrics
265246
}
266247

267248
func (a *agent) TailnetConn() *tailnet.Conn {

‎agent/agent_test.go

Lines changed: 0 additions & 256 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ import (
2121
"runtime"
2222
"strconv"
2323
"strings"
24-
"sync"
2524
"sync/atomic"
26-
"syscall"
2725
"testing"
2826
"time"
2927

@@ -37,19 +35,15 @@ import (
3735
"github.com/stretchr/testify/assert"
3836
"github.com/stretchr/testify/require"
3937
"go.uber.org/goleak"
40-
"go.uber.org/mock/gomock"
4138
"golang.org/x/crypto/ssh"
4239
"golang.org/x/exp/slices"
4340
"golang.org/x/xerrors"
4441
"tailscale.com/net/speedtest"
4542
"tailscale.com/tailcfg"
4643

4744
"cdr.dev/slog"
48-
"cdr.dev/slog/sloggers/sloghuman"
4945
"cdr.dev/slog/sloggers/slogtest"
5046
"github.com/coder/coder/v2/agent"
51-
"github.com/coder/coder/v2/agent/agentproc"
52-
"github.com/coder/coder/v2/agent/agentproc/agentproctest"
5347
"github.com/coder/coder/v2/agent/agentssh"
5448
"github.com/coder/coder/v2/agent/agenttest"
5549
"github.com/coder/coder/v2/agent/proto"
@@ -2668,242 +2662,6 @@ func TestAgent_Metrics_SSH(t *testing.T) {
26682662
require.NoError(t, err)
26692663
}
26702664

2671-
func TestAgent_ManageProcessPriority(t *testing.T) {
2672-
t.Parallel()
2673-
2674-
t.Run("OK", func(t *testing.T) {
2675-
t.Parallel()
2676-
2677-
if runtime.GOOS != "linux" {
2678-
t.Skip("Skipping non-linux environment")
2679-
}
2680-
2681-
var (
2682-
expectedProcs = map[int32]agentproc.Process{}
2683-
fs = afero.NewMemMapFs()
2684-
syscaller = agentproctest.NewMockSyscaller(gomock.NewController(t))
2685-
ticker = make(chan time.Time)
2686-
modProcs = make(chan []*agentproc.Process)
2687-
logger = slog.Make(sloghuman.Sink(io.Discard))
2688-
)
2689-
2690-
requireFileWrite(t, fs, "/proc/self/oom_score_adj", "-500")
2691-
2692-
// Create some processes.
2693-
for i := 0; i < 4; i++ {
2694-
// Create a prioritized process.
2695-
var proc agentproc.Process
2696-
if i == 0 {
2697-
proc = agentproctest.GenerateProcess(t, fs,
2698-
func(p *agentproc.Process) {
2699-
p.CmdLine = "./coder\x00agent\x00--no-reap"
2700-
p.PID = int32(i)
2701-
},
2702-
)
2703-
} else {
2704-
proc = agentproctest.GenerateProcess(t, fs,
2705-
func(p *agentproc.Process) {
2706-
// Make the cmd something similar to a prioritized
2707-
// process but differentiate the arguments.
2708-
p.CmdLine = "./coder\x00stat"
2709-
},
2710-
)
2711-
2712-
syscaller.EXPECT().GetPriority(proc.PID).Return(20, nil)
2713-
syscaller.EXPECT().SetPriority(proc.PID, 10).Return(nil)
2714-
}
2715-
syscaller.EXPECT().
2716-
Kill(proc.PID, syscall.Signal(0)).
2717-
Return(nil)
2718-
2719-
expectedProcs[proc.PID] = proc
2720-
}
2721-
2722-
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
2723-
o.Syscaller = syscaller
2724-
o.ModifiedProcesses = modProcs
2725-
o.EnvironmentVariables = map[string]string{agent.EnvProcPrioMgmt: "1"}
2726-
o.Filesystem = fs
2727-
o.Logger = logger
2728-
o.ProcessManagementTick = ticker
2729-
})
2730-
actualProcs := <-modProcs
2731-
require.Len(t, actualProcs, len(expectedProcs)-1)
2732-
for _, proc := range actualProcs {
2733-
requireFileEquals(t, fs, fmt.Sprintf("/proc/%d/oom_score_adj", proc.PID), "0")
2734-
}
2735-
})
2736-
2737-
t.Run("IgnoreCustomNice", func(t *testing.T) {
2738-
t.Parallel()
2739-
2740-
if runtime.GOOS != "linux" {
2741-
t.Skip("Skipping non-linux environment")
2742-
}
2743-
2744-
var (
2745-
expectedProcs = map[int32]agentproc.Process{}
2746-
fs = afero.NewMemMapFs()
2747-
ticker = make(chan time.Time)
2748-
syscaller = agentproctest.NewMockSyscaller(gomock.NewController(t))
2749-
modProcs = make(chan []*agentproc.Process)
2750-
logger = slog.Make(sloghuman.Sink(io.Discard))
2751-
)
2752-
2753-
err := afero.WriteFile(fs, "/proc/self/oom_score_adj", []byte("0"), 0o644)
2754-
require.NoError(t, err)
2755-
2756-
// Create some processes.
2757-
for i := 0; i < 3; i++ {
2758-
proc := agentproctest.GenerateProcess(t, fs)
2759-
syscaller.EXPECT().
2760-
Kill(proc.PID, syscall.Signal(0)).
2761-
Return(nil)
2762-
2763-
if i == 0 {
2764-
// Set a random nice score. This one should not be adjusted by
2765-
// our management loop.
2766-
syscaller.EXPECT().GetPriority(proc.PID).Return(25, nil)
2767-
} else {
2768-
syscaller.EXPECT().GetPriority(proc.PID).Return(20, nil)
2769-
syscaller.EXPECT().SetPriority(proc.PID, 10).Return(nil)
2770-
}
2771-
2772-
expectedProcs[proc.PID] = proc
2773-
}
2774-
2775-
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
2776-
o.Syscaller = syscaller
2777-
o.ModifiedProcesses = modProcs
2778-
o.EnvironmentVariables = map[string]string{agent.EnvProcPrioMgmt: "1"}
2779-
o.Filesystem = fs
2780-
o.Logger = logger
2781-
o.ProcessManagementTick = ticker
2782-
})
2783-
actualProcs := <-modProcs
2784-
// We should ignore the process with a custom nice score.
2785-
require.Len(t, actualProcs, 2)
2786-
for _, proc := range actualProcs {
2787-
_, ok := expectedProcs[proc.PID]
2788-
require.True(t, ok)
2789-
requireFileEquals(t, fs, fmt.Sprintf("/proc/%d/oom_score_adj", proc.PID), "998")
2790-
}
2791-
})
2792-
2793-
t.Run("CustomOOMScore", func(t *testing.T) {
2794-
t.Parallel()
2795-
2796-
if runtime.GOOS != "linux" {
2797-
t.Skip("Skipping non-linux environment")
2798-
}
2799-
2800-
var (
2801-
fs = afero.NewMemMapFs()
2802-
ticker = make(chan time.Time)
2803-
syscaller = agentproctest.NewMockSyscaller(gomock.NewController(t))
2804-
modProcs = make(chan []*agentproc.Process)
2805-
logger = slog.Make(sloghuman.Sink(io.Discard))
2806-
)
2807-
2808-
err := afero.WriteFile(fs, "/proc/self/oom_score_adj", []byte("0"), 0o644)
2809-
require.NoError(t, err)
2810-
2811-
// Create some processes.
2812-
for i := 0; i < 3; i++ {
2813-
proc := agentproctest.GenerateProcess(t, fs)
2814-
syscaller.EXPECT().
2815-
Kill(proc.PID, syscall.Signal(0)).
2816-
Return(nil)
2817-
syscaller.EXPECT().GetPriority(proc.PID).Return(20, nil)
2818-
syscaller.EXPECT().SetPriority(proc.PID, 10).Return(nil)
2819-
}
2820-
2821-
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
2822-
o.Syscaller = syscaller
2823-
o.ModifiedProcesses = modProcs
2824-
o.EnvironmentVariables = map[string]string{
2825-
agent.EnvProcPrioMgmt: "1",
2826-
agent.EnvProcOOMScore: "-567",
2827-
}
2828-
o.Filesystem = fs
2829-
o.Logger = logger
2830-
o.ProcessManagementTick = ticker
2831-
})
2832-
actualProcs := <-modProcs
2833-
// We should ignore the process with a custom nice score.
2834-
require.Len(t, actualProcs, 3)
2835-
for _, proc := range actualProcs {
2836-
requireFileEquals(t, fs, fmt.Sprintf("/proc/%d/oom_score_adj", proc.PID), "-567")
2837-
}
2838-
})
2839-
2840-
t.Run("DisabledByDefault", func(t *testing.T) {
2841-
t.Parallel()
2842-
2843-
if runtime.GOOS != "linux" {
2844-
t.Skip("Skipping non-linux environment")
2845-
}
2846-
2847-
var (
2848-
buf bytes.Buffer
2849-
wr = &syncWriter{
2850-
w: &buf,
2851-
}
2852-
)
2853-
log := slog.Make(sloghuman.Sink(wr)).Leveled(slog.LevelDebug)
2854-
2855-
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
2856-
o.Logger = log
2857-
})
2858-
2859-
require.Eventually(t, func() bool {
2860-
wr.mu.Lock()
2861-
defer wr.mu.Unlock()
2862-
return strings.Contains(buf.String(), "process priority not enabled")
2863-
}, testutil.WaitLong, testutil.IntervalFast)
2864-
})
2865-
2866-
t.Run("DisabledForNonLinux", func(t *testing.T) {
2867-
t.Parallel()
2868-
2869-
if runtime.GOOS == "linux" {
2870-
t.Skip("Skipping linux environment")
2871-
}
2872-
2873-
var (
2874-
buf bytes.Buffer
2875-
wr = &syncWriter{
2876-
w: &buf,
2877-
}
2878-
)
2879-
log := slog.Make(sloghuman.Sink(wr)).Leveled(slog.LevelDebug)
2880-
2881-
_, _, _, _, _ = setupAgent(t, agentsdk.Manifest{}, 0, func(c *agenttest.Client, o *agent.Options) {
2882-
o.Logger = log
2883-
// Try to enable it so that we can assert that non-linux
2884-
// environments are truly disabled.
2885-
o.EnvironmentVariables = map[string]string{agent.EnvProcPrioMgmt: "1"}
2886-
})
2887-
require.Eventually(t, func() bool {
2888-
wr.mu.Lock()
2889-
defer wr.mu.Unlock()
2890-
2891-
return strings.Contains(buf.String(), "process priority not enabled")
2892-
}, testutil.WaitLong, testutil.IntervalFast)
2893-
})
2894-
}
2895-
2896-
type syncWriter struct {
2897-
mu sync.Mutex
2898-
w io.Writer
2899-
}
2900-
2901-
func (s *syncWriter) Write(p []byte) (int, error) {
2902-
s.mu.Lock()
2903-
defer s.mu.Unlock()
2904-
return s.w.Write(p)
2905-
}
2906-
29072665
// echoOnce accepts a single connection, reads 4 bytes and echos them back
29082666
func echoOnce(t *testing.T, ll net.Listener) {
29092667
t.Helper()
@@ -2933,17 +2691,3 @@ func requireEcho(t *testing.T, conn net.Conn) {
29332691
require.NoError(t, err)
29342692
require.Equal(t, "test", string(b))
29352693
}
2936-
2937-
func requireFileWrite(t testing.TB, fs afero.Fs, fp, data string) {
2938-
t.Helper()
2939-
err := afero.WriteFile(fs, fp, []byte(data), 0o600)
2940-
require.NoError(t, err)
2941-
}
2942-
2943-
func requireFileEquals(t testing.TB, fs afero.Fs, fp, expect string) {
2944-
t.Helper()
2945-
actual, err := afero.ReadFile(fs, fp)
2946-
require.NoError(t, err)
2947-
2948-
require.Equal(t, expect, string(actual))
2949-
}

0 commit comments

Comments
 (0)