8000 chore: also surface errors on 'devcontainer up' · coder/coder@08576b1 · GitHub
[go: up one dir, main page]

Skip to content

Commit 08576b1

Browse files
chore: also surface errors on 'devcontainer up'
1 parent 525337f commit 08576b1

File tree

2 files changed

+196
-84
lines changed

2 files changed

+196
-84
lines changed

agent/agentcontainers/api.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,7 @@ func (api *API) CreateDevcontainer(workspaceFolder, configPath string, opts ...D
10211021
api.mu.Lock()
10221022
dc = api.knownDevcontainers[dc.WorkspaceFolder]
10231023
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusError
1024+
dc.Error = err.Error()
10241025
api.knownDevcontainers[dc.WorkspaceFolder] = dc
10251026
api.recreateErrorTimes[dc.WorkspaceFolder] = api.clock.Now("agentcontainers", "recreate", "errorTimes")
10261027
api.mu.Unlock()
@@ -1044,6 +1045,7 @@ func (api *API) CreateDevcontainer(workspaceFolder, configPath string, opts ...D
10441045
}
10451046
}
10461047
dc.Dirty = false
1048+
dc.Error = ""
10471049
api.recreateSuccessTimes[dc.WorkspaceFolder] = api.clock.Now("agentcontainers", "recreate", "successTimes")
10481050
api.knownDevcontainers[dc.WorkspaceFolder] = dc
10491051
api.mu.Unlock()

agent/agentcontainers/api_test.go

Lines changed: 194 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package agentcontainers_test
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"math/rand"
89
"net/http"
@@ -1656,107 +1657,216 @@ func TestAPI(t *testing.T) {
16561657
t.Skip("Dev Container tests are not supported on Windows (this test uses mocks but fails due to Windows paths)")
16571658
}
16581659

