Skip to content

Commit 5c2022e

Browse files
DanielleMaywoodDanielleMaywood
authored
fix(coderd): fix devcontainer mock recreate flaky test (coder#19568)
Fix coder/internal#826 I wasn't able to recreate the flake, but my underlying assumption (from reading the logs we have) is that there is a race condition where the test will begin cleanup before the dev container recreation goroutine has a chance to call `devcontainer up`. I've refactored the test slightly and made it so that the test will not finish until either the context has timed out, or `Up` has been called.
1 parent bd139f3 commit 5c2022e

File tree

1 file changed

+60
-47
lines changed

1 file changed

+60
-47
lines changed

‎coderd/workspaceagents_test.go

Lines changed: 60 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1610,63 +1610,77 @@ func TestWorkspaceAgentRecreateDevcontainer(t *testing.T) {
16101610
)
16111611

16121612
for _, tc := range []struct {
1613-
name string
1614-
devcontainerID string
1615-
setupDevcontainers []codersdk.WorkspaceAgentDevcontainer
1616-
setupMock func(mccli *acmock.MockContainerCLI, mdccli *acmock.MockDevcontainerCLI) (status int)
1613+
name string
1614+
devcontainerID string
1615+
devcontainers []codersdk.WorkspaceAgentDevcontainer
1616+
containers []codersdk.WorkspaceAgentContainer
1617+
expectRecreate bool
1618+
expectErrorCode int
16171619
}{
16181620
{
1619-
name: "Recreate",
1620-
devcontainerID: devcontainerID.String(),
1621-
setupDevcontainers: []codersdk.WorkspaceAgentDevcontainer{devcontainer},
1622-
setupMock: func(mccli *acmock.MockContainerCLI, mdccli *acmock.MockDevcontainerCLI) int {
1623-
mccli.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{
1624-
Containers: []codersdk.WorkspaceAgentContainer{devContainer},
1625-
}, nil).AnyTimes()
1626-
// DetectArchitecture always returns "<none>" for this test to disable agent injection.
1627-
mccli.EXPECT().DetectArchitecture(gomock.Any(), devContainer.ID).Return("<none>", nil).AnyTimes()
1628-
mdccli.EXPECT().ReadConfig(gomock.Any(), workspaceFolder, configFile, gomock.Any()).Return(agentcontainers.DevcontainerConfig{}, nil).AnyTimes()
1629-
mdccli.EXPECT().Up(gomock.Any(), workspaceFolder, configFile, gomock.Any()).Return("someid", nil).Times(1)
1630-
return 0
1631-
},
1621+
name: "Recreate",
1622+
devcontainerID: devcontainerID.String(),
1623+
devcontainers: []codersdk.WorkspaceAgentDevcontainer{devcontainer},
1624+
containers: []codersdk.WorkspaceAgentContainer{devContainer},
1625+
expectRecreate: true,
16321626
},
16331627
{
1634-
name: "Devcontainer does not exist",
1635-
devcontainerID: uuid.NewString(),
1636-
setupDevcontainers: nil,
1637-
setupMock: func(mccli *acmock.MockContainerCLI, mdccli *acmock.MockDevcontainerCLI) int {
1638-
mccli.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{}, nil).AnyTimes()
1639-
return http.StatusNotFound
1640-
},
1628+
name: "Devcontainer does not exist",
1629+
devcontainerID: uuid.NewString(),
1630+
devcontainers: nil,
1631+
containers: []codersdk.WorkspaceAgentContainer{},
1632+
expectErrorCode: http.StatusNotFound,
16411633
},
16421634
} {
16431635
t.Run(tc.name, func(t *testing.T) {
16441636
t.Parallel()
16451637

1646-
ctrl := gomock.NewController(t)
1647-
mccli := acmock.NewMockContainerCLI(ctrl)
1648-
mdccli := acmock.NewMockDevcontainerCLI(ctrl)
1649-
wantStatus := tc.setupMock(mccli, mdccli)
1650-
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
1651-
client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{
1652-
Logger: &logger,
1653-
})
1654-
user := coderdtest.CreateFirstUser(t, client)
1655-
r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
1656-
OrganizationID: user.OrganizationID,
1657-
OwnerID: user.UserID,
1658-
}).WithAgent(func(agents []*proto.Agent) []*proto.Agent {
1659-
return agents
1660-
}).Do()
1638+
var (
1639+
ctx = testutil.Context(t, testutil.WaitLong)
1640+
mCtrl = gomock.NewController(t)
1641+
mCCLI = acmock.NewMockContainerCLI(mCtrl)
1642+
mDCCLI = acmock.NewMockDevcontainerCLI(mCtrl)
1643+
logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
1644+
client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{
1645+
Logger: &logger,
1646+
})
1647+
user = coderdtest.CreateFirstUser(t, client)
1648+
r = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
1649+
OrganizationID: user.OrganizationID,
1650+
OwnerID: user.UserID,
1651+
}).WithAgent(func(agents []*proto.Agent) []*proto.Agent {
1652+
return agents
1653+
}).Do()
1654+
)
1655+
1656+
mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{
1657+
Containers: tc.containers,
1658+
}, nil).AnyTimes()
1659+
1660+
var upCalled chan struct{}
1661+
1662+
if tc.expectRecreate {
1663+
upCalled = make(chan struct{})
1664+
1665+
// DetectArchitecture always returns "<none>" for this test to disable agent injection.
1666+
mCCLI.EXPECT().DetectArchitecture(gomock.Any(), devContainer.ID).Return("<none>", nil).AnyTimes()
1667+
mDCCLI.EXPECT().ReadConfig(gomock.Any(), workspaceFolder, configFile, gomock.Any()).Return(agentcontainers.DevcontainerConfig{}, nil).AnyTimes()
1668+
mDCCLI.EXPECT().Up(gomock.Any(), workspaceFolder, configFile, gomock.Any()).
1669+
DoAndReturn(func(_ context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
1670+
close(upCalled)
1671+
1672+
return "someid", nil
1673+
}).Times(1)
1674+
}
16611675

