8000 fix(agent/agentcontainers): reduce need to recreate sub agents · coder/coder@d496771 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit d496771

Browse files
mafredrimafredri
committed
fix(agent/agentcontainers): reduce need to recreate sub agents
Updates #18332
1 parent c033618 commit d496771

File tree

2 files changed

+59
-34
lines changed

2 files changed

+59
-34
lines changed

‎agent/agentcontainers/api.go

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
977977
)
978978

979979
// Check if subagent already exists for this devcontainer.
980-
recreateSubAgent := false
980+
maybeRecreateSubAgent := false
981981
proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder]
982982
if injected {
983983
if proc.containerID == container.ID && proc.ctx.Err() == nil {
@@ -992,12 +992,15 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
992992
logger.Debug(ctx, "container ID changed, injecting subagent into new container",
993993
slog.F("old_container_id", proc.containerID),
994994
)
995-
recreateSubAgent = true
995+
maybeRecreateSubAgent = true
996996
}
997997

998998
// Container ID changed or the subagent process is not running,
999999
// stop the existing subagent context to replace it.
10001000
proc.stop()
1001+
} else {
1002+
// Set SubAgent defaults.
1003+
proc.agent.OperatingSystem = "linux" // Assuming Linux for devcontainers.
10011004
}
10021005

10031006
// Prepare the subAgentProcess to be used when running the subagent.
@@ -1090,36 +1093,33 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
10901093
// logger.Warn(ctx, "set CAP_NET_ADMIN on agent binary failed", slog.Error(err))
10911094
// }
10921095

1096+
updatedSubAgent := proc.agent
1097+
10931098
// Detect workspace folder by execu 10000 ting `pwd` in the container.
10941099
// NOTE(mafredri): This is a quick and dirty way to detect the
10951100
// workspace folder inside the container. In the future we will
10961101
// rely more on `devcontainer read-configuration`.
1097-
var pwdBuf bytes.Buffer
1098-
err = api.dccli.Exec(ctx, dc.WorkspaceFolder, dc.ConfigPath, "pwd", []string{},
1099-
WithExecOutput(&pwdBuf, io.Discard),
1100-
WithExecContainerID(container.ID),
1101-
)
1102-
if err != nil {
1103-
return xerrors.Errorf("check workspace folder in container: %w", err)
1104-
}
1105-
directory := strings.TrimSpace(pwdBuf.String())
1106-
if directory == "" {
1107-
logger.Warn(ctx, "detected workspace folder is empty, using default workspace folder",
1108-
slog.F("default_workspace_folder", DevcontainerDefaultContainerWorkspaceFolder),
1102+
if proc.agent.Directory == "" || maybeRecreateSubAgent {
1103+
var pwdBuf bytes.Buffer
1104+
err = api.dccli.Exec(ctx, dc.WorkspaceFolder, dc.ConfigPath, "pwd", []string{},
1105+
WithExecOutput(&pwdBuf, io.Discard),
1106+
WithExecContainerID(container.ID),
11091107
)
1110-
directory = DevcontainerDefaultContainerWorkspaceFolder
1111-
}
1112-
1113-
if proc.agent.ID != uuid.Nil && recreateSubAgent {
1114-
logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID))
1115-
client := *api.subAgentClient.Load()
1116-
err = client.Delete(ctx, proc.agent.ID)
11171108
if err != nil {
1118-
return xerrors.Errorf("delete existing subagent failed: %w", err)
1109+
return xerrors.Errorf("check workspace folder in container: %w", err)
1110+
}
1111+
directory := strings.TrimSpace(pwdBuf.String())
1112+
if directory == "" {
1113+
logger.Warn(ctx, "detected workspace folder is empty, using default workspace folder",
1114+
slog.F("default_workspace_folder", DevcontainerDefaultContainerWorkspaceFolder),
1115+
)
1116+
directory = DevcontainerDefaultContainerWorkspaceFolder
11191117
}
1120-
proc.agent = SubAgent{}
1118+
1119+
updatedSubAgent.Directory = directory
11211120
}
1122-
if proc.agent.ID == uuid.Nil {
1121+
1122+
if maybeRecreateSubAgent {
11231123
displayAppsMap := map[codersdk.DisplayApp]bool{
11241124
// NOTE(DanielleMaywood):
11251125
// We use the same defaults here as set in terraform-provider-coder.
@@ -1138,6 +1138,9 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11381138

11391139
for _, customization := range coderCustomization {
11401140
for app, enabled := range customization.DisplayApps {
1141+
if _, ok := displayAppsMap[app]; !ok {
1142+
continue // Ignore unknown display apps.
1143+
}
11411144
displayAppsMap[app] = enabled
11421145
}
11431146
}
@@ -1149,24 +1152,37 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11491152
displayApps = append(displayApps, app)
11501153
}
11511154
}
1155+
slices.Sort(displayApps)
1156+
1157+
updatedSubAgent.Name = dc.Name
1158+
updatedSubAgent.DisplayApps = displayApps
1159+
}
1160+
1161+
recreateSubAgent := maybeRecreateSubAgent && !proc.agent.Equal(updatedSubAgent)
1162+
if recreateSubAgent {
1163+
logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID))
1164+
client := *api.subAgentClient.Load()
1165+
err = client.Delete(ctx, proc.agent.ID)
1166+
if err != nil {
1167+
return xerrors.Errorf("delete existing subagent failed: %w", err)
1168+
}
1169+
proc.agent = SubAgent{} // In case of error, make sure this is cleared.
1170+
1171+
// Reset fields that are not part of the request.
1172+
updatedSubAgent.ID = uuid.Nil
1173+
updatedSubAgent.AuthToken = uuid.Nil
11521174

11531175
logger.Debug(ctx, "creating new subagent",
1154-
slog.F("directory", directory),
1155-
slog.F("display_apps", displayApps),
1176+
slog.F("directory", updatedSubAgent.Directory),
1177+
slog.F("display_apps", updatedSubAgent.DisplayApps),
11561178
)
11571179

11581180
// Create new subagent record in the database to receive the auth token.
1159-
client := *api.subAgentClient.Load()
1160-
proc.agent, err = client.Create(ctx, SubAgent{
1161-
Name: dc.Name,
1162-
Directory: directory,
1163-
OperatingSystem: "linux", // Assuming Linux for devcontainers.
1164-
Architecture: arch,
1165-
DisplayApps: displayApps,
1166-
})
1181+
newSubAgent, err := client.Create(ctx, updatedSubAgent)
11671182
if err != nil {
11681183
return xerrors.Errorf("create subagent failed: %w", err)
11691184
}
1185+
proc.agent = newSubAgent
11701186

11711187
logger.Info(ctx, "created new subagent", slog.F("agent_id", proc.agent.ID))
11721188
}

‎agent/agentcontainers/subagent.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package agentcontainers
22

33
import (
44
"context"
5+
"slices"
56

67
"github.com/google/uuid"
78
"golang.org/x/xerrors"
@@ -23,6 +24,14 @@ type SubAgent struct {
2324
DisplayApps []codersdk.DisplayApp
2425
}
2526

27+
func (s SubAgent) Equal(other SubAgent) bool {
28+
return s.Name == other.Name &&
29+
s.Directory == other.Directory &&
30+
s.Architecture == other.Architecture &&
31+
s.OperatingSystem == other.OperatingSystem &&
32+
slices.Equal(s.DisplayApps, other.DisplayApps)
33+
}
34+
2635
// SubAgentClient is an interface for managing sub agents and allows
2736
// changing the implementation without having to deal with the
2837
// agentproto package directly.

0 commit comments

Comments
 (0)
0