Skip to content

feat: add env driver#909

Closed
developer-guy wants to merge 1 commit intodocker:masterfrom
developer-guy:master
Closed

feat: add env driver#909
developer-guy wants to merge 1 commit intodocker:masterfrom
developer-guy:master

Conversation

@developer-guy
Copy link
Copy Markdown
Contributor

@developer-guy developer-guy commented Jan 9, 2022

Signed-off-by: Batuhan Apaydın [email protected]

Fixes #23

cc: @AkihiroSuda @tonistiigi

@AkihiroSuda AkihiroSuda marked this pull request as draft January 9, 2022 12:46
@errordeveloper errordeveloper self-assigned this Jan 10, 2022
@errordeveloper errordeveloper added the kind/enhancement New feature or request label Jan 10, 2022
Comment thread commands/util.go Outdated
Comment thread driver/manager.go Outdated
Comment thread commands/create.go Outdated
Comment thread commands/util.go Outdated
Comment thread driver/manager.go Outdated
Comment thread commands/create.go
Comment thread driver/remote/driver.go Outdated
@developer-guy developer-guy changed the title WIP: feat: add remote driver feat: add remote driver Mar 17, 2022
Comment thread commands/util.go
dis[i] = di
}()

buildkitAPI, err := buildkitclient.New(ctx, n.Endpoint)
Copy link
Copy Markdown
Contributor

@Dentrax Dentrax Mar 17, 2022

Choose a reason for hiding this comment

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

i think we're initializing both buildkitAPI and dockerapi here if we set --driver=env. So dockerapi initializing unnecessarily. But couldn't find a better way. Any ideas?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is definitely not allowed. Only the driver.Client() is allowed to call this. dockerapi initialization is for the features like --load that move things to Docker.

Another difference is that clientForEndpoint is just a struct setup, while buildkitclient.New() actually dials out to the endpoint.

@developer-guy
Copy link
Copy Markdown
Contributor Author

developer-guy commented Mar 17, 2022

We've tested it by providing a buildkitd.sock through our buildkit-machine project, everything worked fine, right now. Thanks a ton, @Dentrax.

$ buildkit-machine start buildkitd --unix /tmp/buildkitd.sock
$ docker buildx create --name demo --driver env /tmp/buildkit.sock --use

# build some random go project
$ docker buildx build -t <tag> <img> .

@developer-guy
Copy link
Copy Markdown
Contributor Author

Screen Shot 2022-03-17 at 20 29 48
Screen Shot 2022-03-17 at 20 29 28

@developer-guy
Copy link
Copy Markdown
Contributor Author

cc: @crazy-max

@developer-guy developer-guy marked this pull request as ready for review March 17, 2022 17:41
@developer-guy developer-guy changed the title feat: add remote driver feat: add env driver Mar 17, 2022
@developer-guy developer-guy force-pushed the master branch 2 times, most recently from f1bf153 to 9a816c3 Compare March 17, 2022 19:42
Signed-off-by: Batuhan Apaydın <[email protected]>
Co-authored-by: Furkan Türkal <[email protected]>
Signed-off-by: Batuhan Apaydın <[email protected]>
Comment thread driver/env/driver.go
Comment thread driver/env/driver.go
}

func (d *Driver) Features() map[driver.Feature]bool {
return map[driver.Feature]bool{}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why empty?

@tonistiigi
Copy link
Copy Markdown
Member

@developer-guy Any update on the comments? Lmk if you have questions.

@jedevc jedevc mentioned this pull request Apr 25, 2022
@developer-guy
Copy link
Copy Markdown
Contributor Author

@developer-guy Any update on the comments? Lmk if you have questions.

Hi @tonistiigi, sorry for delay, I was in military, I've just returned, I'll continue where I left, thanks for the mention 🙋🏻‍♂️

@jedevc
Copy link
Copy Markdown
Collaborator

jedevc commented Apr 28, 2022

@developer-guy have you seen #1078? I've tried to finish off the work you started, do let me know what you think 😄

@tonistiigi
Copy link
Copy Markdown
Member

Merged in #1078

@tonistiigi tonistiigi closed this Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add "env" driver that connects to $BUILDKIT_HOST

6 participants