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

Commita6b0eae

Browse files
authored
refactor(coderd): drop sidebar app constraint and simplify provisionerdserver for tasks (#20591)
Updatescoder/internal#973Updatescoder/internal#974
1 parent1961252 commita6b0eae

File tree

6 files changed

+93
-91
lines changed

6 files changed

+93
-91
lines changed

‎coderd/database/check_constraint.go‎

Lines changed: 10 additions & 11 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/dump.sql‎

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- WARNING: Restoring this constraint after running a newer version of coderd
2+
-- and using tasks is bound to break this constraint.
3+
ALTERTABLE workspace_builds
4+
ADDCONSTRAINT workspace_builds_ai_task_sidebar_app_id_requiredCHECK (((((has_ai_task ISNULL)OR (has_ai_task= false))AND (ai_task_sidebar_app_id ISNULL))OR ((has_ai_task= true)AND (ai_task_sidebar_app_idIS NOT NULL))));
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- We no longer need to enforce this constraint as tasks have their own data
2+
-- model.
3+
ALTERTABLE workspace_builds
4+
DROPCONSTRAINT workspace_builds_ai_task_sidebar_app_id_required;

‎coderd/provisionerdserver/provisionerdserver.go‎

Lines changed: 24 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -2006,10 +2006,12 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro
20062006
}
20072007
}
20082008

2009-
vartaskAppID uuid.NullUUID
2010-
vartaskAgentID uuid.NullUUID
2011-
varhasAITaskbool
2012-
varwarnUnknownTaskAppIDbool
2009+
var (
2010+
hasAITaskbool
2011+
unknownAppIDstring
2012+
taskAppID uuid.NullUUID
2013+
taskAgentID uuid.NullUUID
2014+
)
20132015
iftasks:=jobType.WorkspaceBuild.GetAiTasks();len(tasks)>0 {
20142016
hasAITask=true
20152017
task:=tasks[0]
@@ -2026,59 +2028,29 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro
20262028
}
20272029

20282030
if!slices.Contains(appIDs,appID) {
2029-
warnUnknownTaskAppID=true
2030-
}
2031-
2032-
id,err:=uuid.Parse(appID)
2033-
iferr!=nil {
2034-
returnxerrors.Errorf("parse app id: %w",err)
2035-
}
2036-
2037-
taskAppID= uuid.NullUUID{UUID:id,Valid:true}
2038-
2039-
agentID,ok:=agentIDByAppID[appID]
2040-
taskAgentID= uuid.NullUUID{UUID:agentID,Valid:ok}
2041-
}
2042-
2043-
// This is a hacky workaround for the issue with tasks 'disappearing' on stop:
2044-
// reuse has_ai_task and sidebar_app_id from the previous build.
2045-
// This workaround should be removed as soon as possible.
2046-
ifworkspaceBuild.Transition==database.WorkspaceTransitionStop&&workspaceBuild.BuildNumber>1 {
2047-
ifprevBuild,err:=s.Database.GetWorkspaceBuildByWorkspaceIDAndBuildNumber(ctx, database.GetWorkspaceBuildByWorkspaceIDAndBuildNumberParams{
2048-
WorkspaceID:workspaceBuild.WorkspaceID,
2049-
BuildNumber:workspaceBuild.BuildNumber-1,
2050-
});err==nil {
2051-
hasAITask=prevBuild.HasAITask.Bool
2052-
taskAppID=prevBuild.AITaskSidebarAppID
2053-
warnUnknownTaskAppID=false
2054-
s.Logger.Debug(ctx,"task workaround: reused has_ai_task and app_id from previous build to keep track of task",
2055-
slog.F("job_id",job.ID.String()),
2056-
slog.F("build_number",prevBuild.BuildNumber),
2057-
slog.F("workspace_id",workspace.ID),
2058-
slog.F("workspace_build_id",workspaceBuild.ID),
2059-
slog.F("transition",string(workspaceBuild.Transition)),
2060-
slog.F("sidebar_app_id",taskAppID.UUID),
2061-
slog.F("has_ai_task",hasAITask),
2062-
)
2031+
unknownAppID=appID
2032+
hasAITask=false
20632033
}else {
2064-
s.Logger.Error(ctx,"task workaround: tracking via has_ai_task and app_id from previous build failed",
2065-
slog.Error(err),
2066-
slog.F("job_id",job.ID.String()),
2067-
slog.F("workspace_id",workspace.ID),
2068-
slog.F("workspace_build_id",workspaceBuild.ID),
2069-
slog.F("transition",string(workspaceBuild.Transition)),
2070-
)
2034+
// Only parse for valid app and agent to avoid fk violation.
2035+
id,err:=uuid.Parse(appID)
2036+
iferr!=nil {
2037+
returnxerrors.Errorf("parse app id: %w",err)
2038+
}
2039+
taskAppID= uuid.NullUUID{UUID:id,Valid:true}
2040+
2041+
agentID,ok:=agentIDByAppID[appID]
2042+
taskAgentID= uuid.NullUUID{UUID:agentID,Valid:ok}
20712043
}
20722044
}
20732045

