8000 rewrite · coder/coder@700a00c · GitHub
[go: up one dir, main page]
Skip to content

Commit 700a00c

Browse files
mafredrimafredri
committed
rewrite
1 parent 5bac6ef commit 700a00c

File tree

2 files changed

+42
-44
lines changed

2 files changed

+42
-44
lines changed

‎agent/agentcontainers/api.go

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,7 @@ func (api *API) markDevcontainerDirty(configPath string, modifiedAt time.Time) {
988988
logger.Info(api.ctx, "marking devcontainer as dirty")
989989
dc.Dirty = true
990990
}
991-
if api.ignoredDevcontainers[dc.WorkspaceFolder] {
991+
if _, ok := api.ignoredDevcontainers[dc.WorkspaceFolder]; ok {
992992
logger.Debug(api.ctx, "clearing devcontainer ignored state")
993993
delete(api.ignoredDevcontainers, dc.WorkspaceFolder) // Allow re-reading config.
994994
}
@@ -997,37 +997,6 @@ func (api *API) markDevcontainerDirty(configPath string, modifiedAt time.Time) {
997997
}
998998
}
999999

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-
10311000
// cleanupSubAgents removes subagents that are no longer managed by
10321001
// this agent. This is usually only run at startup to ensure a clean
10331002
// slate. This method has an internal timeout to prevent blocking
@@ -1118,11 +1087,24 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11181087
return xerrors.Errorf("read devcontainer config: %w", err)
11191088
}
11201089

1121-
if config.Configuration.Customizations.Coder.Ignore {
1122-
api.mu.Unlock()
1123-
proc = api.markDevcontainerIgnored(ctx, dc, proc)
1124-
api.mu.Lock()
1090+
ignore := config.Configuration.Customizations.Coder.Ignore
1091+
if ignore {
1092+
proc.stop()
1093+
if proc.agent.ID != uuid.Nil {
1094+
// Unlock while doing the delete operation.
1095+
api.mu.Unlock()
1096+
client := *api.subAgentClient.Load()
1097+
if err := client.Delete(ctx, proc.agent.ID); err != nil {
1098+
api.mu.Lock()
1099+
return xerrors.Errorf("delete subagent: %w", err)
1100+
}
1101+
api.mu.Lock()
1102+
}
1103+
// Reset agent and containerID to force config re-reading if ignore is toggled.
1104+
proc.agent = SubAgent{}
1105+
proc.containerID = ""
11251106
api.injectedSubAgentProcs[dc.WorkspaceFolder] = proc
1107+
api.ignoredDevcontainers[dc.WorkspaceFolder] = ignore
11261108
return nil
11271109
}
11281110
}
@@ -1165,7 +1147,11 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11651147
ranSubAgent := false
11661148

11671149
// Clean up if injection fails.
1150+
var ignored, setIgnored bool
11681151
defer func() {
1152+
if setIgnored {
1153+
api.ignoredDevcontainers[dc.WorkspaceFolder] = ignored
1154+
}
11691155
if !ranSubAgent {
11701156
proc.stop()
11711157
if !api.closed {
@@ -1219,7 +1205,6 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12191205
}
12201206

12211207
var (
1222-
ignore bool
12231208
appsWithPossibleDuplicates []SubAgentApp
12241209
workspaceFolder = DevcontainerDefaultContainerWorkspaceFolder
12251210
)
@@ -1243,13 +1228,15 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12431228
return err
12441229
}
12451230

1246-
ignore = config.Configuration.Customizations.Coder.Ignore
1247-
workspaceFolder = config.Workspace.WorkspaceFolder
1248-
1249-
if ignore {
1231+
// We only allow ignore to be set in the root customization layer to
1232+
// prevent weird interactions with devcontainer features.
1233+
ignored, setIgnored = config.Configuration.Customizations.Coder.Ignore, true
1234+
if ignored {
12501235
return nil
12511236
}
12521237

1238+
workspaceFolder = config.Workspace.WorkspaceFolder
1239+
12531240
// NOTE(DanielleMaywood):
12541241
// We only want to take an agent name specified in the root customization layer.
12551242
// This restricts the ability for a feature to specify the agent name. We may revisit
@@ -1295,8 +1282,19 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12951282
api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err))
12961283
}
12971284

1298-
if ignore {
1299-
proc = api.markDevcontainerIgnored(ctx, dc, proc)
1285+
if ignored {
1286+
proc.stop()
1287+
if proc.agent.ID != uuid.Nil {
1288+
// If we stop the subagent, we also need to delete it.
1289+
client := *api.subAgentClient.Load()
1290+
if err := client.Delete(ctx, proc.agent.ID); err != nil {
1291+
return xerrors.Errorf("delete subagent: %w", err)
1292+
}
1293+
}
1294+
// Reset agent and containerID to force config re-reading if
1295+
// ignore is toggled.
1296+
proc.agent = SubAgent{}
1297+
proc.containerID = ""
13001298
return nil
13011299
}
13021300

‎agent/agentcontainers/api_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2220,7 +2220,7 @@ func TestAPI(t *testing.T) {
22202220

22212221
assert.Empty(t, response.Devcontainers, "devcontainer should be filtered out when ignore=true again")
22222222
assert.Len(t, response.Containers, 1, "regular container should still be listed")
2223-
assert.Len(t, fakeSAC.deleted, 1, "sub agent should be deleted when ignore=true")
2223+
require.Len(t, fakeSAC.deleted, 1, "sub agent should be deleted when ignore=true")
22242224
assert.Equal(t, createdAgentID, fakeSAC.deleted[0], "the same sub agent that was created should be deleted")
22252225
})
22262226
}

0 commit comments

Comments
 (0)
0