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

Commit e443f86

Browse files
mafredrimafredri
authored
feat(agent/agentcontainers): implement ignore customization for devcontainers (#18530)
Fixes coder/internal#737
1 parent 06c997a commit e443f86

File tree

4 files changed

+294
-43
lines changed

4 files changed

+294
-43
lines changed

‎agent/agentcontainers/api.go

Lines changed: 121 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ type API struct {
8383
recreateErrorTimes map[string]time.Time // By workspace folder.
8484
injectedSubAgentProcs map[string]subAgentProcess // By workspace folder.
8585
usingWorkspaceFolderName map[string]bool // By workspace folder.
86+
ignoredDevcontainers map[string]bool // By workspace folder. Tracks three states (true, false and not checked).
8687
asyncWg sync.WaitGroup
8788

8889
devcontainerLogSourceIDs map[string]uuid.UUID // By workspace folder.
@@ -276,6 +277,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
276277
devcontainerNames: make(map[string]bool),
277278
knownDevcontainers: make(map[string]codersdk.WorkspaceAgentDevcontainer),
278279
configFileModifiedTimes: make(map[string]time.Time),
280+
ignoredDevcontainers: make(map[string]bool),
279281
recreateSuccessTimes: make(map[string]time.Time),
280282
recreateErrorTimes: make(map[string]time.Time),
281283
scriptLogger: func(uuid.UUID) ScriptLogger { return noopScriptLogger{} },
@@ -804,6 +806,10 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse,
804806
if len(api.knownDevcontainers) > 0 {
805807
devcontainers = make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers))
806808
for _, dc := range api.knownDevcontainers {
809+
if api.ignoredDevcontainers[dc.WorkspaceFolder] {
810+
continue
811+
}
812+
807813
// Include the agent if it's running (we're iterating over
808814
// copies, so mutating is fine).
809815
if proc := api.injectedSubAgentProcs[dc.WorkspaceFolder]; proc.agent.ID != uuid.Nil {
@@ -1036,6 +1042,10 @@ func (api *API) markDevcontainerDirty(configPath string, modifiedAt time.Time) {
10361042
logger.Info(api.ctx, "marking devcontainer as dirty")
10371043
dc.Dirty = true
10381044
}
1045+
if _, ok := api.ignoredDevcontainers[dc.WorkspaceFolder]; ok {
1046+
logger.Debug(api.ctx, "clearing devcontainer ignored state")
1047+
delete(api.ignoredDevcontainers, dc.WorkspaceFolder) // Allow re-reading config.
1048+
}
10391049

10401050
api.knownDevcontainers[dc.WorkspaceFolder] = dc
10411051
}
@@ -1092,6 +1102,10 @@ func (api *API) cleanupSubAgents(ctx context.Context) error {
10921102
// This method uses an internal timeout to prevent blocking indefinitely
10931103
// if something goes wrong with the injection.
10941104
func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc codersdk.WorkspaceAgentDevcontainer) (err error) {
1105+
if api.ignoredDevcontainers[dc.WorkspaceFolder] {
1106+
return nil
1107+
}
1108+
10951109
ctx, cancel := context.WithTimeout(ctx, defaultOperationTimeout)
10961110
defer cancel()
10971111

@@ -1113,6 +1127,42 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11131127
maybeRecreateSubAgent := false
11141128
proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder]
11151129
if injected {
1130+
if _, ignoreChecked := api.ignoredDevcontainers[dc.WorkspaceFolder]; !ignoreChecked {
1131+
// If ignore status has not yet been checked, or cleared by
1132+
// modifications to the devcontainer.json, we must read it
1133+
// to determine the current status. This can happen while
1134+
// the devcontainer subagent is already running or before
1135+
// we've had a chance to inject it.
1136+
//
1137+
// Note, for simplicity, we do not try to optimize to reduce
1138+
// ReadConfig calls here.
1139+
config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath, nil)
1140+
if err != nil {
1141+
return xerrors.Errorf("read devcontainer config: %w", err)
1142+
}
1143+
1144+
dcIgnored := config.Configuration.Customizations.Coder.Ignore
1145+
if dcIgnored {
1146+
proc.stop()
1147+
if proc.agent.ID != uuid.Nil {
1148+
// Unlock while doing the delete operation.
1149+
api.mu.Unlock()
1150+
client := *api.subAgentClient.Load()
1151+
if err := client.Delete(ctx, proc.agent.ID); err != nil {
1152+
api.mu.Lock()
1153+
return xerrors.Errorf("delete subagent: %w", err)
1154+
}
1155+
api.mu.Lock()
1156+
}
1157+
// Reset agent and containerID to force config re-reading if ignore is toggled.
1158+
proc.agent = SubAgent{}
1159+
proc.containerID = ""
1160+
api.injectedSubAgentProcs[dc.WorkspaceFolder] = proc
1161+
api.ignoredDevcontainers[dc.WorkspaceFolder] = dcIgnored
1162+
return nil
1163+
}
1164+
}
1165+
11161166
if proc.containerID == container.ID && proc.ctx.Err() == nil {
11171167
// Same container and running, no need to reinject.
11181168
return nil
@@ -1131,7 +1181,8 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11311181
// Container ID changed or the subagent process is not running,
11321182
// stop the existing subagent context to replace it.
11331183
proc.stop()
1134-
} else {
1184+
}
1185+
if proc.agent.OperatingSystem == "" {
11351186
// Set SubAgent defaults.
11361187
proc.agent.OperatingSystem = "linux" // Assuming Linux for devcontainers.
11371188
}
@@ -1150,7 +1201,11 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11501201
ranSubAgent := false
11511202

11521203
// Clean up if injection fails.
1204+
var dcIgnored, setDCIgnored bool
11531205
defer func() {
1206+
if setDCIgnored {
1207+
api.ignoredDevcontainers[dc.WorkspaceFolder] = dcIgnored
1208+
}
11541209
if !ranSubAgent {
11551210
proc.stop()
11561211
if !api.closed {
@@ -1188,48 +1243,6 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11881243
proc.agent.Architecture = arch
11891244
}
11901245

1191-
agentBinaryPath, err := os.Executable()
1192-
if err != nil {
1193-
return xerrors.Errorf("get agent binary path: %w", err)
1194-
}
1195-
agentBinaryPath, err = filepath.EvalSymlinks(agentBinaryPath)
1196-
if err != nil {
1197-
return xerrors.Errorf("resolve agent binary path: %w", err)
1198-
}
1199-
1200-
// If we scripted this as a `/bin/sh` script, we could reduce these
1201-
// steps to one instruction, speeding up the injection process.
1202-
//
1203-
// Note: We use `path` instead of `filepath` here because we are
1204-
// working with Unix-style paths inside the container.
1205-
if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "mkdir", "-p", path.Dir(coderPathInsideContainer)); err != nil {
1206-
return xerrors.Errorf("create agent directory in container: %w", err)
1207-
}
1208-
1209-
if err := api.ccli.Copy(ctx, container.ID, agentBinaryPath, coderPathInsideContainer); err != nil {
1210-
return xerrors.Errorf("copy agent binary: %w", err)
1211-
}
1212-
1213-
logger.Info(ctx, "copied agent binary to container")
1214-
1215-
// Make sure the agent binary is executable so we can run it (the
1216-
// user doesn't matter since we're making it executable for all).
1217-
if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "chmod", "0755", path.Dir(coderPathInsideContainer), coderPathInsideContainer); err != nil {
1218-
return xerrors.Errorf("set agent binary executable: %w", err)
1219-
}
1220-
1221-
// Attempt to add CAP_NET_ADMIN to the binary to improve network
1222-
// performance (optional, allow to fail). See `bootstrap_linux.sh`.
1223-
// TODO(mafredri): Disable for now until we can figure out why this
1224-
// causes the following error on some images:
1225-
//
1226-
// Image: mcr.microsoft.com/devcontainers/base:ubuntu
1227-
// Error: /.coder-agent/coder: Operation not permitted
1228-
//
1229-
// if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "setcap", "cap_net_admin+ep", coderPathInsideContainer); err != nil {
1230-
// logger.Warn(ctx, "set CAP_NET_ADMIN on agent binary failed", slog.Error(err))
1231-
// }
1232-
12331246
subAgentConfig := proc.agent.CloneConfig(dc)
12341247
if proc.agent.ID == uuid.Nil || maybeRecreateSubAgent {
12351248
subAgentConfig.Architecture = arch
@@ -1269,6 +1282,13 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12691282
return err
12701283
}
12711284

1285+
// We only allow ignore to be set in the root customization layer to
1286+
// prevent weird interactions with devcontainer features.
1287+
dcIgnored, setDCIgnored = config.Configuration.Customizations.Coder.Ignore, true
1288+
if dcIgnored {
1289+
return nil
1290+
}
1291+
12721292
workspaceFolder = config.Workspace.WorkspaceFolder
12731293

12741294
// NOTE(DanielleMaywood):
@@ -1317,6 +1337,22 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
13171337
api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err))
13181338
}
13191339

