Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix(coderd): mark sub agent deletion via boolean instead of delete#18411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mafredri merged 7 commits intomainfrommafredri/fix-subagent-deletion
Jun 19, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletionscoderd/agentapi/subagent_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -875,14 +875,9 @@ func TestSubAgentAPI(t *testing.T) {
require.NoError(t, err)
})

t.Run("DeletesWorkspaceApps", func(t *testing.T) {
t.Run("DeleteRetainsWorkspaceApps", func(t *testing.T) {
t.Parallel()

// Skip test on in-memory database since CASCADE DELETE is not implemented
if !dbtestutil.WillUsePostgres() {
t.Skip("CASCADE DELETE behavior requires PostgreSQL")
}

log := testutil.Logger(t)
ctx := testutil.Context(t, testutil.WaitShort)
clock := quartz.NewMock(t)
Expand DownExpand Up@@ -931,11 +926,11 @@ func TestSubAgentAPI(t *testing.T) {
_, err = api.Database.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), subAgentID) //nolint:gocritic // this is a test.
require.ErrorIs(t, err, sql.ErrNoRows)

// And: The apps arealso deleted (duetoCASCADE DELETE)
//Use raw database since authorization layer requires agenttoexist
// And: The apps are*retained*toavoid causing issues
//where the resources are expectedtobe present.
appsAfterDeletion, err := db.GetWorkspaceAppsByAgentID(ctx, subAgentID)
require.NoError(t, err)
require.Empty(t, appsAfterDeletion)
require.NotEmpty(t, appsAfterDeletion)
})
})

Expand Down
20 changes: 20 additions & 0 deletionscoderd/database/dbfake/dbfake.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"encoding/json"
"errors"
"testing"
"time"

Expand DownExpand Up@@ -243,6 +244,25 @@ func (b WorkspaceBuildBuilder) Do() WorkspaceResponse {
require.NoError(b.t, err)
}

agents, err := b.db.GetWorkspaceAgentsByWorkspaceAndBuildNumber(ownerCtx, database.GetWorkspaceAgentsByWorkspaceAndBuildNumberParams{
WorkspaceID: resp.Workspace.ID,
BuildNumber: resp.Build.BuildNumber,
})
if !errors.Is(err, sql.ErrNoRows) {
require.NoError(b.t, err, "get workspace agents")
// Insert deleted subagent test antagonists for the workspace build.
// See also `dbgen.WorkspaceAgent()`.
for _, agent := range agents {
subAgent := dbgen.WorkspaceSubAgent(b.t, b.db, agent, database.WorkspaceAgent{
TroubleshootingURL: "I AM A TEST ANTAGONIST AND I AM HERE TO MESS UP YOUR TESTS. IF YOU SEE ME, SOMETHING IS WRONG AND SUB AGENT DELETION MAY NOT BE HANDLED CORRECTLY IN A QUERY.",
})
err = b.db.DeleteWorkspaceSubAgentByID(ownerCtx, subAgent.ID)
require.NoError(b.t, err, "delete workspace agent subagent antagonist")

b.t.Logf("inserted deleted subagent antagonist %s (%v) for workspace agent %s (%v)", subAgent.Name, subAgent.ID, agent.Name, agent.ID)
}
}

return resp
}

Expand Down
43 changes: 42 additions & 1 deletioncoderd/database/dbgen/dbgen.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -209,7 +209,7 @@ func WorkspaceAgent(t testing.TB, db database.Store, orig database.WorkspaceAgen
},
ConnectionTimeoutSeconds: takeFirst(orig.ConnectionTimeoutSeconds, 3600),
TroubleshootingURL: takeFirst(orig.TroubleshootingURL, "https://example.com"),
MOTDFile: takeFirst(orig.TroubleshootingURL, ""),
MOTDFile: takeFirst(orig.MOTDFile, ""),
DisplayApps: append([]database.DisplayApp{}, orig.DisplayApps...),
DisplayOrder: takeFirst(orig.DisplayOrder, 1),
APIKeyScope: takeFirst(orig.APIKeyScope, database.AgentKeyScopeEnumAll),
Expand All@@ -226,9 +226,50 @@ func WorkspaceAgent(t testing.TB, db database.Store, orig database.WorkspaceAgen
})
require.NoError(t, err, "update workspace agent first connected at")
}