2074-
ifwarnUnknownTaskAppID {
2046+
ifunknownAppID!=""&&workspaceBuild.Transition==database.WorkspaceTransitionStart {
20752047
// Ref: https://github.com/coder/coder/issues/18776
20762048
// This can happen for a number of reasons:
20772049
// 1. Misconfigured template
20782050
// 2. Count=0 on the agent due to stop transition, meaning the associated coder_app was not inserted.
20792051
// Failing the build at this point is not ideal, so log a warning instead.
20802052
s.Logger.Warn(ctx,"unknown ai_task_app_id",
2081-
slog.F("ai_task_app_id",taskAppID.UUID.String()),
2053+
slog.F("ai_task_app_id",unknownAppID),
20822054
slog.F("job_id",job.ID.String()),
20832055
slog.F("workspace_id",workspace.ID),
20842056
slog.F("workspace_build_id",workspaceBuild.ID),
@@ -2105,9 +2077,6 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro
21052077
slog.F("transition",string(workspaceBuild.Transition)),
21062078
)
21072079
}
2108-
// Important: reset hasAITask and sidebarAppID so that we don't run into a fk constraint violation.
2109-
hasAITask=false
2110-
taskAppID= uuid.NullUUID{}
21112080
}
21122081

21132082
ifhasAITask&&workspaceBuild.Transition==database.WorkspaceTransitionStart {
@@ -2124,14 +2093,6 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro
21242093
}
21252094
}
21262095

2127-
hasExternalAgent:=false
2128-
for_,resource:=rangejobType.WorkspaceBuild.Resources {
2129-
ifresource.Type=="coder_external_agent" {
2130-
hasExternalAgent=true
2131-
break
2132-
}
2133-
}
2134-
21352096
iftask,err:=db.GetTaskByWorkspaceID(ctx,workspace.ID);err==nil {
21362097
// Irrespective of whether the agent or sidebar app is present,
21372098
// perform the upsert to ensure a link between the task and
@@ -2153,20 +2114,21 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro
21532114
returnxerrors.Errorf("get task by workspace id: %w",err)
21542115
}
21552116

2156-
// Regardless of whether there is an AI task or not, update the field to indicate one way or the other since it
2157-
// always defaults to nil. ONLY if has_ai_task=true MUST ai_task_sidebar_app_id be set.
2117+
_,hasExternalAgent:=slice.Find(jobType.WorkspaceBuild.Resources,func(resource*sdkproto.Resource)bool {
2118+
returnresource.Type=="coder_external_agent"
2119+
})
21582120
iferr:=db.UpdateWorkspaceBuildFlagsByID(ctx, database.UpdateWorkspaceBuildFlagsByIDParams{
21592121
ID:workspaceBuild.ID,
21602122
HasAITask: sql.NullBool{
21612123
Bool:hasAITask,
21622124
Valid:true,
21632125
},
2126+
SidebarAppID:taskAppID,// SidebarAppID is not required, but kept for API backwards compatibility.
21642127
HasExternalAgent: sql.NullBool{
21652128
Bool:hasExternalAgent,
21662129
Valid:true,
21672130
},
2168-
SidebarAppID:taskAppID,
2169-
UpdatedAt:now,
2131+
UpdatedAt:now,
21702132
});err!=nil {
21712133
returnxerrors.Errorf("update workspace build ai tasks and external agent flag: %w",err)
21722134
}

‎coderd/provisionerdserver/provisionerdserver_test.go‎

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2864,11 +2864,12 @@ func TestCompleteJob(t *testing.T) {
28642864
input*proto.CompletedJob_WorkspaceBuild
28652865
isTaskbool
28662866
expectTaskStatus database.TaskStatus
2867+
expectAppID uuid.NullUUID
28672868
expectHasAiTaskbool
28682869
expectUsageEventbool
28692870
}
28702871