1340+
if dcIgnored {
1341+
proc.stop()
1342+
if proc.agent.ID != uuid.Nil {
1343+
// If we stop the subagent, we also need to delete it.
1344+
client := *api.subAgentClient.Load()
1345+
if err := client.Delete(ctx, proc.agent.ID); err != nil {
1346+
return xerrors.Errorf("delete subagent: %w", err)
1347+
}
1348+
}
1349+
// Reset agent and containerID to force config re-reading if
1350+
// ignore is toggled.
1351+
proc.agent = SubAgent{}
1352+
proc.containerID = ""
1353+
return nil
1354+
}
1355+
13201356
displayApps := make([]codersdk.DisplayApp, 0, len(displayAppsMap))
13211357
for app, enabled := range displayAppsMap {
13221358
if enabled {
@@ -1349,6 +1385,48 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
13491385
subAgentConfig.Directory = workspaceFolder
13501386
}
13511387

1388+
agentBinaryPath, err := os.Executable()
1389+
if err != nil {
1390+
return xerrors.Errorf("get agent binary path: %w", err)
1391+
}
1392+
agentBinaryPath, err = filepath.EvalSymlinks(agentBinaryPath)
1393+
if err != nil {
1394+
return xerrors.Errorf("resolve agent binary path: %w", err)
1395+
}
1396+
1397+
// If we scripted this as a `/bin/sh` script, we could reduce these
1398+
// steps to one instruction, speeding up the injection process.
1399+
//
1400+
// Note: We use `path` instead of `filepath` here because we are
1401+
// working with Unix-style paths inside the container.
1402+
if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "mkdir", "-p", path.Dir(coderPathInsideContainer)); err != nil {
1403+
return xerrors.Errorf("create agent directory in container: %w", err)
1404+
}
1405+
1406+
if err := api.ccli.Copy(ctx, container.ID, agentBinaryPath, coderPathInsideContainer); err != nil {
1407+
return xerrors.Errorf("copy agent binary: %w", err)
1408+
}
1409+
1410+
logger.Info(ctx, "copied agent binary to container")
1411+
1412+
// Make sure the agent binary is executable so we can run it (the
1413+
// user doesn't matter since we're making it executable for all).
1414+
if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "chmod", "0755", path.Dir(coderPathInsideContainer), coderPathInsideContainer); err != nil {
1415+
return xerrors.Errorf("set agent binary executable: %w", err)
1416+
}
1417+
1418+
// Attempt to add CAP_NET_ADMIN to the binary to improve network
1419+
// performance (optional, allow to fail). See `bootstrap_linux.sh`.
1420+
// TODO(mafredri): Disable for now until we can figure out why this
1421+
// causes the following error on some images:
1422+
//
1423+
// Image: mcr.microsoft.com/devcontainers/base:ubuntu
1424+
// Error: /.coder-agent/coder: Operation not permitted
1425+
//
1426+
// if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "setcap", "cap_net_admin+ep", coderPathInsideContainer); err != nil {
1427+
// logger.Warn(ctx, "set CAP_NET_ADMIN on agent binary failed", slog.Error(err))
1428+
// }
1429+
13521430
deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig)
13531431
if deleteSubAgent {
13541432
logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID))

0 commit comments

Comments
 (0)
0