8000 feat(agent/agentcontainers): implement ignore customization for devco… · coder/coder@5bac6ef · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 5bac6ef

Browse files
mafredrimafredri
committed
feat(agent/agentcontainers): implement ignore customization for devcontainers
Fixes coder/internal#737
1 parent 3fb5d0b commit 5bac6ef

File tree

3 files changed

+291
-43
lines changed

3 files changed

+291
-43
lines changed

‎agent/agentcontainers/api.go

Lines changed: 123 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ type API struct {
7878
devcontainerNames map[string]bool // By devcontainer name.
7979
knownDevcontainers map[string]codersdk.WorkspaceAgentDevcontainer // By workspace folder.
8080
configFileModifiedTimes map[string]time.Time // By config file path.
81+
ignoredDevcontainers map[string]bool // By workspace folder. Tracks three states (true, false and not checked).
8182
recreateSuccessTimes map[string]time.Time // By workspace folder.
8283
recreateErrorTimes map[string]time.Time // By workspace folder.
8384
injectedSubAgentProcs map[string]subAgentProcess // By workspace folder.
@@ -274,6 +275,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
274275
devcontainerNames: make(map[string]bool),
275276
knownDevcontainers: make(map[string]codersdk.WorkspaceAgentDevcontainer),
276277
configFileModifiedTimes: make(map[string]time.Time),
278+
ignoredDevcontainers: make(map[string]bool),
277279
recreateSuccessTimes: make(map[string]time.Time),
278280
recreateErrorTimes: make(map[string]time.Time),
279281
scriptLogger: func(uuid.UUID) ScriptLogger { return noopScriptLogger{} },
@@ -750,6 +752,10 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse,
750752
if len(api.knownDevcontainers) > 0 {
751753
devcontainers = make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers))
752754
for _, dc := range api.knownDevcontainers {
755+
if api.ignoredDevcontainers[dc.WorkspaceFolder] {
756+
continue
757+
}
758+
753759
// Include the agent if it's running (we're iterating over
754760
// copies, so mutating is fine).
755761
if proc := api.injectedSubAgentProcs[dc.WorkspaceFolder]; proc.agent.ID != uuid.Nil {
@@ -982,11 +988,46 @@ func (api *API) markDevcontainerDirty(configPath string, modifiedAt time.Time) {
982988
logger.Info(api.ctx, "marking devcontainer as dirty")
983989
dc.Dirty = true
984990
}
991+
if api.ignoredDevcontainers[dc.WorkspaceFolder] {
992+
logger.Debug(api.ctx, "clearing devcontainer ignored state")
993+
delete(api.ignoredDevcontainers, dc.WorkspaceFolder) // Allow re-reading config.
994+
}
985995

986996
api.knownDevcontainers[dc.WorkspaceFolder] = dc
987997
}
988998
}
989999