1659-
var (
1660-
ctx = testutil.Context(t, testutil.WaitMedium)
1661-
logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
1662-
mClock = quartz.NewMock(t)
1663-
mCCLI = acmock.NewMockContainerCLI(gomock.NewController(t))
1664-
fDCCLI = &fakeDevcontainerCLI{}
1665-
fSAC = &fakeSubAgentClient{
1666-
logger: logger.Named("fakeSubAgentClient"),
1667-
createErrC: make(chan error, 1),
1668-
}
1660+
t.Run("DuringUp", func(t *testing.T) {
1661+
t.Parallel()
16691662

1670-
containerCreatedAt = time.Now()
1671-
testContainer = codersdk.WorkspaceAgentContainer{
1672-
ID: "test-container-id",
1673-
FriendlyName: "test-container",
1674-
Image: "test-image",
1675-
Running: true,
1676-
CreatedAt: containerCreatedAt,
1677-
Labels: map[string]string{
1678-
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces",
1679-
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json",
1680-
},
1681-
}
1682-
)
1663+
var (
1664+
ctx = testutil.Context(t, testutil.WaitMedium)
1665+
logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
1666+
mClock = quartz.NewMock(t)
1667+
fCCLI = &fakeContainerCLI{arch: "<none>"}
1668+
fDCCLI = &fakeDevcontainerCLI{
1669+
upErrC: make(chan error, 1),
1670+
}
1671+
fSAC = &fakeSubAgentClient{
1672+
logger: logger.Named("fakeSubAgentClient"),
1673+
}
16831674

1684-
coderBin, err := os.Executable()
1685-
require.NoError(t, err)
1675+
testDevcontainer = codersdk.WorkspaceAgentDevcontainer{
1676+
ID: uuid.New(),
1677+
Name: "test-devcontainer",
1678+
WorkspaceFolder: "/workspaces/project",
1679+
ConfigPath: "/workspaces/project/.devcontainer/devcontainer.json",
1680+
Status: codersdk.WorkspaceAgentDevcontainerStatusStopped,
1681+
}
1682+
)
16861683

1687-
// Mock the `List` function to always return the test container.
1688-
mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{
1689-
Containers: []codersdk.WorkspaceAgentContainer{testContainer},
1690-
}, nil).AnyTimes()
1684+
mClock.Set(time.Now()).MustWait(ctx)
1685+
tickerTrap := mClock.Trap().TickerFunc("updaterLoop")
1686+
nowRecreateErrorTrap := mClock.Trap().Now("recreate", "errorTimes")
1687+
nowRecreateSuccessTrap := mClock.Trap().Now("recreate", "successTimes")
16911688

1692-
// We're going to force the container CLI to fail, which will allow us to test the
1693-
// error handling.
1694-
simulatedError := xerrors.New("simulated error")
1695-
mCCLI.EXPECT().DetectArchitecture(gomock.Any(), testContainer.ID).Return("", simulatedError).Times(1)
1689+
api := agentcontainers.NewAPI(logger,
1690+
agentcontainers.WithClock(mClock),
1691+
agentcontainers.WithContainerCLI(fCCLI),
1692+
agentcontainers.WithDevcontainerCLI(fDCCLI),
1693+
agentcontainers.WithDevcontainers(
1694+
[]codersdk.WorkspaceAgentDevcontainer{testDevcontainer},
1695+
[]codersdk.WorkspaceAgentScript{{ID: testDevcontainer.ID, LogSourceID: uuid.New()}},
1696+
),
1697+
agentcontainers.WithSubAgentClient(fSAC),
1698+
agentcontainers.WithSubAgentURL("test-subagent-url"),
1699+
agentcontainers.WithWatcher(watcher.NewNoop()),
1700+
)
1701+
api.Start()
1702+
defer func() {
1703+
close(fDCCLI.upErrC)
1704+
api.Close()
1705+
}()
16961706

1697-
mClock.Set(containerCreatedAt).MustWait(ctx)
1698-
tickerTrap := mClock.Trap().TickerFunc("updaterLoop")
1707+
r := chi.NewRouter()
1708+
r.Mount("/", api.Routes())
16991709

1700-
api := agentcontainers.NewAPI(logger,
1701-
agentcontainers.WithClock(mClock),
1702-
agentcontainers.WithContainerCLI(mCCLI),
1703-
agentcontainers.WithDevcontainerCLI(fDCCLI),
1704-
agentcontainers.WithSubAgentClient(fSAC),
1705-
agentcontainers.WithSubAgentURL("test-subagent-url"),
1706-
agentcontainers.WithWatcher(watcher.NewNoop()),
1707-
)
1708-
api.Start()
1709-
defer func() {
1710-
close(fSAC.createErrC)
1711-
api.Close()
1712-
}()
1710+
tickerTrap.MustWait(ctx).MustRelease(ctx)
1711+
tickerTrap.Close()
17131712

1714-
r := chi.NewRouter()
1715-
r.Mount("/", api.Routes())
1713+
// Given: We send a 'recreate' request.
1714+
req := httptest.NewRequest(http.MethodPost, "/devcontainers/"+testDevcontainer.ID.String()+"/recreate", nil)
1715+
rec := httptest.NewRecorder()
1716+
r.ServeHTTP(rec, req)
1717+
require.Equal(t, http.StatusAccepted, rec.Code)
17161718

1717-
// Given: We allow an attempt at creation to occur.
1718-
tickerTrap.MustWait(ctx).MustRelease(ctx)
1719-
tickerTrap.Close()
1719+
// Given: We simulate an error running `devcontainer up`
1720+
simulatedError := errors.New("simulated error")
1721+
testutil.RequireSend(ctx, t, fDCCLI.upErrC, simulatedError)
17201722

1721-
req := httptest.NewRequest(http.MethodGet, "/", nil)
1722-
rec := httptest.NewRecorder()
1723-
r.ServeHTTP(rec, req)
1724-
require.Equal(t, http.StatusOK, rec.Code)
1723+
nowRecreateErrorTrap.MustWait(ctx).MustRelease(ctx)
1724+
nowRecreateErrorTrap.Close()
17251725

1726-
var response codersdk.WorkspaceAgentListContainersResponse
1727-
err = json.NewDecoder(rec.Body).Decode(&response)
1728-
require.NoError(t, err)
1726+
req = httptest.NewRequest(http.MethodGet, "/", nil)
1727+
rec = httptest.NewRecorder()
1728+
r.ServeHTTP(rec, req)
1729+
require.Equal(t, http.StatusOK, rec.Code)
17291730

1730-
// Then: We expect that there will be an error associated with the devcontainer.
1731-
require.Len(t, response.Devcontainers, 1)
1732-
require.Equal(t, "detect architecture: simulated error", response.Devcontainers[0].Error)
1731+
var response codersdk.WorkspaceAgentListContainersResponse
1732+
err := json.NewDecoder(rec.Body).Decode(&response)
1733+
require.NoError(t, err)
17331734

1734-
gomock.InOrder(
1735-
mCCLI.EXPECT().DetectArchitecture(gomock.Any(), testContainer.ID).Return(runtime.GOARCH, nil),
1736-
mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "mkdir", "-p", "/.coder-agent").Return(nil, nil),
1737-
mCCLI.EXPECT().Copy(gomock.Any(), testContainer.ID, coderBin, "/.coder-agent/coder").Return(nil),
1738-
mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "chmod", "0755", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil),
1739-
mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "/bin/sh", "-c", "chown $(id -u):$(id -g) /.coder-agent/coder").Return(nil, nil),
1740-
)
1735+
// Then: We expect that there will be an error associated with the devcontainer.
1736+
require.Len(t, response.Devcontainers, 1)
1737+
require.Equal(t, "simulated error", response.Devcontainers[0].Error)
17411738

