Skip to content

Commit 0782534

Browse files
committed
Fix builder activity detection and busy-create response
1 parent 156c878 commit 0782534

4 files changed

Lines changed: 75 additions & 7 deletions

File tree

cmd/api/api/builds.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,11 @@ func (s *ApiService) CreateBuild(ctx context.Context, request oapi.CreateBuildRe
276276
Code: "not_found",
277277
Message: "builder not found",
278278
}, nil
279+
case errors.Is(err, builders.ErrInUse):
280+
return oapi.CreateBuild400JSONResponse{
281+
Code: "conflict",
282+
Message: "builder is not ready",
283+
}, nil
279284
default:
280285
log.ErrorContext(ctx, "failed to create build", "error", err)
281286
return oapi.CreateBuild500JSONResponse{

cmd/api/api/builds_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package api
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"mime/multipart"
7+
"testing"
8+
9+
"github.com/kernel/hypeman/lib/builders"
10+
"github.com/kernel/hypeman/lib/builds"
11+
"github.com/kernel/hypeman/lib/oapi"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
type createBuildErrManager struct {
17+
builds.Manager
18+
err error
19+
}
20+
21+
func (m *createBuildErrManager) CreateBuild(context.Context, builds.CreateBuildRequest, []byte) (*builds.Build, error) {
22+
return nil, m.err
23+
}
24+
25+
func TestCreateBuild_ReturnsBadRequestWhenBuilderBusy(t *testing.T) {
26+
svc := newTestService(t)
27+
svc.BuildManager = &createBuildErrManager{
28+
Manager: svc.BuildManager,
29+
err: builders.ErrInUse,
30+
}
31+
32+
var body bytes.Buffer
33+
writer := multipart.NewWriter(&body)
34+
sourcePart, err := writer.CreateFormFile("source", "source.tar.gz")
35+
require.NoError(t, err)
36+
_, err = sourcePart.Write([]byte("fake-source-data"))
37+
require.NoError(t, err)
38+
require.NoError(t, writer.Close())
39+
40+
resp, err := svc.CreateBuild(ctx(), oapi.CreateBuildRequestObject{
41+
Body: multipart.NewReader(bytes.NewReader(body.Bytes()), writer.Boundary()),
42+
})
43+
require.NoError(t, err)
44+
45+
badReq, ok := resp.(oapi.CreateBuild400JSONResponse)
46+
require.True(t, ok, "expected a 400 when builder is busy")
47+
assert.Equal(t, "conflict", badReq.Code)
48+
assert.Equal(t, "builder is not ready", badReq.Message)
49+
}

lib/builds/builder_disk_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,3 +459,22 @@ func TestBuilderHasBuilds_CountsDiskPendingBeforeRecovery(t *testing.T) {
459459
mgr.RecoverPendingBuilds()
460460
assert.False(t, mgr.BuilderHasBuilds("builder-a"))
461461
}
462+
463+
// TestBuilderHasBuilds_CountsDiskBuildingAfterRecovery verifies a build that
464+
// is still building on disk remains visible to builder activity checks even
465+
// after startup recovery has completed.
466+
func TestBuilderHasBuilds_CountsDiskBuildingAfterRecovery(t *testing.T) {
467+
mgr, _, _, tempDir := setupTestManager(t)
468+
defer os.RemoveAll(tempDir)
469+
470+
require.NoError(t, writeMetadata(mgr.paths, &buildMetadata{
471+
ID: "build-post-vm",
472+
Status: StatusBuilding,
473+
Request: &CreateBuildRequest{BuilderID: "builder-a"},
474+
CreatedAt: time.Now(),
475+
}))
476+
mgr.pendingRecovered.Store(true)
477+
478+
assert.True(t, mgr.BuilderHasBuilds("builder-a"), "building metadata must count after recovery")
479+
assert.False(t, mgr.BuilderHasBuilds("builder-b"))
480+
}

lib/builds/manager.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,7 @@ type manager struct {
148148
createMu sync.Mutex
149149
builderReady atomic.Bool
150150
// pendingRecovered is set once RecoverPendingBuilds has re-enqueued
151-
// persisted pending builds; until then BuilderHasBuilds also scans
152-
// disk so delete, prune, and the idle reaper see them.
151+
// persisted pending builds.
153152
pendingRecovered atomic.Bool
154153

155154
// Status subscription system for SSE streaming
@@ -599,15 +598,11 @@ func (m *manager) QueuedBuildsForBuilder(builderID string) []string {
599598
}
600599

601600
// BuilderHasBuilds reports whether any build targeting the builder is
602-
// queued or running. Until startup recovery completes, persisted pending
603-
// builds are not in the queue yet, so they are matched on disk.
601+
// queued or running, including persisted queued/building metadata.
604602
func (m *manager) BuilderHasBuilds(builderID string) bool {
605603
if m.queue.HasSerialKey(builderID) {
606604
return true
607605
}
608-
if m.pendingRecovered.Load() {
609-
return false
610-
}
611606
pending, err := listPendingBuilds(m.paths)
612607
if err != nil {
613608
// Fail closed: blocking a delete is recoverable, deleting a

0 commit comments

Comments
 (0)