if orig.ParentID.UUID == uuid.Nil {
// Add a test antagonist. For every agent we add a deleted sub agent
// to discover cases where deletion should be handled.
// See also `(dbfake.WorkspaceBuildBuilder).Do()`.
subAgt, err := db.InsertWorkspaceAgent(genCtx, database.InsertWorkspaceAgentParams{
ID: uuid.New(),
ParentID: uuid.NullUUID{UUID: agt.ID, Valid: true},
CreatedAt: dbtime.Now(),
UpdatedAt: dbtime.Now(),
Name: testutil.GetRandomName(t),
ResourceID: agt.ResourceID,
AuthToken: uuid.New(),
AuthInstanceID: sql.NullString{},
Architecture: agt.Architecture,
EnvironmentVariables: pqtype.NullRawMessage{},
OperatingSystem: agt.OperatingSystem,
Directory: agt.Directory,
InstanceMetadata: pqtype.NullRawMessage{},
ResourceMetadata: pqtype.NullRawMessage{},
ConnectionTimeoutSeconds: agt.ConnectionTimeoutSeconds,
TroubleshootingURL: "I AM A TEST ANTAGONIST AND I AM HERE TO MESS UP YOUR TESTS. IF YOU SEE ME, SOMETHING IS WRONG AND SUB AGENT DELETION MAY NOT BE HANDLED CORRECTLY IN A QUERY.",
MOTDFile: "",
DisplayApps: nil,
DisplayOrder: agt.DisplayOrder,
APIKeyScope: agt.APIKeyScope,
})
require.NoError(t, err, "insert workspace agent subagent antagonist")
err = db.DeleteWorkspaceSubAgentByID(genCtx, subAgt.ID)
require.NoError(t, err, "delete workspace agent subagent antagonist")

t.Logf("inserted deleted subagent antagonist %s (%v) for workspace agent %s (%v)", subAgt.Name, subAgt.ID, agt.Name, agt.ID)
}

return agt
}

func WorkspaceSubAgent(t testing.TB, db database.Store, parentAgent database.WorkspaceAgent, orig database.WorkspaceAgent) database.WorkspaceAgent {
orig.ParentID = uuid.NullUUID{UUID: parentAgent.ID, Valid: true}
orig.ResourceID = parentAgent.ResourceID
subAgt := WorkspaceAgent(t, db, orig)
return subAgt
}