1742-
// Given: We allow creation to succeed.
1743-
testutil.RequireSend(ctx, t, fSAC.createErrC, nil)
1739+
// Given: We send another 'recreate' request.
1740+
req = httptest.NewRequest(http.MethodPost, "/devcontainers/"+testDevcontainer.ID.String()+"/recreate", nil)
1741+
rec = httptest.NewRecorder()
1742+
r.ServeHTTP(rec, req)
1743+
require.Equal(t, http.StatusAccepted, rec.Code)
17441744

1745-
_, aw := mClock.AdvanceNext()
1746-
aw.MustWait(ctx)
1745+
// Given: We allow `devcontainer up` to succeed.
1746+
testutil.RequireSend(ctx, t, fDCCLI.upErrC, nil)
17471747

1748-
req = httptest.NewRequest(http.MethodGet, "/", nil)
1749-
rec = httptest.NewRecorder()
1750-
r.ServeHTTP(rec, req)
1751-
require.Equal(t, http.StatusOK, rec.Code)
1748+
nowRecreateSuccessTrap.MustWait(ctx).MustRelease(ctx)
1749+
nowRecreateSuccessTrap.Close()
17521750

1753-
response = codersdk.WorkspaceAgentListContainersResponse{}
1754-
err = json.NewDecoder(rec.Body).Decode(&response)
1755-
require.NoError(t, err)
1751+
req = httptest.NewRequest(http.MethodGet, "/", nil)
1752+
rec = httptest.NewRecorder()
1753+
r.ServeHTTP(rec, req)
1754+
require.Equal(t, http.StatusOK, rec.Code)
17561755

