From 23a114a9773f10ed8b69a11361af31cd0769693e Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 4 Oct 2017 22:31:18 -0700 Subject: [PATCH 1/2] solver: simplify solver public api Signed-off-by: Tonis Tiigi --- control/control.go | 8 +- frontend/dockerfile/dockerfile.go | 8 +- frontend/frontend.go | 8 +- frontend/gateway/gateway.go | 15 +++- solver/solver.go | 121 ++++++++++++++---------------- solver/vertex.go | 16 +++- 6 files changed, 102 insertions(+), 74 deletions(-) diff --git a/control/control.go b/control/control.go index e5d51fc4..9c40a589 100644 --- a/control/control.go +++ b/control/control.go @@ -45,6 +45,7 @@ func NewController(opt Opt) (*Controller, error) { Worker: opt.Worker, InstructionCache: opt.InstructionCache, ImageSource: opt.ImageSource, + Frontends: opt.Frontends, }), } return c, nil @@ -105,7 +106,12 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (* } } - if err := c.solver.Solve(ctx, req.Ref, frontend, req.Definition, expi, req.FrontendAttrs, c.opt.Frontends); err != nil { + if err := c.solver.Solve(ctx, req.Ref, solver.SolveRequest{ + Frontend: frontend, + Definition: req.Definition, + Exporter: expi, + FrontendOpt: req.FrontendAttrs, + }); err != nil { return nil, err } return &controlapi.SolveResponse{}, nil diff --git a/frontend/dockerfile/dockerfile.go b/frontend/dockerfile/dockerfile.go index b1d30a5c..fd68c5bf 100644 --- a/frontend/dockerfile/dockerfile.go +++ b/frontend/dockerfile/dockerfile.go @@ -53,7 +53,9 @@ func (f *dfFrontend) Solve(ctx context.Context, llbBridge frontend.FrontendLLBBr return nil, nil, err } - ref, _, err := llbBridge.Solve(ctx, def.ToPB(), "", nil) + ref, _, err := llbBridge.Solve(ctx, frontend.SolveRequest{ + Definition: def.ToPB(), + }) if err != nil { return nil, nil, err } @@ -112,7 +114,9 @@ func (f *dfFrontend) Solve(ctx context.Context, llbBridge frontend.FrontendLLBBr if err != nil { return nil, nil, err } - retRef, _, err = llbBridge.Solve(ctx, def.ToPB(), "", nil) + retRef, _, err = llbBridge.Solve(ctx, frontend.SolveRequest{ + Definition: def.ToPB(), + }) if err != nil { return nil, nil, err } diff --git a/frontend/frontend.go b/frontend/frontend.go index 0cebb027..29765617 100644 --- a/frontend/frontend.go +++ b/frontend/frontend.go @@ -15,7 +15,13 @@ type Frontend interface { } type FrontendLLBBridge interface { - Solve(ctx context.Context, def *pb.Definition, frontend string, opts map[string]string) (cache.ImmutableRef, map[string][]byte, error) + Solve(ctx context.Context, req SolveRequest) (cache.ImmutableRef, map[string][]byte, error) ResolveImageConfig(ctx context.Context, ref string) (digest.Digest, []byte, error) Exec(ctx context.Context, meta worker.Meta, rootfs cache.ImmutableRef, stdin io.ReadCloser, stdout, stderr io.WriteCloser) error } + +type SolveRequest struct { + Definition *pb.Definition + Frontend string + FrontendOpt map[string]string +} diff --git a/frontend/gateway/gateway.go b/frontend/gateway/gateway.go index 299ddd95..5b1f877e 100644 --- a/frontend/gateway/gateway.go +++ b/frontend/gateway/gateway.go @@ -67,7 +67,11 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten var rootFS cache.ImmutableRef if isDevel { - ref, exp, err := llbBridge.Solve(session.NewContext(ctx, "gateway:"+sid), nil, source, filterPrefix(opts, "gateway-")) + ref, exp, err := llbBridge.Solve(session.NewContext(ctx, "gateway:"+sid), + frontend.SolveRequest{ + Frontend: source, + FrontendOpt: filterPrefix(opts, "gateway-"), + }) if err != nil { return nil, nil, err } @@ -105,7 +109,9 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten return nil, nil, err } - ref, _, err := llbBridge.Solve(ctx, def.ToPB(), "", nil) + ref, _, err := llbBridge.Solve(ctx, frontend.SolveRequest{ + Definition: def.ToPB(), + }) if err != nil { return nil, nil, err } @@ -240,7 +246,10 @@ func (lbf *llbBrideForwarder) ResolveImageConfig(ctx context.Context, req *pb.Re } func (lbf *llbBrideForwarder) Solve(ctx context.Context, req *pb.SolveRequest) (*pb.SolveResponse, error) { - ref, expResp, err := lbf.llbBridge.Solve(ctx, req.Definition, req.Frontend, nil) + ref, expResp, err := lbf.llbBridge.Solve(ctx, frontend.SolveRequest{ + Definition: req.Definition, + Frontend: req.Frontend, + }) if err != nil { return nil, err } diff --git a/solver/solver.go b/solver/solver.go index 8b2b08b0..2946316a 100644 --- a/solver/solver.go +++ b/solver/solver.go @@ -11,7 +11,6 @@ import ( "github.com/moby/buildkit/client" "github.com/moby/buildkit/exporter" "github.com/moby/buildkit/frontend" - "github.com/moby/buildkit/identity" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/source" "github.com/moby/buildkit/util/bgfunc" @@ -26,10 +25,11 @@ import ( type LLBOpt struct { SourceManager *source.Manager - CacheManager cache.Manager // TODO: this shouldn't be needed before instruction cache + CacheManager cache.Manager Worker worker.Worker InstructionCache InstructionCache ImageSource source.Source + Frontends map[string]frontend.Frontend // used by nested invocations } func NewLLBSolver(opt LLBOpt) *Solver { @@ -45,7 +45,7 @@ func NewLLBSolver(opt LLBOpt) *Solver { default: return nil, nil } - }, opt.InstructionCache, opt.ImageSource, opt.Worker, opt.CacheManager) + }, opt.InstructionCache, opt.ImageSource, opt.Worker, opt.CacheManager, opt.Frontends) return s } @@ -79,46 +79,55 @@ type Solver struct { imageSource source.Source worker worker.Worker cm cache.Manager // TODO: remove with immutableRef.New() + frontends map[string]frontend.Frontend } -func New(resolve ResolveOpFunc, cache InstructionCache, imageSource source.Source, worker worker.Worker, cm cache.Manager) *Solver { - return &Solver{resolve: resolve, jobs: newJobList(), cache: cache, imageSource: imageSource, worker: worker, cm: cm} +func New(resolve ResolveOpFunc, cache InstructionCache, imageSource source.Source, worker worker.Worker, cm cache.Manager, f map[string]frontend.Frontend) *Solver { + return &Solver{resolve: resolve, jobs: newJobList(), cache: cache, imageSource: imageSource, worker: worker, cm: cm, frontends: f} } -func (s *Solver) Solve(ctx context.Context, id string, f frontend.Frontend, def *pb.Definition, exp exporter.ExporterInstance, frontendOpt map[string]string, allFrontends map[string]frontend.Frontend) error { +type SolveRequest struct { + Definition *pb.Definition + Frontend frontend.Frontend + Exporter exporter.ExporterInstance + FrontendOpt map[string]string +} + +func (s *Solver) solve(ctx context.Context, j *job, req SolveRequest) (Reference, map[string][]byte, error) { + if req.Definition == nil { + if req.Frontend == nil { + return nil, nil, errors.Errorf("invalid request: no definition nor frontend") + } + return req.Frontend.Solve(ctx, s.llbBridge(j), req.FrontendOpt) + } + + inp, err := j.load(req.Definition, s.resolve) + if err != nil { + return nil, nil, err + } + ref, err := j.getRef(ctx, inp.Vertex.(*vertex), inp.Index) + return ref, nil, err +} + +func (s *Solver) llbBridge(j *job) *llbBridge { + return &llbBridge{job: j, Solver: s, resolveImageConfig: s.imageSource.(resolveImageConfig)} +} + +func (s *Solver) Solve(ctx context.Context, id string, req SolveRequest) error { ctx, cancel := context.WithCancel(ctx) defer cancel() pr, ctx, closeProgressWriter := progress.NewContext(ctx) - defer closeProgressWriter() + // register a build job. vertex needs to be loaded to a job to run ctx, j, err := s.jobs.new(ctx, id, pr, s.cache) if err != nil { return err } - var ref Reference - var exporterOpt map[string][]byte - if def != nil { - var inp *Input - inp, err = j.load(def, s.resolve) - if err != nil { - j.discard() - return err - } - ref, err = j.getRef(ctx, inp.Vertex.(*vertex), inp.Index) - } else { - ref, exporterOpt, err = f.Solve(ctx, &llbBridge{ - worker: s.worker, - job: j, - cm: s.cm, - resolveOp: s.resolve, - resolveImageConfig: s.imageSource.(resolveImageConfig), - allFrontends: allFrontends, - }, frontendOpt) - } - j.discard() + ref, exporterOpt, err := s.solve(ctx, j, req) + defer j.discard() if err != nil { return err } @@ -135,19 +144,10 @@ func (s *Solver) Solve(ctx context.Context, id string, f frontend.Frontend, def return err } - if exp != nil { - v := client.Vertex{ - Digest: digest.FromBytes([]byte(identity.NewID())), - Name: exp.Name(), - } - notifyStarted(ctx, &v) - pw, _, ctx := progress.FromContext(ctx, progress.WithMetadata("vertex", v.Digest)) - defer pw.Close() - err := exp.Export(ctx, immutable, exporterOpt) - notifyCompleted(ctx, &v, err) - if err != nil { - return err - } + if exp := req.Exporter; exp != nil { + return inVertexContext(ctx, exp.Name(), func(ctx context.Context) error { + return exp.Export(ctx, immutable, exporterOpt) + }) } return err } @@ -571,39 +571,29 @@ type VertexResult struct { // llbBridge is an helper used by frontends type llbBridge struct { + *Solver + job *job resolveImageConfig - job *job - resolveOp ResolveOpFunc - worker worker.Worker - allFrontends map[string]frontend.Frontend - cm cache.Manager } type resolveImageConfig interface { ResolveImageConfig(ctx context.Context, ref string) (digest.Digest, []byte, error) } -func (s *llbBridge) Solve(ctx context.Context, def *pb.Definition, frontend string, opts map[string]string) (cache.ImmutableRef, map[string][]byte, error) { - if def == nil { - f, ok := s.allFrontends[frontend] +func (s *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest) (cache.ImmutableRef, map[string][]byte, error) { + var f frontend.Frontend + if req.Frontend != "" { + var ok bool + f, ok = s.frontends[req.Frontend] if !ok { - return nil, nil, errors.Errorf("invalid frontend: %s", frontend) + return nil, nil, errors.Errorf("invalid frontend: %s", req.Frontend) } - ref, exporterOpt, err := f.Solve(ctx, s, opts) - if err != nil { - return nil, nil, err - } - immutable, ok := toImmutableRef(ref) - if !ok { - return nil, nil, errors.Errorf("invalid reference for exporting: %T", ref) - } - return immutable, exporterOpt, nil } - inp, err := s.job.load(def, s.resolveOp) - if err != nil { - return nil, nil, err - } - ref, err := s.job.getRef(ctx, inp.Vertex.(*vertex), inp.Index) + ref, exp, err := s.solve(ctx, s.job, SolveRequest{ + Definition: req.Definition, + Frontend: f, + FrontendOpt: req.FrontendOpt, + }) if err != nil { return nil, nil, err } @@ -611,8 +601,7 @@ func (s *llbBridge) Solve(ctx context.Context, def *pb.Definition, frontend stri if !ok { return nil, nil, errors.Errorf("invalid reference for exporting: %T", ref) } - - return immutable, nil, nil + return immutable, exp, nil } func (s *llbBridge) Exec(ctx context.Context, meta worker.Meta, rootFS cache.ImmutableRef, stdin io.ReadCloser, stdout, stderr io.WriteCloser) error { diff --git a/solver/vertex.go b/solver/vertex.go index 5978807b..fcd0a73b 100644 --- a/solver/vertex.go +++ b/solver/vertex.go @@ -1,13 +1,14 @@ package solver import ( - "context" "sync" "time" "github.com/moby/buildkit/client" + "github.com/moby/buildkit/identity" "github.com/moby/buildkit/util/progress" digest "github.com/opencontainers/go-digest" + "golang.org/x/net/context" ) // Vertex is one node in the build graph @@ -112,3 +113,16 @@ func notifyCompleted(ctx context.Context, v *client.Vertex, err error) { } pw.Write(v.Digest.String(), *v) } + +func inVertexContext(ctx context.Context, name string, f func(ctx context.Context) error) error { + v := client.Vertex{ + Digest: digest.FromBytes([]byte(identity.NewID())), + Name: name, + } + pw, _, ctx := progress.FromContext(ctx, progress.WithMetadata("vertex", v.Digest)) + notifyStarted(ctx, &v) + defer pw.Close() + err := f(ctx) + notifyCompleted(ctx, &v, err) + return err +} From 59910481ca2e780639cb60684940dcd82077cb43 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 5 Oct 2017 13:42:53 -0700 Subject: [PATCH 2/2] solver: make nested build use solverequest Signed-off-by: Tonis Tiigi --- client/solve.go | 39 ++++++++++++++++++++++----------------- hack/test-integration | 2 +- solver/build.go | 6 +++--- solver/jobs.go | 24 ------------------------ solver/solver.go | 24 ++++++++++++++++++++++-- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/client/solve.go b/client/solve.go index 32b38a14..41b8f35d 100644 --- a/client/solve.go +++ b/client/solve.go @@ -46,7 +46,7 @@ func (c *Client) Solve(ctx context.Context, def *llb.Definition, opt SolveOpt, s return errors.Errorf("invalid definition for frontend %s", opt.Frontend) } - syncedDirs, err := prepareSyncedDirs(def.Def, opt.LocalDirs) + syncedDirs, err := prepareSyncedDirs(def, opt.LocalDirs) if err != nil { return err } @@ -87,9 +87,13 @@ func (c *Client) Solve(ctx context.Context, def *llb.Definition, opt SolveOpt, s logrus.Debugf("stopping session") s.Close() }() + var pbd *pb.Definition + if def != nil { + pbd = def.ToPB() + } _, err = c.controlClient().Solve(ctx, &controlapi.SolveRequest{ Ref: ref, - Definition: def.ToPB(), + Definition: pbd, Exporter: opt.Exporter, ExporterAttrs: opt.ExporterAttrs, Session: s.ID(), @@ -158,7 +162,7 @@ func (c *Client) Solve(ctx context.Context, def *llb.Definition, opt SolveOpt, s return eg.Wait() } -func prepareSyncedDirs(defs [][]byte, localDirs map[string]string) ([]filesync.SyncedDir, error) { +func prepareSyncedDirs(def *llb.Definition, localDirs map[string]string) ([]filesync.SyncedDir, error) { for _, d := range localDirs { fi, err := os.Stat(d) if err != nil { @@ -169,24 +173,25 @@ func prepareSyncedDirs(defs [][]byte, localDirs map[string]string) ([]filesync.S } } dirs := make([]filesync.SyncedDir, 0, len(localDirs)) - if len(defs) == 0 { + if def == nil { for name, d := range localDirs { dirs = append(dirs, filesync.SyncedDir{Name: name, Dir: d}) } - } - for _, dt := range defs { - var op pb.Op - if err := (&op).Unmarshal(dt); err != nil { - return nil, errors.Wrap(err, "failed to parse llb proto op") - } - if src := op.GetSource(); src != nil { - if strings.HasPrefix(src.Identifier, "local://") { // TODO: just make a type property - name := strings.TrimPrefix(src.Identifier, "local://") - d, ok := localDirs[name] - if !ok { - return nil, errors.Errorf("local directory %s not enabled", name) + } else { + for _, dt := range def.Def { + var op pb.Op + if err := (&op).Unmarshal(dt); err != nil { + return nil, errors.Wrap(err, "failed to parse llb proto op") + } + if src := op.GetSource(); src != nil { + if strings.HasPrefix(src.Identifier, "local://") { // TODO: just make a type property + name := strings.TrimPrefix(src.Identifier, "local://") + d, ok := localDirs[name] + if !ok { + return nil, errors.Errorf("local directory %s not enabled", name) + } + dirs = append(dirs, filesync.SyncedDir{Name: name, Dir: d}) // TODO: excludes } - dirs = append(dirs, filesync.SyncedDir{Name: name, Dir: d}) // TODO: excludes } } } diff --git a/hack/test-integration b/hack/test-integration index f4051567..966cf2f5 100755 --- a/hack/test-integration +++ b/hack/test-integration @@ -5,4 +5,4 @@ set -eu -o pipefail -x # update this to iidfile after 17.06 docker build -t buildkit:test --target integration-tests -f ./hack/dockerfiles/test.Dockerfile --force-rm . docker run --rm -v /tmp --privileged buildkit:test go test -tags 'containerd standalone' ./client - +docker run --rm buildkit:test go build ./frontend/gateway/client diff --git a/solver/build.go b/solver/build.go index ab725951..36eea2b7 100644 --- a/solver/build.go +++ b/solver/build.go @@ -104,13 +104,13 @@ func (b *buildOp) Run(ctx context.Context, inputs []Reference) (outputs []Refere f.Close() return nil, err } - defPB := def.ToPB() - f.Close() lm.Unmount() lm = nil - newref, err := b.s.loadAndSolve(ctx, b.v.Digest(), defPB) + newref, err := b.s.subBuild(ctx, b.v.Digest(), SolveRequest{ + Definition: def.ToPB(), + }) if err != nil { return nil, err } diff --git a/solver/jobs.go b/solver/jobs.go index 6c86f3e0..9283b1f3 100644 --- a/solver/jobs.go +++ b/solver/jobs.go @@ -91,30 +91,6 @@ func (jl *jobList) get(id string) (*job, error) { } } -func (jl *jobList) loadAndSolve(ctx context.Context, dgst digest.Digest, def *pb.Definition, f ResolveOpFunc, cache InstructionCache) (Reference, error) { - jl.mu.Lock() - - st, ok := jl.actives[dgst] - if !ok { - jl.mu.Unlock() - return nil, errors.Errorf("no such parent vertex: %v", dgst) - } - - var inp *Input - for j := range st.jobs { - var err error - inp, err = j.loadInternal(def, f) - if err != nil { - jl.mu.Unlock() - return nil, err - } - } - st = jl.actives[inp.Vertex.Digest()] - jl.mu.Unlock() - - return getRef(st.solver, ctx, inp.Vertex.(*vertex), inp.Index, cache) // TODO: combine to pass single input -} - type job struct { l *jobList pr *progress.MultiReader diff --git a/solver/solver.go b/solver/solver.go index 2946316a..1c7e0be7 100644 --- a/solver/solver.go +++ b/solver/solver.go @@ -161,8 +161,28 @@ func (s *Solver) Status(ctx context.Context, id string, statusChan chan *client. return j.pipe(ctx, statusChan) } -func (s *Solver) loadAndSolve(ctx context.Context, dgst digest.Digest, def *pb.Definition) (Reference, error) { - return s.jobs.loadAndSolve(ctx, dgst, def, s.resolve, s.cache) +func (s *Solver) subBuild(ctx context.Context, dgst digest.Digest, req SolveRequest) (Reference, error) { + jl := s.jobs + jl.mu.Lock() + st, ok := jl.actives[dgst] + if !ok { + jl.mu.Unlock() + return nil, errors.Errorf("no such parent vertex: %v", dgst) + } + + var inp *Input + for j := range st.jobs { + var err error + inp, err = j.loadInternal(req.Definition, s.resolve) + if err != nil { + jl.mu.Unlock() + return nil, err + } + } + st = jl.actives[inp.Vertex.Digest()] + jl.mu.Unlock() + + return getRef(st.solver, ctx, inp.Vertex.(*vertex), inp.Index, s.cache) // TODO: combine to pass single input } type VertexSolver interface {