1000+
// markDevcontainerIgnored marks a devcontainer as ignored. Must not be called
1001+
// with the API lock held.
1002+
func (api *API) markDevcontainerIgnored(ctx context.Context, dc codersdk.WorkspaceAgentDevcontainer, proc subAgentProcess) subAgentProcess {
1003+
logger := api.logger.With(
1004+
slog.F("devcontainer_id", dc.ID),
1005+
slog.F("devcontainer_name", dc.Name),
1006+
slog.F("workspace_folder", dc.WorkspaceFolder),
1007+
slog.F("config_path", dc.ConfigPath),
1008+
)
1009+
1010+
// We only allow ignore to be set in the root customization layer to
1011+
// prevent weird interactions with devcontainer features.
1012+
logger.Debug(ctx, "marking devcontainer as ignored")
1013+
proc.stop()
1014+
if proc.agent.ID != uuid.Nil {
1015+
// If we stop the subagent, we also need to delete it.
1016+
client := *api.subAgentClient.Load()
1017+
if err := client.Delete(ctx, proc.agent.ID); err != nil {
1018+
api.logger.Error(ctx, "delete subagent failed", slog.Error(err), slog.F("subagent_id", proc.agent.ID))
1019+
}
1020+
}
1021+
// Reset agent and containerID to force config re-reading if ignore is toggled.
1022+
proc.agent = SubAgent{}
1023+
proc.containerID = ""
1024+
api.mu.Lock()
1025+
api.ignoredDevcontainers[dc.WorkspaceFolder] = true
1026+
api.mu.Unlock()
1027+
1028+
return proc
1029+
}
1030+
9901031
// cleanupSubAgents removes subagents that are no longer managed by
9911032
// this agent. This is usually only run at startup to ensure a clean
9921033
// slate. This method has an internal timeout to prevent blocking
@@ -1038,6 +1079,10 @@ func (api *API) cleanupSubAgents(ctx context.Context) error {
10381079
// This method uses an internal timeout to prevent blocking indefinitely
10391080
// if something goes wrong with the injection.
10401081
func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc codersdk.WorkspaceAgentDevcontainer) (err error) {
1082+
if api.ignoredDevcontainers[dc.WorkspaceFolder] {
1083+
return nil
1084+
}
1085+
10411086
ctx, cancel := context.WithTimeout(ctx, defaultOperationTimeout)
10421087
defer cancel()
10431088

@@ -1059,6 +1104,29 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
10591104
maybeRecreateSubAgent := false
10601105
proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder]
10611106
if injected {
1107+
if _, ignoreChecked := api.ignoredDevcontainers[dc.WorkspaceFolder]; !ignoreChecked {
1108+
// If ignore status has not yet been checked, or cleared by
1109+
// modifications to the devcontainer.json, we must read it
1110+
// to determine the current status. This can happen while
1111+
// the devcontainer subagent is already running or before
1112+
// we've had a chance to inject it.
1113+
//
1114+
// Note, for simplicity, we do not try to optimize to reduce
1115+
// ReadConfig calls here.
1116+
config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath, nil)
1117+
if err != nil {
1118+
return xerrors.Errorf("read devcontainer config: %w", err)
1119+
}
1120+
1121+
if config.Configuration.Customizations.Coder.Ignore {
1122+
api.mu.Unlock()
1123+
proc = api.markDevcontainerIgnored(ctx, dc, proc)
1124+
api.mu.Lock()
1125+
api.injectedSubAgentProcs[dc.WorkspaceFolder] = proc
1126+
return nil
1127+
}
1128+
}
1129+
10621130
if proc.containerID == container.ID && proc.ctx.Err() == nil {
10631131
// Same container and running, no need to reinject.
10641132
return< F438 /span> nil
@@ -1077,7 +1145,8 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
10771145
// Container ID changed or the subagent process is not running,
10781146
// stop the existing subagent context to replace it.
10791147
proc.stop()
1080-
} else {
1148+
}
1149+
if proc.agent.OperatingSystem == "" {
10811150
// Set SubAgent defaults.
10821151
proc.agent.OperatingSystem = "linux" // Assuming Linux for devcontainers.
10831152
}
@@ -1134,48 +1203,6 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11341203
proc.agent.Architecture = arch
11351204
}
11361205

1137-
agentBinaryPath, err := os.Executable()
1138-
if err != nil {
1139-
return xerrors.Errorf("get agent binary path: %w", err)
1140-
}
1141-
agentBinaryPath, err = filepath.EvalSymlinks(agentBinaryPath)
1142-
if err != nil {
1143-
return xerrors.Errorf("resolve agent binary path: %w", err)
1144-
}
1145-
1146-
// If we scripted this as a `/bin/sh` script, we could reduce these
1147-
// steps to one instruction, speeding up the injection process.
1148-
//
1149-
// Note: We use `path` instead of `filepath` here because we are
1150-
// working with Unix-style paths inside the container.
1151-
if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "mkdir", "-p", path.Dir(coderPathInsideContainer)); err != nil {
1152-
return xerrors.Errorf("create agent directory in container: %w", err)
1153-
}
1154-
1155-
if err := api.ccli.Copy(ctx, container.ID, agentBinaryPath, coderPathInsideContainer); err != nil {
1156-
return xerrors.Errorf("copy agent binary: %w", err)
1157-
}
1158-
1159-
logger.Info(ctx, "copied agent binary to container")
1160-
1161-
// Make sure the agent binary is executable so we can run it (the
1162-
// user doesn't matter since we're making it executable for all).
1163-
if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "chmod", "0755", path.Dir(coderPathInsideContainer), coderPathInsideContainer); err != nil {
1164-
return xerrors.Errorf("set agent binary executable: %w", err)
1165-
}
1166-
1167-
// Attempt to add CAP_NET_ADMIN to the binary to improve network
1168-
// performance (optional, allow to fail). See `bootstrap_linux.sh`.
1169-
// TODO(mafredri): Disable for now until we can figure out why this
1170-
// causes the following error on some images:
1171-
//
1172-
// Image: mcr.microsoft.com/devcontainers/base:ubuntu
1173-
// Error: /.coder-agent/coder: Operation not permitted
1174-
//
1175-
// if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "setcap", "cap_net_admin+ep", coderPathInsideContainer); err != nil {
1176-
// logger.Warn(ctx, "set CAP_NET_ADMIN on agent binary failed", slog.Error(err))
1177-
// }
1178-
11791206
subAgentConfig := proc.agent.CloneConfig(dc)
11801207
if proc.agent.ID == uuid.Nil || maybeRecreateSubAgent {
11811208
subAgentConfig.Architecture = arch
@@ -1192,6 +1219,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11921219
}
11931220