1757-
// Then: We expect that the error will be gone
1758-
require.Len(t, response.Devcontainers, 1)
1759-
require.Equal(t, "", response.Devcontainers[0].Error)
1756+
response = codersdk.WorkspaceAgentListContainersResponse{}
1757+
err = json.NewDecoder(rec.Body).Decode(&response)
1758+
require.NoError(t, err)
1759+
1760+
// Then: We expect that there will be no error associated with the devcontainer.
1761+
require.Len(t, response.Devcontainers, 1)
1762+
require.Equal(t, "", response.Devcontainers[0].Error)
1763+
})
1764+
1765+
t.Run("DuringInjection", func(t *testing.T) {
1766+
t.Parallel()
1767+
1768+
var (
1769+
ctx = testutil.Context(t, testutil.WaitMedium)
1770+
logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
1771+
mClock = quartz.NewMock(t)
1772+
mCCLI = acmock.NewMockContainerCLI(gomock.NewController(t))
1773+
fDCCLI = &fakeDevcontainerCLI{}
1774+
fSAC = &fakeSubAgentClient{
1775+
logger: logger.Named("fakeSubAgentClient"),
1776+
createErrC: make(chan error, 1),
1777+
}
1778+
1779+
containerCreatedAt = time.Now()
1780+
testContainer = codersdk.WorkspaceAgentContainer{
1781+
ID: "test-container-id",
1782+
FriendlyName: "test-container",
1783+
Image: "test-image",
1784+
Running: true,
1785+
CreatedAt: containerCreatedAt,
1786+
Labels: map[string]string{
1787+
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces",
1788+
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json",
1789+
},
1790+
}
1791+
)
1792+
1793+
coderBin, err := os.Executable()
1794+
require.NoError(t, err)
1795+
1796+
// Mock the `List` function to always return the test container.
1797+
mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{
1798+
Containers: []codersdk.WorkspaceAgentContainer{testContainer},
1799+
}, nil).AnyTimes()
1800+
1801+
// We're going to force the container CLI to fail, which will allow us to test the
1802+
// error handling.
1803+
simulatedError := xerrors.New("simulated error")
1804+
mCCLI.EXPECT().DetectArchitecture(gomock.Any(), testContainer.ID).Return("", simulatedError).Times(1)
1805+
1806+
mClock.Set(containerCreatedAt).MustWait(ctx)
1807+
tickerTrap := mClock.Trap().TickerFunc("updaterLoop")
1808+
1809+
api := agentcontainers.NewAPI(logger,
1810+
agentcontainers.WithClock(mClock),
1811+
agentcontainers.WithContainerCLI(mCCLI),
1812+
agentcontainers.WithDevcontainerCLI(fDCCLI),
1813+
agentcontainers.WithSubAgentClient(fSAC),
1814+
agentcontainers.WithSubAgentURL("test-subagent-url"),
1815+
agentcontainers.WithWatcher(watcher.NewNoop()),
1816+
)
1817+
api.Start()
1818+
defer func() {
1819+
close(fSAC.createErrC)
1820+
api.Close()
1821+
}()
1822+
1823+
r := chi.NewRouter()
1824+
r.Mount("/", api.Routes())
1825+
1826+
// Given: We allow an attempt at creation to occur.
1827+
tickerTrap.MustWait(ctx).MustRelease(ctx)
1828+
tickerTrap.Close()
1829+
1830+
req := httptest.NewRequest(http.MethodGet, "/", nil)
1831+
rec := httptest.NewRecorder()
1832+
r.ServeHTTP(rec, req)
1833+
require.Equal(t, http.StatusOK, rec.Code)
1834+
1835+
var response codersdk.WorkspaceAgentListContainersResponse
1836+
err = json.NewDecoder(rec.Body).Decode(&response)
1837+
require.NoError(t, err)
1838+
1839+
// Then: We expect that there will be an error associated with the devcontainer.
1840+
require.Len(t, response.Devcontainers, 1)
1841+
require.Equal(t, "detect architecture: simulated error", response.Devcontainers[0].Error)
1842+
1843+
gomock.InOrder(
1844+
mCCLI.EXPECT().DetectArchitecture(gomock.Any(), testContainer.ID).Return(runtime.GOARCH, nil),
1845+
mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "mkdir", "-p", "/.coder-agent").Return(nil, nil),
1846+
mCCLI.EXPECT().Copy(gomock.Any(), testContainer.ID, coderBin, "/.coder-agent/coder").Return(nil),
1847+
mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "chmod", "0755", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil),
1848+
mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "/bin/sh", "-c", "chown $(id -u):$(id -g) /.coder-agent/coder").Return(nil, nil),
1849+
)
1850+
1851+
// Given: We allow creation to succeed.
1852+
testutil.RequireSend(ctx, t, fSAC.createErrC, nil)
1853+
1854+
_, aw := mClock.AdvanceNext()
1855+
aw.MustWait(ctx)
1856+
1857+
req = httptest.NewRequest(http.MethodGet, "/", nil)
1858+
rec = httptest.NewRecorder()
1859+
r.ServeHTTP(rec, req)
1860+
require.Equal(t, http.StatusOK, rec.Code)
1861+
1862+
response = codersdk.WorkspaceAgentListContainersResponse{}
1863+
err = json.NewDecoder(rec.Body).Decode(&response)
1864+
require.NoError(t, err)
1865+
1866+
// Then: We expect that the error will be gone
1867+
require.Len(t, response.Devcontainers, 1)
1868+
require.Equal(t, "", response.Devcontainers[0].Error)
1869+
})
17601870
})
17611871

17621872
t.Run("Create", func(t *testing.T) {

0 commit comments

Comments
 (0)
0