From 17712b201725f4dda390eb16b2e43b309d45a85d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 27 May 2025 08:45:32 +0000 Subject: [PATCH 1/6] feat(site): allow recreating devcontainers and showing dirty status This change allows showing the devcontainer dirty status in the UI as well as a recreate button to update the devcontainer. Closes #16424 --- agent/agent_test.go | 2 +- coderd/workspaceagents.go | 4 +- coderd/workspaceagents_test.go | 2 +- codersdk/workspaceagents.go | 14 ++- codersdk/workspacesdk/agentconn.go | 12 ++- .../resources/AgentDevcontainerCard.tsx | 100 ++++++++++++++++-- 6 files changed, 111 insertions(+), 23 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 6c0feca812e8b..3a2562237b603 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -2226,7 +2226,7 @@ func TestAgent_DevcontainerRecreate(t *testing.T) { // devcontainer, we do it in a goroutine so we can process logs // concurrently. go func(container codersdk.WorkspaceAgentContainer) { - err := conn.RecreateDevcontainer(ctx, container.ID) + _, err := conn.RecreateDevcontainer(ctx, container.ID) assert.NoError(t, err, "recreate devcontainer should succeed") }(container) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 5a8adab6132c5..6b25fcbcfeaf6 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -956,7 +956,7 @@ func (api *API) workspaceAgentRecreateDevcontainer(rw http.ResponseWriter, r *ht } defer release() - err = agentConn.RecreateDevcontainer(ctx, container) + m, err := agentConn.RecreateDevcontainer(ctx, container) if err != nil { if errors.Is(err, context.Canceled) { httpapi.Write(ctx, rw, http.StatusRequestTimeout, codersdk.Response{ @@ -977,7 +977,7 @@ func (api *API) workspaceAgentRecreateDevcontainer(rw http.ResponseWriter, r *ht return } - httpapi.Write(ctx, rw, http.StatusNoContent, nil) + httpapi.Write(ctx, rw, http.StatusAccepted, m) } // @Summary Get connection info for workspace agent diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index 1d17560c38816..5635296d1a47b 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -1483,7 +1483,7 @@ func TestWorkspaceAgentRecreateDevcontainer(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) - err := client.WorkspaceAgentRecreateDevcontainer(ctx, agentID, devContainer.ID) + _, err := client.WorkspaceAgentRecreateDevcontainer(ctx, agentID, devContainer.ID) if wantStatus > 0 { cerr, ok := codersdk.AsError(err) require.True(t, ok, "expected error to be a coder error") diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index 0c5aaddf913da..a0fe12c4ac8ef 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -518,16 +518,20 @@ func (c *Client) WorkspaceAgentListContainers(ctx context.Context, agentID uuid. } // WorkspaceAgentRecreateDevcontainer recreates the devcontainer with the given ID. -func (c *Client) WorkspaceAgentRecreateDevcontainer(ctx context.Context, agentID uuid.UUID, containerIDOrName string) error { +func (c *Client) WorkspaceAgentRecreateDevcontainer(ctx context.Context, agentID uuid.UUID, containerIDOrName string) (Response, error) { res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/workspaceagents/%s/containers/devcontainers/container/%s/recreate", agentID, containerIDOrName), nil) if err != nil { - return err + return Response{}, err } defer res.Body.Close() - if res.StatusCode != http.StatusNoContent { - return ReadBodyAsError(res) + if res.StatusCode != http.StatusAccepted { + return Response{}, ReadBodyAsError(res) } - return nil + var m Response + if err := json.NewDecoder(res.Body).Decode(&m); err != nil { + return Response{}, xerrors.Errorf("decode response body: %w", err) + } + return m, nil } //nolint:revive // Follow is a control flag on the server as well. diff --git a/codersdk/workspacesdk/agentconn.go b/codersdk/workspacesdk/agentconn.go index c9e9824e2950f..3477ec98328ac 100644 --- a/codersdk/workspacesdk/agentconn.go +++ b/codersdk/workspacesdk/agentconn.go @@ -389,18 +389,22 @@ func (c *AgentConn) ListContainers(ctx context.Context) (codersdk.WorkspaceAgent // RecreateDevcontainer recreates a devcontainer with the given container. // This is a blocking call and will wait for the container to be recreated. -func (c *AgentConn) RecreateDevcontainer(ctx context.Context, containerIDOrName string) error { +func (c *AgentConn) RecreateDevcontainer(ctx context.Context, containerIDOrName string) (codersdk.Response, error) { ctx, span := tracing.StartSpan(ctx) defer span.End() res, err := c.apiRequest(ctx, http.MethodPost, "/api/v0/containers/devcontainers/container/"+containerIDOrName+"/recreate", nil) if err != nil { - return xerrors.Errorf("do request: %w", err) + return codersdk.Response{}, xerrors.Errorf("do request: %w", err) } defer res.Body.Close() if res.StatusCode != http.StatusAccepted { - return codersdk.ReadBodyAsError(res) + return codersdk.Response{}, codersdk.ReadBodyAsError(res) } - return nil + var m codersdk.Response + if err := json.NewDecoder(res.Body).Decode(&m); err != nil { + return codersdk.Response{}, xerrors.Errorf("decode response body: %w", err) + } + return m, nil } // apiRequest makes a request to the workspace agent's HTTP API server. diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 543004de5c1e2..0072c3feced25 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -3,14 +3,24 @@ import type { WorkspaceAgent, WorkspaceAgentContainer, } from "api/typesGenerated"; +import { Button } from "components/Button/Button"; +import { displayError } from "components/GlobalSnackbar/utils"; +import { + HelpTooltip, + HelpTooltipContent, + HelpTooltipText, + HelpTooltipTitle, + HelpTooltipTrigger, +} from "components/HelpTooltip/HelpTooltip"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger, } from "components/Tooltip/Tooltip"; -import { ExternalLinkIcon } from "lucide-react"; +import { ExternalLinkIcon, Loader2Icon } from "lucide-react"; import type { FC } from "react"; +import { useState } from "react"; import { portForwardURL } from "utils/portForward"; import { AgentButton } from "./AgentButton"; import { AgentDevcontainerSSHButton } from "./SSHButton/SSHButton"; @@ -32,24 +42,94 @@ export const AgentDevcontainerCard: FC = ({ }) => { const folderPath = container.labels["devcontainer.local_folder"]; const containerFolder = container.volumes[folderPath]; + const [isRecreating, setIsRecreating] = useState(false); + + const handleRecreateDevcontainer = async () => { + setIsRecreating(true); + let recreateSucceeded = false; + try { + const response = await fetch( + `/api/v2/workspaceagents/${agent.id}/containers/devcontainers/container/${container.id}/recreate`, + { + method: "POST", + }, + ); + if (!response.ok) { + const errorData = await response.json().catch(() => ({})); + throw new Error( + errorData.message || `Failed to recreate: ${response.statusText}`, + ); + } + // If the request was accepted (e.g. 202), we mark it as succeeded. + // Once complete, the component will unmount, so the spinner will + // disappear with it. + if (response.status === 202) { + recreateSucceeded = true; + } + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : "An unknown error occurred."; + displayError(`Failed to recreate devcontainer: ${errorMessage}`); + console.error("Failed to recreate devcontainer:", error); + } finally { + if (!recreateSucceeded) { + setIsRecreating(false); + } + } + }; return (
-
-