11941221
var (
1222+
ignore bool
11951223
appsWithPossibleDuplicates []SubAgentApp
11961224
workspaceFolder = DevcontainerDefaultContainerWorkspaceFolder
11971225
)
@@ -1215,8 +1243,13 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12151243
return err
12161244
}
12171245

1246+
ignore = config.Configuration.Customizations.Coder.Ignore
12181247
workspaceFolder = config.Workspace.WorkspaceFolder
12191248

1249+
if ignore {
1250+
return nil
1251+
}
1252+
12201253
// NOTE(DanielleMaywood):
12211254
// We only want to take an agent name specified in the root customization layer.
12221255
// This restricts the ability for a feature to specify the agent name. We may revisit
@@ -1262,6 +1295,11 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12621295
api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err))
12631296
}
12641297

1298+
if ignore {
1299+
proc = api.markDevcontainerIgnored(ctx, dc, proc)
1300+
return nil
1301+
}
1302+
12651303
displayApps := make([]codersdk.DisplayApp, 0, len(displayAppsMap))
12661304
for app, enabled := range displayAppsMap {
12671305
if enabled {
@@ -1294,6 +1332,48 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12941332
subAgentConfig.Directory = workspaceFolder
12951333
}
12961334

1335+
agentBinaryPath, err := os.Executable()
1336+
if err != nil {
1337+
return xerrors.Errorf("get agent binary path: %w", err)
1338+
}
1339+
agentBinaryPath, err = filepath.EvalSymlinks(agentBinaryPath)
1340+
if err != nil {
1341+
return xerrors.Errorf("resolve agent binary path: %w", err)
1342+
}
1343+
1344+
// If we scripted this as a `/bin/sh` script, we could reduce these
1345+
// steps to one instruction, speeding up the injection process.
1346+
//
1347+
// Note: We use `path` instead of `filepath` here because we are
1348+
// working with Unix-style paths inside the container.
1349+
if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "mkdir", "-p", path.Dir(coderPathInsideContainer)); err != nil {
1350+
return xerrors.Errorf("create agent directory in container: %w", err)
1351+
}
1352+
1353+
if err := api.ccli.Copy(ctx, container.ID, agentBinaryPath, coderPathInsideContainer); err != nil {
1354+
return xerrors.Errorf("copy agent binary: %w", err)
1355+
}
1356+
1357+
logger.Info(ctx, "copied agent binary to container")
1358+
1359+
// Make sure the agent binary is executable so we can run it (the
1360+
// user doesn't matter since we're making it executable for all).
1361+
if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "chmod", "0755", path.Dir(coderPathInsideContainer), coderPathInsideContainer); err != nil {
1362+
return xerrors.Errorf("set agent binary executable: %w", err)
1363+
}
1364+
1365+
// Attempt to add CAP_NET_ADMIN to the binary to improve network
1366+
// performance (optional, allow to fail). See `bootstrap_linux.sh`.
1367+
// TODO(mafredri): Disable for now until we can figure out why this
1368+
// causes the following error on som 7799 e images:
1369+
//
1370+
// Image: mcr.microsoft.com/devcontainers/base:ubuntu
1371+
// Error: /.coder-agent/coder: Operation not permitted
1372+
//
1373+
// if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "setcap", "cap_net_admin+ep", coderPathInsideContainer); err != nil {
1374+
// logger.Warn(ctx, "set CAP_NET_ADMIN on agent binary failed", slog.Error(err))
1375+
// }
1376+
12971377
deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig)
12981378
if deleteSubAgent {
12991379
logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID))

0 commit comments

Comments
 (0)
0