16621676
devcontainerAPIOptions := []agentcontainers.Option{
1663-
agentcontainers.WithContainerCLI(mccli),
1664-
agentcontainers.WithDevcontainerCLI(mdccli),
1677+
agentcontainers.WithContainerCLI(mCCLI),
1678+
agentcontainers.WithDevcontainerCLI(mDCCLI),
16651679
agentcontainers.WithWatcher(watcher.NewNoop()),
16661680
}
1667-
if tc.setupDevcontainers != nil {
1681+
if tc.devcontainers != nil {
16681682
devcontainerAPIOptions = append(devcontainerAPIOptions,
1669-
agentcontainers.WithDevcontainers(tc.setupDevcontainers, nil))
1683+
agentcontainers.WithDevcontainers(tc.devcontainers, nil))
16701684
}
16711685

16721686
_ = agenttest.New(t, client.URL, r.AgentToken, func(o *agent.Options) {
@@ -1679,15 +1693,14 @@ func TestWorkspaceAgentRecreateDevcontainer(t *testing.T) {
16791693
require.Len(t, resources[0].Agents, 1, "expected one agent")
16801694
agentID := resources[0].Agents[0].ID
16811695

1682-
ctx := testutil.Context(t, testutil.WaitLong)
1683-
16841696
_, err := client.WorkspaceAgentRecreateDevcontainer(ctx, agentID, tc.devcontainerID)
1685-
if wantStatus > 0 {
1697+
if tc.expectErrorCode > 0 {
16861698
cerr, ok := codersdk.AsError(err)
16871699
require.True(t, ok, "expected error to be a coder error")
1688-
assert.Equal(t, wantStatus, cerr.StatusCode())
1700+
assert.Equal(t, tc.expectErrorCode, cerr.StatusCode())
16891701
} else {
16901702
require.NoError(t, err, "failed to recreate devcontainer")
1703+
testutil.TryReceive(ctx, t, upCalled)
16911704
}
16921705
})
16931706
}

0 commit comments

Comments
 (0)