- {container.name} -

+
+
+

+ {container.name} +

+ {container.devcontainer_dirty && ( + + + Outdated + + + Devcontainer Outdated + + Devcontainer configuration has been modified and is outdated. + Recreate to get an up-to-date container. + + + + )} +
- +
+ + + +
-

Forwarded ports

+

Forwarded ports

Date: Tue, 27 May 2025 12:01:33 +0000 Subject: [PATCH 2/6] try to add storybook tests --- .../AgentDevcontainerCard.stories.tsx | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index e965efea75b6d..e038884284f92 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -1,4 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react"; +import { within, userEvent } from "@storybook/test"; import { MockWorkspace, MockWorkspaceAgent, @@ -31,3 +32,30 @@ export const WithPorts: Story = { }, }, }; + +export const Dirty: Story = { + args: { + container: { + ...MockWorkspaceAgentContainer, + devcontainer_dirty: true, + ports: MockWorkspaceAgentContainerPorts, + }, + }, +}; + +export const Recreating: Story = { + args: { + container: { + ...MockWorkspaceAgentContainer, + devcontainer_dirty: true, + ports: MockWorkspaceAgentContainerPorts, + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const recreateButton = await canvas.findByRole("button", { + name: /recreate/i, + }); + await userEvent.click(recreateButton); + }, +}; From 84ccd9210a623b2428f720793cbaa84445947ac4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 27 May 2025 13:08:46 +0000 Subject: [PATCH 3/6] reflect devcontainer status in container and fix story --- agent/agentcontainers/api.go | 22 +++++++-- coderd/apidoc/docs.go | 23 +++++++++ coderd/apidoc/swagger.json | 18 +++++++ codersdk/workspaceagents.go | 4 ++ docs/reference/api/agents.md | 1 + docs/reference/api/schemas.md | 48 +++++++++++++------ site/src/api/typesGenerated.ts | 1 + .../AgentDevcontainerCard.stories.tsx | 9 +--- .../resources/AgentDevcontainerCard.tsx | 11 ++++- 9 files changed, 110 insertions(+), 27 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index b28e39ad8c57b..4082dec9598e7 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -403,6 +403,7 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code // Check if the container is running and update the known devcontainers. for i := range updated.Containers { container := &updated.Containers[i] // Grab a reference to the container to allow mutating it. + container.DevcontainerStatus = "" // Reset the status for the container (updated later). container.DevcontainerDirty = false // Reset dirty state for the container (updated later). workspaceFolder := container.Labels[DevcontainerLocalFolderLabel] @@ -463,6 +464,11 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code // Iterate through all known devcontainers and update their status // based on the current state of the containers. for _, dc := range api.knownDevcontainers { + if dc.Container != nil { + dc.Container.DevcontainerStatus = dc.Status + dc.Container.DevcontainerDirty = dc.Dirty + } + switch { case dc.Status == codersdk.WorkspaceAgentDevcontainerStatusStarting: continue // This state is handled by the recreation routine. @@ -608,6 +614,9 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques // Update the status so that we don't try to recreate the // devcontainer multiple times in parallel. dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStarting + if dc.Container != nil { + dc.Container.DevcontainerStatus = dc.Status + } api.knownDevcontainers[dc.WorkspaceFolder] = dc api.recreateWg.Add(1) go api.recreateDevcontainer(dc, configPath) @@ -680,6 +689,9 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con api.mu.Lock() dc = api.knownDevcontainers[dc.WorkspaceFolder] dc.Status = codersdk.WorkspaceAgentDevcontainerStatusError + if dc.Container != nil { + dc.Container.DevcontainerStatus = dc.Status + } api.knownDevcontainers[dc.WorkspaceFolder] = dc api.recreateErrorTimes[dc.WorkspaceFolder] = api.clock.Now("recreate", "errorTimes") api.mu.Unlock() @@ -695,10 +707,12 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con // allows the update routine to update the devcontainer status, but // to minimize the time between API consistency, we guess the status // based on the container state. - if dc.Container != nil && dc.Container.Running { - dc.Status = codersdk.WorkspaceAgentDevcontainerStatusRunning - } else { - dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStopped + dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStopped + if dc.Container != nil { + if dc.Container.Running { + dc.Status = codersdk.WorkspaceAgentDevcontainerStatusRunning + } + dc.Container.DevcontainerStatus = dc.Status } dc.Dirty = false api.recreateSuccessTimes[dc.WorkspaceFolder] = api.clock.Now("recreate", "successTimes") diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index b6a00051bba77..fde3a48cd4462 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -17198,6 +17198,14 @@ const docTemplate = `{ "description": "DevcontainerDirty is true if the devcontainer configuration has changed\nsince the container was created. This is used to determine if the\ncontainer needs to be rebuilt.", "type": "boolean" }, + "devcontainer_status": { + "description": "DevcontainerStatus is the status of the devcontainer, if this\ncontainer is a devcontainer. This is used to determine if the\ndevcontainer is running, stopped, starting, or in an error state.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerStatus" + } + ] + }, "id": { "description": "ID is the unique identifier of the container.", "type": "string" @@ -17262,6 +17270,21 @@ const docTemplate = `{ } } }, + "codersdk.WorkspaceAgentDevcontainerStatus": { + "type": "string", + "enum": [ + "running", + "stopped", + "starting", + "error" + ], + "x-enum-varnames": [ + "WorkspaceAgentDevcontainerStatusRunning", + "WorkspaceAgentDevcontainerStatusStopped", + "WorkspaceAgentDevcontainerStatusStarting", + "WorkspaceAgentDevcontainerStatusError" + ] + }, "codersdk.WorkspaceAgentHealth": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index e5fdca7025089..6023ea23ec481 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -15703,6 +15703,14 @@ "description": "DevcontainerDirty is true if the devcontainer configuration has changed\nsince the container was created. This is used to determine if the\ncontainer needs to be rebuilt.", "type": "boolean" }, + "devcontainer_status": { + "description": "DevcontainerStatus is the status of the devcontainer, if this\ncontainer is a devcontainer. This is used to determine if the\ndevcontainer is running, stopped, starting, or in an error state.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerStatus" + } + ] + }, "id": { "description": "ID is the unique identifier of the container.", "type": "string" @@ -15767,6 +15775,16 @@ } } }, + "codersdk.WorkspaceAgentDevcontainerStatus": { + "type": "string", + "enum": ["running", "stopped", "starting", "error"], + "x-enum-varnames": [ + "WorkspaceAgentDevcontainerStatusRunning", + "WorkspaceAgentDevcontainerStatusStopped", + "WorkspaceAgentDevcontainerStatusStarting", + "WorkspaceAgentDevcontainerStatusError" + ] + }, "codersdk.WorkspaceAgentHealth": { "type": "object", "properties": { diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index a0fe12c4ac8ef..6a4380fed47ac 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -450,6 +450,10 @@ type WorkspaceAgentContainer struct { // Volumes is a map of "things" mounted into the container. Again, this // is somewhat implementation-dependent. Volumes map[string]string `json:"volumes"` + // DevcontainerStatus is the status of the devcontainer, if this + // container is a devcontainer. This is used to determine if the + // devcontainer is running, stopped, starting, or in an error state. + DevcontainerStatus WorkspaceAgentDevcontainerStatus `json:"devcontainer_status,omitempty"` // DevcontainerDirty is true if the devcontainer configuration has changed // since the container was created. This is used to determine if the // container needs to be rebuilt. diff --git a/docs/reference/api/agents.md b/docs/reference/api/agents.md index fd0cd38d355e0..d0169416239d7 100644 --- a/docs/reference/api/agents.md +++ b/docs/reference/api/agents.md @@ -777,6 +777,7 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/{workspaceagent}/con { "created_at": "2019-08-24T14:15:22Z", "devcontainer_dirty": true, + "devcontainer_status": "running", "id": "string", "image": "string", "labels": { diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 2374c6af8800f..7d3f94ff6f12f 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -8636,6 +8636,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| { "created_at": "2019-08-24T14:15:22Z", "devcontainer_dirty": true, + "devcontainer_status": "running", "id": "string", "image": "string", "labels": { @@ -8662,20 +8663,21 @@ If the schedule is empty, the user will be updated to use the default schedule.| ### Properties -| Name | Type | Required | Restrictions | Description | -|----------------------|---------------------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `created_at` | string | false | | Created at is the time the container was created. | -| `devcontainer_dirty` | boolean | false | | Devcontainer dirty is true if the devcontainer configuration has changed since the container was created. This is used to determine if the container needs to be rebuilt. | -| `id` | string | false | | ID is the unique identifier of the container. | -| `image` | string | false | | Image is the name of the container image. | -| `labels` | object | false | | Labels is a map of key-value pairs of container labels. | -| » `[any property]` | string | false | | | -| `name` | string | false | | Name is the human-readable name of the container. | -| `ports` | array of [codersdk.WorkspaceAgentContainerPort](#codersdkworkspaceagentcontainerport) | false | | Ports includes ports exposed by the container. | -| `running` | boolean | false | | Running is true if the container is currently running. | -| `status` | string | false | | Status is the current status of the container. This is somewhat implementation-dependent, but should generally be a human-readable string. | -| `volumes` | object | false | | Volumes is a map of "things" mounted into the container. Again, this is somewhat implementation-dependent. | -| » `[any property]` | string | false | | | +| Name | Type | Required | Restrictions | Description | +|-----------------------|----------------------------------------------------------------------------------------|----------|--------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `created_at` | string | false | | Created at is the time the container was created. | +| `devcontainer_dirty` | boolean | false | | Devcontainer dirty is true if the devcontainer configuration has changed since the container was created. This is used to determine if the container needs to be rebuilt. | +| `devcontainer_status` | [codersdk.WorkspaceAgentDevcontainerStatus](#codersdkworkspaceagentdevcontainerstatus) | false | | Devcontainer status is the status of the devcontainer, if this container is a devcontainer. This is used to determine if the devcontainer is running, stopped, starting, or in an error state. | +| `id` | string | false | | ID is the unique identifier of the container. | +| `image` | string | false | | Image is the name of the container image. | +| `labels` | object | false | | Labels is a map of key-value pairs of container labels. | +| » `[any property]` | string | false | | | +| `name` | string | false | | Name is the human-readable name of the container. | +| `ports` | array of [codersdk.WorkspaceAgentContainerPort](#codersdkworkspaceagentcontainerport) | false | | Ports includes ports exposed by the container. | +| `running` | boolean | false | | Running is true if the container is currently running. | +| `status` | string | false | | Status is the current status of the container. This is somewhat implementation-dependent, but should generally be a human-readable string. | +| `volumes` | object | false | | Volumes is a map of "things" mounted into the container. Again, this is somewhat implementation-dependent. | +| » `[any property]` | string | false | | | ## codersdk.WorkspaceAgentContainerPort @@ -8697,6 +8699,23 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `network` | string | false | | Network is the network protocol used by the port (tcp, udp, etc). | | `port` | integer | false | | Port is the port number *inside* the container. | +## codersdk.WorkspaceAgentDevcontainerStatus + +```json +"running" +``` + +### Properties + +#### Enumerated Values + +| Value | +|------------| +| `running` | +| `stopped` | +| `starting` | +| `error` | + ## codersdk.WorkspaceAgentHealth ```json @@ -8743,6 +8762,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| { "created_at": "2019-08-24T14:15:22Z", "devcontainer_dirty": true, + "devcontainer_status": "running", "id": "string", "image": "string", "labels": { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 5125a554cacc1..9fe982fe40d12 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3351,6 +3351,7 @@ export interface WorkspaceAgentContainer { readonly ports: readonly WorkspaceAgentContainerPort[]; readonly status: string; readonly volumes: Record; + readonly devcontainer_status?: WorkspaceAgentDevcontainerStatus; readonly devcontainer_dirty: boolean; } diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index e038884284f92..fdd85d95c4849 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -1,5 +1,4 @@ import type { Meta, StoryObj } from "@storybook/react"; -import { within, userEvent } from "@storybook/test"; import { MockWorkspace, MockWorkspaceAgent, @@ -48,14 +47,8 @@ export const Recreating: Story = { container: { ...MockWorkspaceAgentContainer, devcontainer_dirty: true, + devcontainer_status: "starting", ports: MockWorkspaceAgentContainerPorts, }, }, - play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - const recreateButton = await canvas.findByRole("button", { - name: /recreate/i, - }); - await userEvent.click(recreateButton); - }, }; diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 0072c3feced25..df5f14f3df6e8 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -20,7 +20,7 @@ import { } from "components/Tooltip/Tooltip"; import { ExternalLinkIcon, Loader2Icon } from "lucide-react"; import type { FC } from "react"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { portForwardURL } from "utils/portForward"; import { AgentButton } from "./AgentButton"; import { AgentDevcontainerSSHButton } from "./SSHButton/SSHButton"; @@ -78,6 +78,15 @@ export const AgentDevcontainerCard: FC = ({ } }; + // If the container is starting, reflect this in the recreate button. + useEffect(() => { + if (container.devcontainer_status === "starting") { + setIsRecreating(true); + } else { + setIsRecreating(false); + } + }, [container.devcontainer_status]); + return (
Date: Tue, 27 May 2025 13:27:19 +0000 Subject: [PATCH 4/6] s/text-warning-foreground/text-content-warning/ --- site/src/modules/resources/AgentDevcontainerCard.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index df5f14f3df6e8..4891c632bbc2a 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -99,7 +99,7 @@ export const AgentDevcontainerCard: FC = ({ {container.devcontainer_dirty && ( - + Outdated From a88c75262671c598ec72144c84470a4722f812a1 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 27 May 2025 13:46:38 +0000 Subject: [PATCH 5/6] update test coverage for container devcontainer status --- agent/agentcontainers/api.go | 14 +++++++----- agent/agentcontainers/api_test.go | 36 ++++++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 4082dec9598e7..349b85e3d269f 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -464,16 +464,19 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code // Iterate through all known devcontainers and update their status // based on the current state of the containers. for _, dc := range api.knownDevcontainers { - if dc.Container != nil { - dc.Container.DevcontainerStatus = dc.Status - dc.Container.DevcontainerDirty = dc.Dirty - } - switch { case dc.Status == codersdk.WorkspaceAgentDevcontainerStatusStarting: + if dc.Container != nil { + dc.Container.DevcontainerStatus = dc.Status + dc.Container.DevcontainerDirty = dc.Dirty + } continue // This state is handled by the recreation routine. case dc.Status == codersdk.WorkspaceAgentDevcontainerStatusError && (dc.Container == nil || dc.Container.CreatedAt.Before(api.recreateErrorTimes[dc.WorkspaceFolder])): + if dc.Container != nil { + dc.Container.DevcontainerStatus = dc.Status + dc.Container.DevcontainerDirty = dc.Dirty + } continue // The devcontainer needs to be recreated. case dc.Container != nil: @@ -481,6 +484,7 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code if dc.Container.Running { dc.Status = codersdk.WorkspaceAgentDevcontainerStatusRunning } + dc.Container.DevcontainerStatus = dc.Status dc.Dirty = false if lastModified, hasModTime := api.configFileModifiedTimes[dc.ConfigPath]; hasModTime && dc.Container.CreatedAt.Before(lastModified) { diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 826e7a5030f23..1fd2fefc9046b 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -477,6 +477,8 @@ func TestAPI(t *testing.T) { require.NoError(t, err, "unmarshal response failed") require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Status, "devcontainer is not starting") + require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference") + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not starting") // Allow the devcontainer CLI to continue the up process. close(tt.devcontainerCLI.continueUp) @@ -503,6 +505,8 @@ func TestAPI(t *testing.T) { require.NoError(t, err, "unmarshal response failed after error") require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response after error") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusError, resp.Devcontainers[0].Status, "devcontainer is not in an error state after up failure") + require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference after up failure") + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusError, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not error after up failure") return } @@ -525,7 +529,9 @@ func TestAPI(t *testing.T) { err = json.NewDecoder(rec.Body).Decode(&resp) require.NoError(t, err, "unmarshal response failed after recreation") require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response after recreation") - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, resp.Devcontainers[0].Status, "devcontainer is not stopped after recreation") + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, resp.Devcontainers[0].Status, "devcontainer is not running after recreation") + require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference after recreation") + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not running after recreation") }) } }) @@ -620,6 +626,7 @@ func TestAPI(t *testing.T) { assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc.Status) require.NotNil(t, dc.Container) assert.Equal(t, "runtime-container-1", dc.Container.ID) + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc.Container.DevcontainerStatus) }, }, { @@ -660,12 +667,14 @@ func TestAPI(t *testing.T) { assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStopped, known2.Status) assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, runtime1.Status) - require.NotNil(t, known1.Container) assert.Nil(t, known2.Container) - require.NotNil(t, runtime1.Container) + require.NotNil(t, known1.Container) assert.Equal(t, "known-container-1", known1.Container.ID) + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, known1.Container.DevcontainerStatus) + require.NotNil(t, runtime1.Container) assert.Equal(t, "runtime-container-1", runtime1.Container.ID) + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, runtime1.Container.DevcontainerStatus) }, }, { @@ -704,10 +713,12 @@ func TestAPI(t *testing.T) { assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStopped, nonRunning.Status) require.NotNil(t, running.Container, "running container should have container reference") - require.NotNil(t, nonRunning.Container, "non-running container should have container reference") - assert.Equal(t, "running-container", running.Container.ID) + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, running.Container.DevcontainerStatus) + + require.NotNil(t, nonRunning.Container, "non-running container should have container reference") assert.Equal(t, "non-running-container", nonRunning.Container.ID) + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStopped, nonRunning.Container.DevcontainerStatus) }, }, { @@ -743,6 +754,7 @@ func TestAPI(t *testing.T) { assert.NotEmpty(t, dc2.ConfigPath) require.NotNil(t, dc2.Container) assert.Equal(t, "known-container-2", dc2.Container.ID) + assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc2.Container.DevcontainerStatus) }, }, { @@ -811,9 +823,14 @@ func TestAPI(t *testing.T) { logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + mClock := quartz.NewMock(t) + mClock.Set(time.Now()).MustWait(testutil.Context(t, testutil.WaitShort)) + tickerTrap := mClock.Trap().TickerFunc("updaterLoop") + // Setup router with the handler under test. r := chi.NewRouter() apiOptions := []agentcontainers.Option{ + agentcontainers.WithClock(mClock), agentcontainers.WithLister(tt.lister), agentcontainers.WithWatcher(watcher.NewNoop()), } @@ -838,6 +855,15 @@ func TestAPI(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) + // Make sure the ticker function has been registered + // before advancing the clock. + tickerTrap.MustWait(ctx).Release() + tickerTrap.Close() + + // Advance the clock to run the updater loop. + _, aw := mClock.AdvanceNext() + aw.MustWait(ctx) + req := httptest.NewRequest(http.MethodGet, "/devcontainers", nil). WithContext(ctx) rec := httptest.NewRecorder() From 93649166aa30146d33e4a0837b22ca5549149616 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 27 May 2025 14:02:32 +0000 Subject: [PATCH 6/6] update quartz api --- agent/agentcontainers/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index b2383226ed0c5..fb55825097190 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -857,7 +857,7 @@ func TestAPI(t *testing.T) { // Make sure the ticker function has been registered // before advancing the clock. - tickerTrap.MustWait(ctx).Release() + tickerTrap.MustWait(ctx).MustRelease(ctx) tickerTrap.Close() // Advance the clock to run the updater loop.