access gateway API from client #533

tonistiigi · 2018-07-23T05:31:33Z

opening to leave some comments to @ijc

tonistiigi

Just an idea but what if we do not add NewBuild/DeleteBuild (yet) and just change the current Solve request to have a behavior that when both frontend and llb definition are unspecified in the request it opens the interactive gateway instead and blocks on waiting for it to return.

tonistiigi · 2018-07-23T05:32:42Z

frontend/gateway/grpcclient/client.go


+func RunX(ctx context.Context, f client.BuildFunc, client *grpcClient) (retError error) {


Make this method of GRPCClient

tonistiigi · 2018-07-23T05:33:16Z

frontend/gateway/grpcclient/client.go

@@ -66,16 +92,21 @@ func convertRef(ref client.Reference) (string, error) {
 return r.id, nil
 }

-func Run(ctx context.Context, f client.BuildFunc) (retError error) {
+func Run(ctx context.Context, f client.BuildFunc) error {


s/Run/RunFromEnvironment/

tonistiigi · 2018-07-23T05:34:58Z

frontend/gateway/grpcclient/client.go

 "google.golang.org/grpc"
 "google.golang.org/grpc/status"
 )

 const frontendPrefix = "BUILDKIT_FRONTEND_OPT_"

+func New(ctx context.Context, session, build, product string, c pb.LLBBridgeClient, w []client.WorkerInfo) (*grpcClient, error) {
+ ctx = buildid.AppendToOutgoingContext(ctx, build)


I think it is better to create a wrapper around pb.LLBBridgeClient methods for injecting the buildid than to do it here.

That wrapper would then be in the client pkg.

ijc · 2018-07-23T13:04:08Z

Thanks, I'll address the individual comments you made.

The proposal you make for a new Solve behaviour is interesting. I'll have a think/play and see what it looks like.

ijc · 2018-07-23T14:57:09Z

I've made the suggested changes.

RE the Solve behaviour suggestion: What I had been planning to investigate was refactoring the existing Controller.Solve to pull out e.g. ResolveExporterInstance(worker, exporter, exporterattrs) llbsolver.ExporterRequest (i.e. a thing you pass the relevant req.Fields to), ResolveCacheExporter(...), something for importing cache refs etc so the appropriate bits could be reused as part of the new machinery. WDYT, worth trying that first or shall I go straight to the new "mode" for Solve?

ijc · 2018-07-23T15:01:17Z

Oh, I forgot the third thing I was going to mention:

I had been planning to make a demonstrator for this by doing a clone and hack job on one of the examples/* things to make a new version which uses this mode, but then I spotted:

$ grep dockerfile.v0 examples/build-using-dockerfile/main.go
		Frontend:       "dockerfile.v0", // TODO: use gateway

so maybe I should just fix that TODO (in-place rather than clone and hack style)?

tonistiigi

WDYT, worth trying that first or shall I go straight to the new "mode" for Solve?

I think reusing the Solve would remove the duplication from the server side.

There could be something like:

	if req.Definition == nil && req.Frontend == "" {
		res, err = s.registerGatewayForwarder(ctx, s.Bridge(j)).waitResult(ctx)
		if err != nil {
			return nil, err
		}
	}

In the current solver and existing bridge/exporter logic should still work.

Refactoring is still needed to remove the duplication from the client though.

so maybe I should just fix that TODO (in-place rather than clone and hack style)?

I think this TODO is just to switch to tonistiigi/dockerfile . Its ok to have some duplication in examples initially. We can decide later if any of them becomes obsolete.

tonistiigi · 2018-07-23T17:12:16Z

frontend/gateway/gateway.go

@@ -492,6 +497,10 @@ func (lbf *llbBridgeForwarder) Return(ctx context.Context, in *pb.ReturnRequest)
 return &pb.ReturnResponse{}, nil
 }

+func (lbf *llbBridgeForwarder) Result() *frontend.Result {


Make sure error is handled as well. This should probably return a channel that Solve() can wait on (in the case where there is no DeleteBuild()), or be a blocking function.

tonistiigi · 2018-07-23T17:12:59Z

frontend/gateway/grpcclient/client.go

+ sessionID: session,
+ workers: w,
+ product: product,
+ caps: pb.Caps.CapSet(resp.FrontendAPICaps),


rebase as llbCaps was added to here last week.

ijc · 2018-07-24T09:11:06Z

I think reusing the Solve would remove the duplication from the server side.

Ack. Will try it. There's a small race since the client will have to call Solve in one goroutine (since it is blocking) and BuildFunc in another, and the build one might get in before the Solve one (so there would be no build/job). But I think I can solve that in the same way Solver.Get waits for a Job to appear -- in fact with the refactoring maybe it can literally use Solver.Get, I'll have to have a play.

Its ok to have some duplication in examples initially

OK, will clone and hack.

ijc

@tonistiigi I've switched over to the new mode for Solve and it seems to be working well.

I've left some self-review on a few things that I'm not sure about.

ijc · 2018-07-25T12:22:12Z

client/build.go

+ }
+
+ feOpts := opt.FrontendAttrs
+ opt.FrontendAttrs = nil


I decided it was better to reuse SolveOpt with the switch to using Solve instead of Create/Delete, it results in this little wrinkle but I think that's tollerable compared with copying every field of a BuildOpt into a SolveOpt.

I think it is fine atm. We may need to look this over later. Maybe remove separate Solve() completely and figure out what is the best way to pass args.

ijc · 2018-07-25T12:23:10Z

client/build.go

+ }
+
+ cb := func(ref string, s *session.Session) error {
+ g, err := grpcclient.New(ctx, feOpts, s.ID(), "", c.gatewayClient(ref), gworkers)


I pass product as "" on the logic that the build callback is in the process so it surely already knows whatever it needs to know, but maybe this should be an argument to this Build method?

The user should pass this in. The default can be "buildkit"

ijc · 2018-07-25T12:23:46Z

client/build.go

+ if err != nil {
+ return err
+ }
+ // TBD defer teardown


I think there isn't anything to do here, will drop this comment.

ijc · 2018-07-25T12:29:01Z

frontend/gateway/gateway.go

@@ -224,15 +224,27 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten
 return lbf.result, nil
 }

-func newLLBBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridge, workers frontend.WorkerInfos) (*llbBridgeForwarder, error) {
+func (lbf *llbBridgeForwarder) Result() (*frontend.Result, error) {


Previously (comment is now collapsed) you said:

This should probably return a channel that Solve() can wait on (in the case where there is no DeleteBuild()), or be a blocking function.

The channel ended up being in the forwarder (solver/llbsolver/gateway.gatewayForwarder) (I confess I'd forgotten about your comment). Should I move it or is it ok as I have it now?

I think the channel would be slightly better in the forwarder. If not then this needs some locking and detection if Return() was called. When this was private it made lots of assumptions when it would be called. Now that it is public we need to protect the cases when it gets called out of place.

I think the channel would be slightly better in the forwarder.

I'm confused because it is in the forwarder right now, but perhaps we are talking about different things/places with that name?

Ok, yeah everything is called a forwarder :) . I meant it might be better in here as a method of llbBridgeForwarder. Maybe we can find a different name for gatewayForwarder?

"Proxy" is generally pretty overloaded but not (yet? 😉) in buildkit, so gatewayForwarder→gatewayServerProxy perhaps? Or since it wraps an LLBBridgeServer perhaps LLBBridgeServerProxy?

For moving the channel I'll change it to be similar to a Context e.g. a Done() <-chan struct() and a Result().

Is it permissible for frontends to call Return more than once? If so are the semantics to keep the first or last answer?

Currently the code independently keeps the most recent err and result, so you could end up setting both which is probably undesirable...

I went with latching the first Return but easy enough to change.

Is it permissible for frontends to call Return more than once?

Not atm but happy to hear use cases. If we keep the restriction we should probably validate it and produce an error if frontend misbehaves.

Ack, I'll modify what I did here to have setResult error if there is already a result (right now it just logs).

ijc · 2018-07-25T12:34:43Z

frontend/gateway/grpcclient/client.go

-
- export := client.caps.Supports(pb.CapReturnResult) == nil
+func (c *grpcClient) Run(ctx context.Context, f client.BuildFunc) (retError error) {
+ // TBD: in a client-side gateway situation if !export and f() returns an error, how to kick the parent `Solve` into exiting?


I'm a bit unclear on this, although I've moved the call to f later such that the if export defer stuff runs even if it fails I'm not convinced that is necessary -- the return from f would cause the surrounding errgroup to cancel which ought to cancel the other goroutine doing the Solve -- but I failed to trace the code far enough to confirm that cancelling the context would really cancel the gRPC.

Also the lack of similar handling in the !export case concerns me. Not sure what I should do there (depending onthe above, perhaps the answer is "nothing").

The export is never false in client gateway situation as client-gateway is (will be) implemented later than CapReturnResult that is already merged.

On the client side though, when this function fails. I think we should wait(maybe with timeout) for the main solve to error as well instead of returning early.

OK, so if we can rely on c.client.Return always being called (which it is now I've moved the call after the defer) then I think we can squash retError to nil in the defer, (unless c.client.Return itself fails, we should propagate that error still).

The call to c.client.Return will cause the main Solve return with that error as the overall result, instead of the errgroup propagating the error from f.

I'm not 100% sure on the impact in the regular frontend/gateway case though, seems like the same overall logic should apply?

I think we can squash retError to nil in the defer, (unless c.client.Return itself fails, we should propagate that error still).

No, we should still return it here. For example, a frontend should use it to return an error code as well. We can skip it in the client if we have the main error instead

We have cb in Client.Build capture any err but still return nil? Then we take the result of c.solve and if it is an err use that, but if it is not an error use the err capturerd from the cb instead? Something like:

var cbErr cb := func(ref string, s *session.Session) error { g, cbErr := grpcclient.New(ctx, feOpts, s.ID(), product, c.gatewayClientForBuild(ref), gworkers) if cbErr != nil { return nil } cbErr = g.Run(ctx, buildFunc) return nil } if err := c.solve(ctx, nil, cb, opt, statusChan); err != nil { return err } return cbErr

???

This wasn't quite right, I've pushed a commit along these lines though.

To make this slightly safer we could use a timeout. So if cb errors, and 5 seconds passes it will cancel the main context. This code can also be in to solve itself, not handled by the closure.

Done. I left it in a separate commit for now.

ijc · 2018-07-25T12:40:43Z

frontend/gateway/pb/caps.go

@@ -15,6 +15,7 @@ const (
 CapReadFile apicaps.CapID = "readfile"
 CapReturnResult apicaps.CapID = "return"
 CapReturnMap apicaps.CapID = "returnmap"
+ //TBD CapClientGateway?


I think this is probably needed, an older buildkitd would successfully return an empty Result if Solve was called by a new client with def and frontend both empty, I think the client needs return an error if this is attempted.

As client gateway can only be used from the controlAPI can we make it so that gateway calls to solve continue to return an empty result. I think this is already the case, but verify. Then no cap is needed as control API is not covered by caps at the moment.

make it so that gateway calls to solve continue to return an empty result

Under which circumstances?

Also not sure if by "gateway calls to solve" you mean calls to controlapi.Solve with def == nil && frontend == nil or when the callback given to controlapi.Build calls the gateway/client.Client.Solve.

The latter. When frontend (or this new logic) calls Solve() through the gateway grpc api.

I'll add a test case.

It panics on a nil deref at the range creq.Definition.Metadata at the top of grpcClient.Solve. Which I suspect a frontend could hit today. I'll add the surrounding check along with the test of that code path via this new logic.

Is there scaffolding for integration testing the APIs available to a proper frontend somewhere I'm failing to find? I think that might be a bit hard to arrange (perhaps it would involve gateway-devel somehow) but testing via this new mode should suffice for lots of the interesting cases I guess.

Yes, the integration tests are lacking that atm. First thing we could do is add a gateway switch to dockerfile tests similarly to the current worker ones. So these tests run using builtin frontend, using the dockerfile pkg directly through client gw and using the master build gateway image. After that we can add something that builds the gateway image from source and tests it directly.

For direct testing of the gateway API though client we can add regular new integration tests under client like you have already started.

So these tests run using builtin frontend, using the dockerfile pkg directly through client gw and using the master build gateway image

So we'd be looking at adding another dimension similar to the code in util/testutil/integration/run.go (which runs a set of tests on a range of workers) to frontend/dockerfile/dockerfile_test.go, except this time iterating over "modes". I'll have a dig.

I'm going to nuke the

//TBD CapClientGateway?

comment this thread is derived from (the topic has skewed a bit), which sadly means this will all get collapsed...

ijc · 2018-07-25T12:42:09Z

solver/llbsolver/gateway.go

+ gwf.mu.Lock()
+ defer gwf.mu.Unlock()
+
+ // TBD cancel things, cleanup


Not sure that there is anything required here.

tonistiigi · 2018-07-24T20:40:35Z

client/build.go

+ opt.FrontendAttrs = nil
+
+ cb := func(ref string, s *session.Session) error {
+ g, err := grpcclient.New(ctx, feOpts, s.ID(), "productTBD", c.gatewayClient(ref), nil /*TBD: worker info*/)


product should be passed in with opt and default to buildkit

tonistiigi · 2018-07-24T20:41:22Z

client/build.go

+
+type gatewayClient struct {
+ gateway gatewayapi.LLBBridgeClient
+ buildid string


tonistiigi · 2018-07-24T20:42:01Z

client/build.go

+ return &gatewayClient{g, buildid}
+}
+
+type gatewayClient struct {


gatewayClientForBuild

tonistiigi · 2018-07-24T20:51:24Z

client/build.go

+ }
+ // TBD defer teardown
+
+ if err := g.Run(ctx, build); err != nil {


comment for later cleanup. Lets try to avoid generic build where possible as it can mean anything. BuildFunc is fine

tonistiigi · 2018-07-25T17:18:23Z

client/build.go

+ }
+
+ cb := func(ref string, s *session.Session) error {
+ g, err := grpcclient.New(ctx, feOpts, s.ID(), "", c.gatewayClient(ref), gworkers)


The user should pass this in. The default can be "buildkit"

tonistiigi · 2018-07-25T17:25:08Z

frontend/gateway/gateway.go

@@ -224,15 +224,27 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten
 return lbf.result, nil
 }

-func newLLBBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridge, workers frontend.WorkerInfos) (*llbBridgeForwarder, error) {
+func (lbf *llbBridgeForwarder) Result() (*frontend.Result, error) {


I think the channel would be slightly better in the forwarder. If not then this needs some locking and detection if Return() was called. When this was private it made lots of assumptions when it would be called. Now that it is public we need to protect the cases when it gets called out of place.

tonistiigi · 2018-07-25T17:31:11Z

solver/llbsolver/gateway.go

+ gwf.mu.Lock()
+ defer gwf.mu.Unlock()
+
+ // TBD cancel things, cleanup


tonistiigi · 2018-07-25T17:38:19Z

solver/llbsolver/solver.go

 s.solver = solver.NewSolver(solver.SolverOpt{
 ResolveOpFunc: s.resolver(),
 DefaultCache: cache,
 })
 return s, nil
 }

+func (s *Solver) RegisterGatewayForwarder(server *grpc.Server) {


This looks bit weird construction. I'd suggest controller makes the gateway forwarder(no parameters) and registers the server. Then on Register, solver creates gateway.NewBridgeForwarder and registers the ID with the bridgeforwarder. Now the lookup is purely a map lookup (with the race handling) based on ID and never calls to the solver or no need to Finalize() the job.

This function is the one time start of day registration of the gRPC endpoint. Sounds like you are mostly talking about the s.gatewayForwarder.RegisterJob stuff in Solver.Solve though which dynamically adds the Job (and hence the NewBridgeForwarder) to the map though?

The issue is that on the client side the calls to Solve and the use of the gateway happen in parallel with no interlock. We cannot guarantee we have got as far as registering the forwarder before the gateway client makes its first RPC. So I was reusing the existing code in the job code which already knows how to wait for a job to be registered. Sound like you would instead prefer to duplicate the mechanism from Solver.Get() into gatewayForwarder.lookupJob (suitably renamed)?

I'll do that.

Sounds like you are mostly talking about the s.gatewayForwarder.RegisterJob

Yes, it was confusing. When "registers the server" I meant this function and with Register I meant gatewayForwarder.RegisterJob

Sound like you would instead prefer to duplicate the mechanism from Solver.Get() into gatewayForwarder.lookupJob (suitably renamed)?

Yes

In case it wasn't clear I also expected this RegisterGatewayForwarder to be replaced by passing in the gatewayforwarder to the solver constructor. Looking at gatewayforwarder, it doesn't seem to use any private stuff from solver anymore so doesn't need to be in same pkg as well.

I've pulled the setup into NewController and moved the code to control/gateway. Let me know if you would rather I pulled this all the way back to cmd/buildkitd/main.go (e.g. the caller of NewController) and/or if you have another preference for where the code live (I'm not super keen on my choice but couldn't really decide on anything better).

tonistiigi · 2018-07-25T17:43:23Z

frontend/gateway/pb/caps.go

@@ -15,6 +15,7 @@ const (
 CapReadFile apicaps.CapID = "readfile"
 CapReturnResult apicaps.CapID = "return"
 CapReturnMap apicaps.CapID = "returnmap"
+ //TBD CapClientGateway?


As client gateway can only be used from the controlAPI can we make it so that gateway calls to solve continue to return an empty result. I think this is already the case, but verify. Then no cap is needed as control API is not covered by caps at the moment.

tonistiigi · 2018-07-25T17:49:18Z

frontend/gateway/grpcclient/client.go

-
- export := client.caps.Supports(pb.CapReturnResult) == nil
+func (c *grpcClient) Run(ctx context.Context, f client.BuildFunc) (retError error) {
+ // TBD: in a client-side gateway situation if !export and f() returns an error, how to kick the parent `Solve` into exiting?


The export is never false in client gateway situation as client-gateway is (will be) implemented later than CapReturnResult that is already merged.

On the client side though, when this function fails. I think we should wait(maybe with timeout) for the main solve to error as well instead of returning early.

ijc · 2018-08-07T09:27:15Z

@tonistiigi what is your opinion of next steps here? Do we want to wait for #552 to get into shape first?

I think the review so far has been addressed (hopefully I didn't miss anything).

I guess I should rebase/squash the final 3 commits back into the first one.

tonistiigi · 2018-08-07T17:21:54Z

@ijc If the gateway itself has integration tests then I don't think Dockerfile is a blocker. Just remove the wip after it is ready for review for your.

Provide a `New` method and a new `Run` method on the `grpcClient` to allow manual creation of a client in addition to the current ability to initialise from the environment. Accordingly the existing `Run` method becomes `RunFromEnvironment`. Signed-off-by: Ian Campbell <ijc@docker.com>

ijc · 2018-08-08T11:23:41Z

@tonistiigi Thanks. I did a pass through the PR and tidied up a bunch of things here and there, rebased, squashed (while also pulling out some standalone fixes) and updated commit messages etc. Will drop the WIP.

tonistiigi

looking good

cc @AkihiroSuda

tonistiigi · 2018-08-08T22:26:10Z

client/build.go

+ }
+
+ if product == "" {
+ product = "buildkit"


apicaps.ExportedProduct

tonistiigi · 2018-08-08T22:37:46Z

client/solve.go

+ return nil
+ }
+
+ // If the callback failed then we wait up to


Add a comment that normally error in here should automatically cause the Solve() to error as well. This is a fallback.

tonistiigi · 2018-08-08T22:39:03Z

client/solve.go

+
+type buildCB func(ref string, s *session.Session) error
+
+func (c *Client) solve(ctx context.Context, def *llb.Definition, build buildCB, opt SolveOpt, statusChan chan *SolveStatus) (*SolveResponse, error) {


maybe s/build/runGateway/

tonistiigi · 2018-08-08T22:42:58Z

examples/build-using-dockerfile-client-gateway/main.go

+ ch := make(chan *client.SolveStatus)
+ eg, ctx := errgroup.WithContext(ctx)
+ eg.Go(func() error {
+ _, err := c.Build(ctx, *buildOpt, "", dockerfile.Build, ch)


I know I said before that duplication doesn't matter in here but as it turned out that the changes from the other file are really minimal we can probably just switch this behavior with an environment variable. This is low priority and can be follow-up.

It was easy, so I went ahead and recombined them.

tonistiigi · 2018-08-08T22:48:36Z

frontend/gateway/grpcclient/client.go

-
- c := pb.NewLLBBridgeClient(conn)
-
+func New(ctx context.Context, opts map[string]string, session, product string, c pb.LLBBridgeClient, w []client.WorkerInfo) (*grpcClient, error) {


return client.Client from a public func

client.Client lacks the Run method which both callers want to use. Run needs to dig inside the grpcClient so I can't just make this a global method which takes a client.Client.

In fact the callers only care about the Run method and none of the client.Client methods, the latter are only needed when Run calls the provided callbacks.

I'm going to try adding a GrpcClient interface with just the Run method, see how that looks.

Doesn't look too bad to me, but I'll leave it as a separate commit for now so you can cast judgement on it.

tonistiigi · 2018-08-08T22:56:48Z

solver/llbsolver/solver.go

 s.solver = solver.NewSolver(solver.SolverOpt{
 ResolveOpFunc: s.resolver(),
 DefaultCache: cache,
 })
 return s, nil
 }

+func (s *Solver) RegisterGatewayForwarder(server *grpc.Server) {


In case it wasn't clear I also expected this RegisterGatewayForwarder to be replaced by passing in the gatewayforwarder to the solver constructor. Looking at gatewayforwarder, it doesn't seem to use any private stuff from solver anymore so doesn't need to be in same pkg as well.

Returning an interface rather than a private-struct from a public interface is good practice. Signed-off-by: Ian Campbell <ijc@docker.com>

…e cases. Refactor the setting/getting of the result for a `llbBridgeForwarder` in order to check that `Return` is only called once and only with the correct options. Signed-off-by: Ian Campbell <ijc@docker.com>

Avoids panicing when accessing `creq.Definition.Metadata`. Signed-off-by: Ian Campbell <ijc@docker.com>

Reorder the code slightly so that an error returned by the call to the `BuildFunc` is picked up by the `defer`d error handling. Signed-off-by: Ian Campbell <ijc@docker.com>

AkihiroSuda · 2018-08-09T15:10:49Z

examples/build-using-dockerfile/main.go

+ cli.BoolFlag{
+ Name: "clientside-frontend",
+ Usage: "run dockerfile frontend client side, rather than builtin to buildkitd",
+ EnvVar: "CLIENTSIDE_FRONTEND",


BUILDKIT_CLIENTSIDE_FRONTEND?

tonistiigi

LGTM (can't approve in github UI as I created the PR)

tonistiigi · 2018-08-09T17:55:12Z

control/control.go

 }

 func NewController(opt Opt) (*Controller, error) {
 cache := solver.NewCacheManager("local", opt.CacheKeyStorage, worker.NewCacheResultStorage(opt.WorkerController))

- solver, err := llbsolver.New(opt.WorkerController, opt.Frontends, cache, opt.ResolveCacheImporterFunc)
+ gatewayForwarder := controlGateway.NewGatewayForwarder()


capitalization in a pkg name is weird

This allows builder code to be written which can be built as either a gateway container or in a purely client side configuration, giving implementors more flexibility. Now when `Solve` sees a request with neither a definition nor a frontend specified it will make the job available via new LLBBridge endpoints on the control socket which the client can then use. These end points require the job id to be present in the gRPC metadata and a client side object is added to facilitate this. The `llbBridgeForwarder` type is now exposed as a public `interface LLBBridgeForwarder` which satisfies the underlying gRPC server interface (`pb.LLBBridgeServer`) as well as a new `Done()` & `Result()` pair which can be used to wait for the client to call `Return()` (using a model similar to `context.Context`). Signed-off-by: Ian Campbell <ijc@docker.com>

Adds an option/envvar to `examples/build-using-dockerfile` which uses a client side instance of the dockerfile frontend. Signed-off-by: Ian Campbell <ijc@docker.com>

ijc · 2018-08-14T10:51:34Z

Addressed the example envvar name and the capitalisation of the controlGateway import. I also folded the Rework gatewayForwarder registration and move code commit into the previous commit.

tonistiigi added the status/do-not-merge label Jul 23, 2018

tonistiigi commented Jul 23, 2018

View reviewed changes

ijc force-pushed the client-gateway branch 7 times, most recently from 61a9018 to 75805e1 Compare July 25, 2018 12:29

ijc reviewed Jul 25, 2018

View reviewed changes

ijc mentioned this pull request Jul 25, 2018

apicaps: record the correct caps when constructing a CapSet #541

Merged

tonistiigi commented Jul 25, 2018

View reviewed changes

ijc force-pushed the client-gateway branch 7 times, most recently from 1987f2e to 7fc8ee2 Compare July 30, 2018 15:22

ijc mentioned this pull request Aug 1, 2018

Test dockerfile in gateway mode #552

Closed

ijc force-pushed the client-gateway branch 2 times, most recently from 8dcd6b5 to 50140d0 Compare August 6, 2018 09:34

ijc force-pushed the client-gateway branch 2 times, most recently from f1e28db to 60ce91c Compare August 8, 2018 11:22

ijc changed the title ~~WIP: access gateway API from client~~ access gateway API from client Aug 8, 2018

tonistiigi removed the status/do-not-merge label Aug 8, 2018

tonistiigi commented Aug 8, 2018

View reviewed changes

ijc force-pushed the client-gateway branch from 60ce91c to ee393c9 Compare August 9, 2018 10:13

Ian Campbell added 4 commits August 9, 2018 11:19

gateway: Return an interface from grpcclient.New.

5485309

Returning an interface rather than a private-struct from a public interface is good practice. Signed-off-by: Ian Campbell <ijc@docker.com>

gateway: make Return a one shot operation and check for various edg…

3f5276b

…e cases. Refactor the setting/getting of the result for a `llbBridgeForwarder` in order to check that `Return` is only called once and only with the correct options. Signed-off-by: Ian Campbell <ijc@docker.com>

gateway: tolerate Solve being called with no creq.Definition

13f53d5

Avoids panicing when accessing `creq.Definition.Metadata`. Signed-off-by: Ian Campbell <ijc@docker.com>

gateway: call Return if call to BuildFunc returns an error.

2a4ec0b

Reorder the code slightly so that an error returned by the call to the `BuildFunc` is picked up by the `defer`d error handling. Signed-off-by: Ian Campbell <ijc@docker.com>

ijc force-pushed the client-gateway branch 3 times, most recently from 2b47a8e to ccdc030 Compare August 9, 2018 11:44

AkihiroSuda reviewed Aug 9, 2018

View reviewed changes

tonistiigi commented Aug 9, 2018

View reviewed changes

ijc force-pushed the client-gateway branch from ccdc030 to d736e32 Compare August 14, 2018 10:36

Ian Campbell added 2 commits August 14, 2018 11:50

examples: demonstrate client-side Build() interface.

5383270

Adds an option/envvar to `examples/build-using-dockerfile` which uses a client side instance of the dockerfile frontend. Signed-off-by: Ian Campbell <ijc@docker.com>

ijc force-pushed the client-gateway branch from d736e32 to 5383270 Compare August 14, 2018 10:50

This was referenced Aug 15, 2018

[WIP] exporters: allow frontend control of the exported image name #572

Closed

Allow some Metadata to be return from frontend to the control client #574

Merged

AkihiroSuda approved these changes Aug 16, 2018

View reviewed changes

AkihiroSuda merged commit af46188 into moby:master Aug 16, 2018

ijc deleted the client-gateway branch August 17, 2018 13:29

ijc mentioned this pull request Aug 21, 2018

control: Add ResolveImageConfig() #472

Closed


		func RunX(ctx context.Context, f client.BuildFunc, client *grpcClient) (retError error) {


		type buildCB func(ref string, s *session.Session) error

		func (c Client) solve(ctx context.Context, def llb.Definition, build buildCB, opt SolveOpt, statusChan chan SolveStatus) (SolveResponse, error) {


		c := pb.NewLLBBridgeClient(conn)

		func New(ctx context.Context, opts map[string]string, session, product string, c pb.LLBBridgeClient, w []client.WorkerInfo) (*grpcClient, error) {

access gateway API from client #533

access gateway API from client #533

Conversation

tonistiigi commented Jul 23, 2018 • edited Loading

tonistiigi left a comment

Choose a reason for hiding this comment

tonistiigi Jul 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijc commented Jul 23, 2018

ijc commented Jul 23, 2018

ijc commented Jul 23, 2018

tonistiigi left a comment

Choose a reason for hiding this comment

tonistiigi Jul 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijc commented Jul 24, 2018

ijc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijc commented Aug 7, 2018

tonistiigi commented Aug 7, 2018

ijc commented Aug 8, 2018

tonistiigi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tonistiigi commented Jul 23, 2018 •

edited

Loading

tonistiigi Jul 23, 2018 •

edited

Loading

tonistiigi Jul 23, 2018 •

edited

Loading

tonistiigi left a comment •

edited

Loading