2871-
sidebarAppID:=uuid.NewString()
2872+
sidebarAppID:=uuid.New()
28722873
for_,tc:=range []testcase{
28732874
{
28742875
name:"has_ai_task is false by default",
@@ -2883,12 +2884,45 @@ func TestCompleteJob(t *testing.T) {
28832884
{
28842885
name:"has_ai_task is set to true",
28852886
transition:database.WorkspaceTransitionStart,
2887+
input:&proto.CompletedJob_WorkspaceBuild{
2888+
AiTasks: []*sdkproto.AITask{
2889+
{
2890+
Id:uuid.NewString(),
2891+
AppId:sidebarAppID.String(),
2892+
},
2893+
},
2894+
Resources: []*sdkproto.Resource{
2895+
{
2896+
Agents: []*sdkproto.Agent{
2897+
{
2898+
Id:uuid.NewString(),
2899+
Name:"a",
2900+
Apps: []*sdkproto.App{
2901+
{
2902+
Id:sidebarAppID.String(),
2903+
Slug:"test-app",
2904+
},
2905+
},
2906+
},
2907+
},
2908+
},
2909+
},
2910+
},
2911+
isTask:true,
2912+
expectTaskStatus:database.TaskStatusInitializing,
2913+
expectAppID: uuid.NullUUID{UUID:sidebarAppID,Valid:true},
2914+
expectHasAiTask:true,
2915+
expectUsageEvent:true,
2916+
},
2917+
{
2918+
name:"has_ai_task is set to true, with sidebar app id",
2919+
transition:database.WorkspaceTransitionStart,
28862920
input:&proto.CompletedJob_WorkspaceBuild{
28872921
AiTasks: []*sdkproto.AITask{
28882922
{
28892923
Id:uuid.NewString(),
28902924
SidebarApp:&sdkproto.AITaskSidebarApp{
2891-
Id:sidebarAppID,
2925+
Id:sidebarAppID.String(),
28922926
},
28932927
},
28942928
},
@@ -2900,7 +2934,7 @@ func TestCompleteJob(t *testing.T) {
29002934
Name:"a",
29012935
Apps: []*sdkproto.App{
29022936
{
2903-
Id:sidebarAppID,
2937+
Id:sidebarAppID.String(),
29042938
Slug:"test-app",
29052939
},
29062940
},
@@ -2911,6 +2945,7 @@ func TestCompleteJob(t *testing.T) {
29112945
},
29122946
isTask:true,
29132947
expectTaskStatus:database.TaskStatusInitializing,
2948+
expectAppID: uuid.NullUUID{UUID:sidebarAppID,Valid:true},
29142949
expectHasAiTask:true,
29152950
expectUsageEvent:true,
29162951
},
@@ -2922,10 +2957,9 @@ func TestCompleteJob(t *testing.T) {
29222957
AiTasks: []*sdkproto.AITask{
29232958
{
29242959
Id:uuid.NewString(),
2925-
SidebarApp:&sdkproto.AITaskSidebarApp{
2926-
// Non-existing app ID would previously trigger a FK violation.
2927-
Id:uuid.NewString(),
2928-
},
2960+
// Non-existing app ID would previously trigger a FK violation.
2961+
// Now it should just be ignored.
2962+
AppId:sidebarAppID.String(),
29292963
},
29302964
},
29312965
},
@@ -2940,10 +2974,8 @@ func TestCompleteJob(t *testing.T) {
29402974
input:&proto.CompletedJob_WorkspaceBuild{
29412975
AiTasks: []*sdkproto.AITask{
29422976
{
2943-
Id:uuid.NewString(),
2944-
SidebarApp:&sdkproto.AITaskSidebarApp{
2945-
Id:sidebarAppID,
2946-
},
2977+
Id:uuid.NewString(),
2978+
AppId:sidebarAppID.String(),
29472979
},
29482980
},
29492981
Resources: []*sdkproto.Resource{
@@ -2954,7 +2986,7 @@ func TestCompleteJob(t *testing.T) {
29542986
Name:"a",
29552987
Apps: []*sdkproto.App{
29562988
{
2957-
Id:sidebarAppID,
2989+
Id:sidebarAppID.String(),
29582990
Slug:"test-app",
29592991
},
29602992
},
@@ -2965,6 +2997,7 @@ func TestCompleteJob(t *testing.T) {
29652997
},
29662998
isTask:true,
29672999
expectTaskStatus:database.TaskStatusPaused,
3000+
expectAppID: uuid.NullUUID{UUID:sidebarAppID,Valid:true},
29683001
expectHasAiTask:true,
29693002
expectUsageEvent:false,
29703003
},
@@ -2978,7 +3011,7 @@ func TestCompleteJob(t *testing.T) {
29783011
},
29793012
isTask:true,
29803013
expectTaskStatus:database.TaskStatusPaused,
2981-
expectHasAiTask:true,
3014+
expectHasAiTask:false,// We no longer inherit this from the previous build.
29823015
expectUsageEvent:false,
29833016
},
29843017
} {
@@ -3092,15 +3125,16 @@ func TestCompleteJob(t *testing.T) {
30923125
require.True(t,build.HasAITask.Valid)// We ALWAYS expect a value to be set, therefore not nil, i.e. valid = true.
30933126
require.Equal(t,tc.expectHasAiTask,build.HasAITask.Bool)
30943127

3128+
task,err:=db.GetTaskByID(ctx,genTask.ID)
30953129
iftc.isTask {
3096-
task,err:=db.GetTaskByID(ctx,genTask.ID)
30973130
require.NoError(t,err)
30983131
require.Equal(t,tc.expectTaskStatus,task.Status)
3132+
}else {
3133+
require.Error(t,err)
30993134
}
31003135

3101-
iftc.expectHasAiTask&&build.Transition!=database.WorkspaceTransitionStop {
3102-
require.Equal(t,sidebarAppID,build.AITaskSidebarAppID.UUID.String())
3103-
}
3136+
require.Equal(t,tc.expectAppID,task.WorkspaceAppID)
3137+
require.Equal(t,tc.expectAppID,build.AITaskSidebarAppID)
31043138

31053139
iftc.expectUsageEvent {
31063140
// Check that a usage event was collected.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp