- Notifications
You must be signed in to change notification settings - Fork1k
feat(coderd): add task status tracking and RBAC#20212
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
6fdbe46 to5fa8a93Compare| // the agent to a task. For now we overwrite the | ||
| // protoAgent.Id because that matches old behavior. | ||
| agentID:=uuid.New() | ||
| protoAgent.Id=agentID.String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Review: This is potentially the most controversial change. I've retain previous behavior inInsertWorkspaceResource if this value isn't set (e.g. template import).
This basically moves the behavior one level up so that we can access the ID here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I definitely want to defer review of this section to someone with more provisioner knowledge than myself.
5fa8a93 tob1e8378CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this touches on some areas (namelywsbuilder andprovisionerdserver) that I don't feel knowledgeable enough to give this the green light, but it looks good for the most part 👍
| ifagentState!="" { | ||
| agent:=dbgen.WorkspaceAgent(t,db, database.WorkspaceAgent{ | ||
| ResourceID:resource.ID, | ||
| }) | ||
| workspaceAgentID:=agent.ID | ||
| _,err:=db.UpsertTaskWorkspaceApp(ctx, database.UpsertTaskWorkspaceAppParams{ | ||
| TaskID:task.ID, | ||
| WorkspaceBuildNumber:workspaceBuildNumber, | ||
| WorkspaceAgentID: uuid.NullUUID{UUID:workspaceAgentID,Valid:true}, | ||
| }) | ||
| require.NoError(t,err) | ||
| err=db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ | ||
| ID:agent.ID, | ||
| LifecycleState:agentState, | ||
| }) | ||
| require.NoError(t,err) | ||
| fori,health:=rangeappHealths { | ||
| app:=dbgen.WorkspaceApp(t,db, database.WorkspaceApp{ | ||
| AgentID:workspaceAgentID, | ||
| Slug:fmt.Sprintf("test-app-%d",i), | ||
| DisplayName:fmt.Sprintf("Test App %d",i+1), | ||
| Health:health, | ||
| }) | ||
| ifi==0 { | ||
| // Assume the first app is the tasks app. | ||
| _,err:=db.UpsertTaskWorkspaceApp(ctx, database.UpsertTaskWorkspaceAppParams{ | ||
| TaskID:task.ID, | ||
| WorkspaceBuildNumber:workspaceBuildNumber, | ||
| WorkspaceAgentID: uuid.NullUUID{UUID:workspaceAgentID,Valid:true}, | ||
| WorkspaceAppID: uuid.NullUUID{UUID:app.ID,Valid:true}, | ||
| }) | ||
| require.NoError(t,err) | ||
| } | ||
| } | ||
| } | ||
| returntask |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Invert the condition to reduce nesting
| ifagentState!="" { | |
| agent:=dbgen.WorkspaceAgent(t,db, database.WorkspaceAgent{ | |
| ResourceID:resource.ID, | |
| }) | |
| workspaceAgentID:=agent.ID | |
| _,err:=db.UpsertTaskWorkspaceApp(ctx, database.UpsertTaskWorkspaceAppParams{ | |
| TaskID:task.ID, | |
| WorkspaceBuildNumber:workspaceBuildNumber, | |
| WorkspaceAgentID: uuid.NullUUID{UUID:workspaceAgentID,Valid:true}, | |
| }) | |
| require.NoError(t,err) | |
| err=db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ | |
| ID:agent.ID, | |
| LifecycleState:agentState, | |
| }) | |
| require.NoError(t,err) | |
| fori,health:=rangeappHealths { | |
| app:=dbgen.WorkspaceApp(t,db, database.WorkspaceApp{ | |
| AgentID:workspaceAgentID, | |
| Slug:fmt.Sprintf("test-app-%d",i), | |
| DisplayName:fmt.Sprintf("Test App %d",i+1), | |
| Health:health, | |
| }) | |
| ifi==0 { | |
| // Assume the first app is the tasks app. | |
| _,err:=db.UpsertTaskWorkspaceApp(ctx, database.UpsertTaskWorkspaceAppParams{ | |
| TaskID:task.ID, | |
| WorkspaceBuildNumber:workspaceBuildNumber, | |
| WorkspaceAgentID: uuid.NullUUID{UUID:workspaceAgentID,Valid:true}, | |
| WorkspaceAppID: uuid.NullUUID{UUID:app.ID,Valid:true}, | |
| }) | |
| require.NoError(t,err) | |
| } | |
| } | |
| } | |
| returntask | |
| ifagentState=="" { | |
| returntask | |
| } | |
| agent:=dbgen.WorkspaceAgent(t,db, database.WorkspaceAgent{ | |
| ResourceID:resource.ID, | |
| }) | |
| workspaceAgentID:=agent.ID | |
| _,err:=db.UpsertTaskWorkspaceApp(ctx, database.UpsertTaskWorkspaceAppParams{ | |
| TaskID:task.ID, | |
| WorkspaceBuildNumber:workspaceBuildNumber, | |
| WorkspaceAgentID: uuid.NullUUID{UUID:workspaceAgentID,Valid:true}, | |
| }) | |
| require.NoError(t,err) | |
| err=db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ | |
| ID:agent.ID, | |
| LifecycleState:agentState, | |
| }) | |
| require.NoError(t,err) | |
| fori,health:=rangeappHealths { | |
| app:=dbgen.WorkspaceApp(t,db, database.WorkspaceApp{ | |
| AgentID:workspaceAgentID, | |
| Slug:fmt.Sprintf("test-app-%d",i), | |
| DisplayName:fmt.Sprintf("Test App %d",i+1), | |
| Health:health, | |
| }) | |
| ifi==0 { | |
| // Assume the first app is the tasks app. | |
| _,err:=db.UpsertTaskWorkspaceApp(ctx, database.UpsertTaskWorkspaceAppParams{ | |
| TaskID:task.ID, | |
| WorkspaceBuildNumber:workspaceBuildNumber, | |
| WorkspaceAgentID: uuid.NullUUID{UUID:workspaceAgentID,Valid:true}, | |
| WorkspaceAppID: uuid.NullUUID{UUID:app.ID,Valid:true}, | |
| }) | |
| require.NoError(t,err) | |
| } | |
| } | |
| returntask |
| buildStatus:database.ProvisionerJobStatusRunning, | ||
| buildTransition:database.WorkspaceTransitionStart, | ||
| agentState:database.WorkspaceAgentLifecycleStateReady, | ||
| appHealths: []database.WorkspaceAppHealth{database.WorkspaceAppHealthHealthy,database.WorkspaceAppHealthHealthy}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It looks like this is the only test with two app healths, did we want any other tests that validate the behavior when an appthat isn't the task app is unhealthy?
| // the agent to a task. For now we overwrite the | ||
| // protoAgent.Id because that matches old behavior. | ||
| agentID:=uuid.New() | ||
| protoAgent.Id=agentID.String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I definitely want to defer review of this section to someone with more provisioner knowledge than myself.
b1e8378 to9f9d402Compare36ff30a tofde6689Comparefde6689 to12dc05eCompare
This is a continuation of#19773.
task_workspace_appsto link to workspace build number, allowing us to utilize sorting for the latest entry as well as retaining historytask_workspace_appsworkspace_build_idsinceworkspace_build_numbermakes it redundant (except for tracking foreign keys, if we want it can be added back)tasks<->task_workspace_appslinking inwsbuildandprovisionerdserverwsbuildcreates an early link to properly track build/job status of task ASAPprovisionerdserverupdates agent and app IDs upon completionClosescoder/internal#948
Notes: