-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: set codersdk.Task current_state during task initialization #20692
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6d099cb
7191e4b
eefbc3f
d391626
c6c5c7f
cf9a97e
306296a
f07e3d8
c7a0c14
b1cb7fe
0ae69ba
23bd215
a069cbf
9e9b80f
979e370
677339d
6582f80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import ( | |
| "golang.org/x/xerrors" | ||
|
|
||
| "cdr.dev/slog" | ||
|
|
||
| "github.com/coder/coder/v2/coderd/audit" | ||
| "github.com/coder/coder/v2/coderd/database" | ||
| "github.com/coder/coder/v2/coderd/database/dbtime" | ||
|
|
@@ -23,6 +24,7 @@ import ( | |
| "github.com/coder/coder/v2/coderd/rbac/policy" | ||
| "github.com/coder/coder/v2/coderd/searchquery" | ||
| "github.com/coder/coder/v2/coderd/taskname" | ||
| "github.com/coder/coder/v2/coderd/util/ptr" | ||
| "github.com/coder/coder/v2/coderd/util/slice" | ||
| "github.com/coder/coder/v2/codersdk" | ||
|
|
||
|
|
@@ -270,37 +272,29 @@ func (api *API) tasksCreate(rw http.ResponseWriter, r *http.Request) { | |
| func taskFromDBTaskAndWorkspace(dbTask database.Task, ws codersdk.Workspace) codersdk.Task { | ||
| var taskAgentLifecycle *codersdk.WorkspaceAgentLifecycle | ||
| var taskAgentHealth *codersdk.WorkspaceAgentHealth | ||
| var taskAppHealth *codersdk.WorkspaceAppHealth | ||
|
|
||
| if dbTask.WorkspaceAgentLifecycleState.Valid { | ||
| taskAgentLifecycle = ptr.Ref(codersdk.WorkspaceAgentLifecycle(dbTask.WorkspaceAgentLifecycleState.WorkspaceAgentLifecycleState)) | ||
| } | ||
| if dbTask.WorkspaceAppHealth.Valid { | ||
| taskAppHealth = ptr.Ref(codersdk.WorkspaceAppHealth(dbTask.WorkspaceAppHealth.WorkspaceAppHealth)) | ||
| } | ||
|
|
||
| // If we have an agent ID from the task, find the agent details in the | ||
| // workspace. | ||
| // If we have an agent ID from the task, find the agent health info | ||
| if dbTask.WorkspaceAgentID.Valid { | ||
| findTaskAgentLoop: | ||
| for _, resource := range ws.LatestBuild.Resources { | ||
| for _, agent := range resource.Agents { | ||
| if agent.ID == dbTask.WorkspaceAgentID.UUID { | ||
| taskAgentLifecycle = &agent.LifecycleState | ||
| taskAgentHealth = &agent.Health | ||
| break findTaskAgentLoop | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Ignore 'latest app status' if it is older than the latest build and the | ||
| // latest build is a 'start' transition. This ensures that you don't show a | ||
| // stale app status from a previous build. For stop transitions, there is | ||
| // still value in showing the latest app status. | ||
| var currentState *codersdk.TaskStateEntry | ||
| if ws.LatestAppStatus != nil { | ||
| if ws.LatestBuild.Transition != codersdk.WorkspaceTransitionStart || ws.LatestAppStatus.CreatedAt.After(ws.LatestBuild.CreatedAt) { | ||
| currentState = &codersdk.TaskStateEntry{ | ||
| Timestamp: ws.LatestAppStatus.CreatedAt, | ||
| State: codersdk.TaskState(ws.LatestAppStatus.State), | ||
| Message: ws.LatestAppStatus.Message, | ||
| URI: ws.LatestAppStatus.URI, | ||
| } | ||
| } | ||
| } | ||
| currentState := deriveTaskCurrentState(dbTask, ws, taskAgentLifecycle, taskAppHealth) | ||
|
|
||
| return codersdk.Task{ | ||
| ID: dbTask.ID, | ||
|
|
@@ -330,6 +324,73 @@ func taskFromDBTaskAndWorkspace(dbTask database.Task, ws codersdk.Workspace) cod | |
| } | ||
| } | ||
|
|
||
| // deriveTaskCurrentState determines the current state of a task based on the | ||
| // workspace's latest app status and initialization phase. | ||
| // Returns nil if no valid state can be determined. | ||
| func deriveTaskCurrentState( | ||
| dbTask database.Task, | ||
| ws codersdk.Workspace, | ||
| taskAgentLifecycle *codersdk.WorkspaceAgentLifecycle, | ||
| taskAppHealth *codersdk.WorkspaceAppHealth, | ||
| ) *codersdk.TaskStateEntry { | ||
| var currentState *codersdk.TaskStateEntry | ||
|
|
||
| // Ignore 'latest app status' if it is older than the latest build and the | ||
| // latest build is a 'start' transition. This ensures that you don't show a | ||
| // stale app status from a previous build. For stop transitions, there is | ||
| // still value in showing the latest app status. | ||
| if ws.LatestAppStatus != nil { | ||
| if ws.LatestBuild.Transition != codersdk.WorkspaceTransitionStart || ws.LatestAppStatus.CreatedAt.After(ws.LatestBuild.CreatedAt) { | ||
| currentState = &codersdk.TaskStateEntry{ | ||
| Timestamp: ws.LatestAppStatus.CreatedAt, | ||
| State: codersdk.TaskState(ws.LatestAppStatus.State), | ||
| Message: ws.LatestAppStatus.Message, | ||
| URI: ws.LatestAppStatus.URI, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If no valid agent state was found for the current build and the task is initializing, | ||
| // provide a descriptive initialization message. | ||
| if currentState == nil && dbTask.Status == database.TaskStatusInitializing { | ||
| message := "Initializing workspace" | ||
|
|
||
| switch { | ||
| case ws.LatestBuild.Status == codersdk.WorkspaceStatusPending || | ||
| ws.LatestBuild.Status == codersdk.WorkspaceStatusStarting: | ||
| message = fmt.Sprintf("Workspace is %s", ws.LatestBuild.Status) | ||
| case taskAgentLifecycle != nil: | ||
| switch { | ||
| case *taskAgentLifecycle == codersdk.WorkspaceAgentLifecycleCreated: | ||
| message = "Agent is connecting" | ||
| case *taskAgentLifecycle == codersdk.WorkspaceAgentLifecycleStarting: | ||
| message = "Agent is starting" | ||
| case *taskAgentLifecycle == codersdk.WorkspaceAgentLifecycleReady: | ||
| if taskAppHealth != nil && *taskAppHealth == codersdk.WorkspaceAppHealthInitializing { | ||
| message = "App is initializing" | ||
| } else { | ||
| // In case the workspace app is not initializing, | ||
| // the overall task status should be updated accordingly | ||
| message = "Initializing workspace applications" | ||
| } | ||
| default: | ||
| // In case the workspace agent is not initializing, | ||
| // the overall task status should be updated accordingly | ||
| message = "Initializing workspace agent" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should never happen, but the fallback is fine, should we log something if this happens?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this should never happen, similarly to the |
||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we take into account the coder app status as well?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking about I'm not sure if we need to enrich
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah, I see. So this would be the case where the agent is ready but the app is still initializing, correct? Do you think it makes sense to get into that level of detail? We have different possible AppHealth statuses: I'm guessing only the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yep! 👍🏻
It would better reflect how we determine the tasks status in the
Correct. 👍🏻 We could consider
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in b1cb7fe |
||
| } | ||
|
|
||
| currentState = &codersdk.TaskStateEntry{ | ||
| Timestamp: ws.LatestBuild.CreatedAt, | ||
| State: codersdk.TaskStateWorking, | ||
| Message: message, | ||
| URI: "", | ||
| } | ||
| } | ||
|
|
||
| return currentState | ||
| } | ||
|
|
||
| // @Summary List AI tasks | ||
| // @Description: EXPERIMENTAL: this endpoint is experimental and not guaranteed to be stable. | ||
| // @ID list-tasks | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,223 @@ | ||
| package coderd | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/google/uuid" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/coder/coder/v2/coderd/database" | ||
| "github.com/coder/coder/v2/coderd/util/ptr" | ||
| "github.com/coder/coder/v2/codersdk" | ||
| ) | ||
|
|
||
| func TestDeriveTaskCurrentState_Unit(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| now := time.Now() | ||
| tests := []struct { | ||
| name string | ||
| task database.Task | ||
| agentLifecycle *codersdk.WorkspaceAgentLifecycle | ||
| appHealth *codersdk.WorkspaceAppHealth | ||
| latestAppStatus *codersdk.WorkspaceAppStatus | ||
| latestBuild codersdk.WorkspaceBuild | ||
| expectCurrentState bool | ||
| expectedTimestamp time.Time | ||
| expectedState codersdk.TaskState | ||
| expectedMessage string | ||
| }{ | ||
| { | ||
| name: "NoAppStatus", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusActive, | ||
| }, | ||
| agentLifecycle: nil, | ||
| appHealth: nil, | ||
| latestAppStatus: nil, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Transition: codersdk.WorkspaceTransitionStart, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: false, | ||
| }, | ||
| { | ||
| name: "BuildStartTransition_AppStatus_NewerThanBuild", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusActive, | ||
| }, | ||
| agentLifecycle: nil, | ||
| appHealth: nil, | ||
| latestAppStatus: &codersdk.WorkspaceAppStatus{ | ||
| State: codersdk.WorkspaceAppStatusStateWorking, | ||
| Message: "Task is working", | ||
| CreatedAt: now.Add(1 * time.Minute), | ||
| }, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Transition: codersdk.WorkspaceTransitionStart, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: true, | ||
| expectedTimestamp: now.Add(1 * time.Minute), | ||
| expectedState: codersdk.TaskState(codersdk.WorkspaceAppStatusStateWorking), | ||
| expectedMessage: "Task is working", | ||
| }, | ||
| { | ||
| name: "BuildStartTransition_StaleAppStatus_OlderThanBuild", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusActive, | ||
| }, | ||
| agentLifecycle: nil, | ||
| appHealth: nil, | ||
| latestAppStatus: &codersdk.WorkspaceAppStatus{ | ||
| State: codersdk.WorkspaceAppStatusStateComplete, | ||
| Message: "Previous task completed", | ||
| CreatedAt: now.Add(-1 * time.Minute), | ||
| }, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Transition: codersdk.WorkspaceTransitionStart, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: false, | ||
| }, | ||
| { | ||
| name: "BuildStopTransition", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusActive, | ||
| }, | ||
| agentLifecycle: nil, | ||
| appHealth: nil, | ||
| latestAppStatus: &codersdk.WorkspaceAppStatus{ | ||
| State: codersdk.WorkspaceAppStatusStateComplete, | ||
| Message: "Task completed before stop", | ||
| CreatedAt: now.Add(-1 * time.Minute), | ||
| }, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Transition: codersdk.WorkspaceTransitionStop, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: true, | ||
| expectedTimestamp: now.Add(-1 * time.Minute), | ||
| expectedState: codersdk.TaskState(codersdk.WorkspaceAppStatusStateComplete), | ||
| expectedMessage: "Task completed before stop", | ||
| }, | ||
| { | ||
| name: "TaskInitializing_WorkspacePending", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusInitializing, | ||
| }, | ||
| agentLifecycle: nil, | ||
| appHealth: nil, | ||
| latestAppStatus: nil, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Status: codersdk.WorkspaceStatusPending, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: true, | ||
| expectedTimestamp: now, | ||
| expectedState: codersdk.TaskStateWorking, | ||
| expectedMessage: "Workspace is pending", | ||
| }, | ||
| { | ||
| name: "TaskInitializing_WorkspaceStarting", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusInitializing, | ||
| }, | ||
| agentLifecycle: nil, | ||
| appHealth: nil, | ||
| latestAppStatus: nil, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Status: codersdk.WorkspaceStatusStarting, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: true, | ||
| expectedTimestamp: now, | ||
| expectedState: codersdk.TaskStateWorking, | ||
| expectedMessage: "Workspace is starting", | ||
| }, | ||
| { | ||
| name: "TaskInitializing_AgentConnecting", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusInitializing, | ||
| }, | ||
| agentLifecycle: ptr.Ref(codersdk.WorkspaceAgentLifecycleCreated), | ||
| appHealth: nil, | ||
| latestAppStatus: nil, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Status: codersdk.WorkspaceStatusRunning, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: true, | ||
| expectedTimestamp: now, | ||
| expectedState: codersdk.TaskStateWorking, | ||
| expectedMessage: "Agent is connecting", | ||
| }, | ||
| { | ||
| name: "TaskInitializing_AgentStarting", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusInitializing, | ||
| }, | ||
| agentLifecycle: ptr.Ref(codersdk.WorkspaceAgentLifecycleStarting), | ||
| appHealth: nil, | ||
| latestAppStatus: nil, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Status: codersdk.WorkspaceStatusRunning, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: true, | ||
| expectedTimestamp: now, | ||
| expectedState: codersdk.TaskStateWorking, | ||
| expectedMessage: "Agent is starting", | ||
| }, | ||
| { | ||
| name: "TaskInitializing_AppInitializing", | ||
| task: database.Task{ | ||
| ID: uuid.New(), | ||
| Status: database.TaskStatusInitializing, | ||
| }, | ||
| agentLifecycle: ptr.Ref(codersdk.WorkspaceAgentLifecycleReady), | ||
| appHealth: ptr.Ref(codersdk.WorkspaceAppHealthInitializing), | ||
| latestAppStatus: nil, | ||
| latestBuild: codersdk.WorkspaceBuild{ | ||
| Status: codersdk.WorkspaceStatusRunning, | ||
| CreatedAt: now, | ||
| }, | ||
| expectCurrentState: true, | ||
| expectedTimestamp: now, | ||
| expectedState: codersdk.TaskStateWorking, | ||
| expectedMessage: "App is initializing", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| ws := codersdk.Workspace{ | ||
| LatestBuild: tt.latestBuild, | ||
| LatestAppStatus: tt.latestAppStatus, | ||
| } | ||
|
|
||
| currentState := deriveTaskCurrentState(tt.task, ws, tt.agentLifecycle, tt.appHealth) | ||
|
|
||
| if tt.expectCurrentState { | ||
| require.NotNil(t, currentState) | ||
| assert.Equal(t, tt.expectedTimestamp.UTC(), currentState.Timestamp.UTC()) | ||
| assert.Equal(t, tt.expectedState, currentState.State) | ||
| assert.Equal(t, tt.expectedMessage, currentState.Message) | ||
| } else { | ||
| assert.Nil(t, currentState) | ||
| } | ||
| }) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.