func WorkspaceAgentScript(t testing.TB, db database.Store, orig database.WorkspaceAgentScript) database.WorkspaceAgentScript {
scripts, err := db.InsertWorkspaceAgentScripts(genCtx, database.InsertWorkspaceAgentScriptsParams{
WorkspaceAgentID: takeFirst(orig.WorkspaceAgentID, uuid.New()),
Expand Down
22 changes: 16 additions & 6 deletionscoderd/database/dbmem/dbmem.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -792,7 +792,7 @@ func (q *FakeQuerier) getWorkspaceAgentByIDNoLock(_ context.Context, id uuid.UUI
// The schema sorts this by created at, so we iterate the array backwards.
for i := len(q.workspaceAgents) - 1; i >= 0; i-- {
agent := q.workspaceAgents[i]
if agent.ID == id {
if!agent.Deleted &&agent.ID == id {
return agent, nil
}
}
Expand All@@ -802,6 +802,9 @@ func (q *FakeQuerier) getWorkspaceAgentByIDNoLock(_ context.Context, id uuid.UUI
func (q *FakeQuerier) getWorkspaceAgentsByResourceIDsNoLock(_ context.Context, resourceIDs []uuid.UUID) ([]database.WorkspaceAgent, error) {
workspaceAgents := make([]database.WorkspaceAgent, 0)
for _, agent := range q.workspaceAgents {
if agent.Deleted {
continue
}
for _, resourceID := range resourceIDs {
if agent.ResourceID != resourceID {
continue
Expand DownExpand Up@@ -2554,13 +2557,13 @@ func (q *FakeQuerier) DeleteWorkspaceAgentPortSharesByTemplate(_ context.Context
return nil
}

func (q *FakeQuerier) DeleteWorkspaceSubAgentByID(ctx context.Context, id uuid.UUID) error {
func (q *FakeQuerier) DeleteWorkspaceSubAgentByID(_ context.Context, id uuid.UUID) error {
q.mutex.Lock()
defer q.mutex.Unlock()

for i, agent := range q.workspaceAgents {
if agent.ID == id && agent.ParentID.Valid {
q.workspaceAgents =slices.Delete(q.workspaceAgents, i, i+1)
q.workspaceAgents[i].Deleted =true
return nil
}
}
Expand DownExpand Up@@ -7077,6 +7080,10 @@ func (q *FakeQuerier) GetWorkspaceAgentAndLatestBuildByAuthToken(_ context.Conte
latestBuildNumber := make(map[uuid.UUID]int32)

for _, agt := range q.workspaceAgents {
if agt.Deleted {
continue
}

// get the related workspace and user
for _, res := range q.workspaceResources {
if agt.ResourceID != res.ID {
Expand DownExpand Up@@ -7146,7 +7153,7 @@ func (q *FakeQuerier) GetWorkspaceAgentByInstanceID(_ context.Context, instanceI
// The schema sorts this by created at, so we iterate the array backwards.
for i := len(q.workspaceAgents) - 1; i >= 0; i-- {
agent := q.workspaceAgents[i]
if agent.AuthInstanceID.Valid && agent.AuthInstanceID.String == instanceID {
if!agent.Deleted &&agent.AuthInstanceID.Valid && agent.AuthInstanceID.String == instanceID {
return agent, nil
}
}
Expand DownExpand Up@@ -7706,13 +7713,13 @@ func (q *FakeQuerier) GetWorkspaceAgentUsageStatsAndLabels(_ context.Context, cr
return stats, nil
}

func (q *FakeQuerier) GetWorkspaceAgentsByParentID(ctx context.Context, parentID uuid.UUID) ([]database.WorkspaceAgent, error) {
func (q *FakeQuerier) GetWorkspaceAgentsByParentID(_ context.Context, parentID uuid.UUID) ([]database.WorkspaceAgent, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()

workspaceAgents := make([]database.WorkspaceAgent, 0)
for _, agent := range q.workspaceAgents {
if !agent.ParentID.Valid || agent.ParentID.UUID != parentID {
if !agent.ParentID.Valid || agent.ParentID.UUID != parentID|| agent.Deleted{
continue
}

Expand DownExpand Up@@ -7759,6 +7766,9 @@ func (q *FakeQuerier) GetWorkspaceAgentsCreatedAfter(_ context.Context, after ti

workspaceAgents := make([]database.WorkspaceAgent, 0)
for _, agent := range q.workspaceAgents {
if agent.Deleted {
continue
}
if agent.CreatedAt.After(after) {
workspaceAgents = append(workspaceAgents, agent)
}
Expand Down
8 changes: 6 additions & 2 deletionscoderd/database/dump.sql
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
-- Restore prebuilds, previously modified in 000323_workspace_latest_builds_optimization.up.sql.
DROP VIEW workspace_prebuilds;

CREATE VIEW workspace_prebuilds AS
WITH all_prebuilds AS (
SELECT w.id,
w.name,
w.template_id,
w.created_at
FROM workspaces w
WHERE (w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid)
), workspaces_with_latest_presets AS (
SELECT DISTINCT ON (workspace_builds.workspace_id) workspace_builds.workspace_id,
workspace_builds.template_version_preset_id
FROM workspace_builds
WHERE (workspace_builds.template_version_preset_id IS NOT NULL)
ORDER BY workspace_builds.workspace_id, workspace_builds.build_number DESC
), workspaces_with_agents_status AS (
SELECT w.id AS workspace_id,
bool_and((wa.lifecycle_state = 'ready'::workspace_agent_lifecycle_state)) AS ready
FROM (((workspaces w
JOIN workspace_latest_builds wlb ON ((wlb.workspace_id = w.id)))
JOIN workspace_resources wr ON ((wr.job_id = wlb.job_id)))
JOIN workspace_agents wa ON ((wa.resource_id = wr.id)))
WHERE (w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid)
GROUP BY w.id
), current_presets AS (
SELECT w.id AS prebuild_id,
wlp.template_version_preset_id
FROM (workspaces w
JOIN workspaces_with_latest_presets wlp ON ((wlp.workspace_id = w.id)))
WHERE (w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid)
)
SELECT p.id,
p.name,
p.template_id,
p.created_at,
COALESCE(a.ready, false) AS ready,
cp.template_version_preset_id AS current_preset_id
FROM ((all_prebuilds p
LEFT JOIN workspaces_with_agents_status a ON ((a.workspace_id = p.id)))
JOIN current_presets cp ON ((cp.prebuild_id = p.id)));

-- Restore trigger without deleted check.
DROP TRIGGER IF EXISTS workspace_agent_name_unique_trigger ON workspace_agents;
DROP FUNCTION IF EXISTS check_workspace_agent_name_unique();

CREATE OR REPLACE FUNCTION check_workspace_agent_name_unique()
RETURNS TRIGGER AS $$
DECLARE
workspace_build_id uuid;
agents_with_name int;
BEGIN
-- Find the workspace build the workspace agent is being inserted into.
SELECT workspace_builds.id INTO workspace_build_id
FROM workspace_resources
JOIN workspace_builds ON workspace_builds.job_id = workspace_resources.job_id
WHERE workspace_resources.id = NEW.resource_id;

-- If the agent doesn't have a workspace build, we'll allow the insert.
IF workspace_build_id IS NULL THEN
RETURN NEW;
END IF;

-- Count how many agents in this workspace build already have the given agent name.
SELECT COUNT(*) INTO agents_with_name
FROM workspace_agents
JOIN workspace_resources ON workspace_resources.id = workspace_agents.resource_id
JOIN workspace_builds ON workspace_builds.job_id = workspace_resources.job_id
WHERE workspace_builds.id = workspace_build_id
AND workspace_agents.name = NEW.name
AND workspace_agents.id != NEW.id;

-- If there's already an agent with this name, raise an error
IF agents_with_name > 0 THEN
RAISE EXCEPTION 'workspace agent name "%" already exists in this workspace build', NEW.name
USING ERRCODE = 'unique_violation';
END IF;

RETURN NEW;
END;
$$ LANGUAGE plpgsql;

CREATE TRIGGER workspace_agent_name_unique_trigger
BEFORE INSERT OR UPDATE OF name, resource_id ON workspace_agents
FOR EACH ROW
EXECUTE FUNCTION check_workspace_agent_name_unique();

COMMENT ON TRIGGER workspace_agent_name_unique_trigger ON workspace_agents IS
'Use a trigger instead of a unique constraint because existing data may violate
the uniqueness requirement. A trigger allows us to enforce uniqueness going
forward without requiring a migration to clean up historical data.';


ALTER TABLE workspace_agents
DROP COLUMN deleted;
Loading
Loading
[8]ページ先頭
©2009-2025 Movatter.jp