Skip to content

Register builds before recording build history #3726

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.

Sign up for GitHub

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

Merged
tonistiigi merged 1 commit into from
Mar 22, 2023
Merged

Register builds before recording build history #3726

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions solver/llbsolver/solver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,15 +421,6 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro

if internal {
defer j.CloseProgress()
} else {
rec, err1 := s.recordBuildHistory(ctx, id, req, exp, j)
if err != nil {
defer j.CloseProgress()
return nil, err1
}
defer func() {
err = rec(resProv, descref, err)
}()
}

set, err := entitlements.WhiteList(ent, supportedEntitlements(s.entitlements))
Expand All @@ -445,14 +436,32 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
j.SessionID = sessionID

br := s.bridge(j)
var fwd gateway.LLBBridgeForwarder
if s.gatewayForwarder != nil && req.Definition == nil && req.Frontend == "" {
fwd := gateway.NewBridgeForwarder(ctx, br, s.workerController, req.FrontendInputs, sessionID, s.sm)
fwd = gateway.NewBridgeForwarder(ctx, br, s.workerController, req.FrontendInputs, sessionID, s.sm)
defer fwd.Discard()
// Register build before calling s.recordBuildHistory, because
// s.recordBuildHistory can block for several seconds on
// LeaseManager calls, and there is a fixed 3s timeout in
// GatewayForwarder on build registration.
if err := s.gatewayForwarder.RegisterBuild(ctx, id, fwd); err != nil {
return nil, err
}
defer s.gatewayForwarder.UnregisterBuild(ctx, id)
}

if !internal {
rec, err1 := s.recordBuildHistory(ctx, id, req, exp, j)
Copy link
Copy Markdown
Member

@tonistiigi tonistiigi Mar 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there is a typo (refactor bug) with this err1/err switch.

Copy link
Copy Markdown
Collaborator Author

@aaronlehmann aaronlehmann Mar 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing this. Looks like this was present before this PR. I've gone ahead and fixed the check on the next line (which I think is what's intended...).

if err1 != nil {
defer j.CloseProgress()
return nil, err1
}
defer func() {
err = rec(resProv, descref, err)
}()
}

if fwd != nil {
var err error
select {
case <-fwd.Done():
Expand Down