Skip to content

Commit

Permalink
Complete remote driver
Browse files Browse the repository at this point in the history
This patch completes the work started in creating a remote driver:

- Renames the env driver to the remote driver (an alternative suggestion
  that should be more user-friendly)
- Adds support for TLS to encrypt connections with buildkitd
- Fixes outstanding review comments
- Reworks the buildx create command endpoint construction to be clearer
  and include better support for this new driver.

Signed-off-by: Justin Chadwell <[email protected]>
  • Loading branch information
jedevc committed Apr 28, 2022
1 parent 3dc83e5 commit d7e4aff
Show file tree
Hide file tree
Showing 12 changed files with 243 additions and 123 deletions.
17 changes: 17 additions & 0 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ jobs:
- docker
- docker-container
- kubernetes
- remote
buildkit:
- moby/buildkit:buildx-stable-1
- moby/buildkit:master
Expand All @@ -68,13 +69,19 @@ jobs:
include:
- driver: kubernetes
driver-opt: qemu.install=true
- driver: remote
endpoint: tcp://localhost:1234
exclude:
- driver: docker
multi-node: mnode-true
- driver: docker
buildkit-cfg: bkcfg-true
- driver: docker-container
multi-node: mnode-true
- driver: remote
multi-node: mnode-true
- driver: remote
buildkit-cfg: bkcfg-true
steps:
-
name: Checkout
Expand Down Expand Up @@ -128,6 +135,15 @@ jobs:
if: matrix.driver == 'kubernetes'
run: |
kubectl get nodes
-
name: Launch remote buildkitd
if: matrix.driver == 'remote'
run: |
docker run -d --privileged \
--name=remote-buildkit \
-p 1234:1234 \
${{ matrix.buildkit }} \
--addr tcp://0.0.0.0:1234
-
name: Test
run: |
Expand All @@ -136,4 +152,5 @@ jobs:
BUILDKIT_IMAGE: ${{ matrix.buildkit }}
DRIVER: ${{ matrix.driver }}
DRIVER_OPT: ${{ matrix.driver-opt }}
ENDPOINT: ${{ matrix.endpoint }}
PLATFORMS: ${{ matrix.platforms }}
2 changes: 1 addition & 1 deletion cmd/buildx/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import (

_ "github.com/docker/buildx/driver/docker"
_ "github.com/docker/buildx/driver/docker-container"
_ "github.com/docker/buildx/driver/env"
_ "github.com/docker/buildx/driver/kubernetes"
_ "github.com/docker/buildx/driver/remote"
)

func init() {
Expand Down
71 changes: 48 additions & 23 deletions commands/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,26 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
}
}

buildkitHost := os.Getenv("BUILDKIT_HOST")

driverName := in.driver
if driverName == "" {
f, err := driver.GetDefaultFactory(ctx, dockerCli.Client(), true)
if err != nil {
return err
}
if f == nil {
return errors.Errorf("no valid drivers found")
if len(args) == 0 && buildkitHost != "" {
driverName = "remote"
} else {
var arg string
if len(args) > 0 {
arg = args[0]
}
f, err := driver.GetDefaultFactory(ctx, arg, dockerCli.Client(), true)
if err != nil {
return err
}
if f == nil {
return errors.Errorf("no valid drivers found")
}
driverName = f.Name()
}
driverName = f.Name()
}

if driver.GetFactory(driverName, true) == nil {
Expand Down Expand Up @@ -129,44 +139,59 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
}

var ep string
var setEp bool
if in.actionLeave {
if err := ng.Leave(in.nodeName); err != nil {
return err
}
} else {
if len(args) > 0 {
switch {
case driverName == "kubernetes":
// naming endpoint to make --append works
ep = (&url.URL{
Scheme: driverName,
Path: "/" + in.name,
RawQuery: (&url.Values{
"deployment": {in.nodeName},
"kubeconfig": {os.Getenv("KUBECONFIG")},
}).Encode(),
}).String()
setEp = false
case driverName == "remote":
if len(args) > 0 {
ep = args[0]
} else if buildkitHost != "" {
ep = buildkitHost
} else {
return errors.Errorf("no remote endpoint provided")
}
ep, err = validateBuildkitEndpoint(ep)
if err != nil {
return err
}
setEp = true
case len(args) > 0:
ep, err = validateEndpoint(dockerCli, args[0])
if err != nil {
return err
}
} else {
setEp = true
default:
if dockerCli.CurrentContext() == "default" && dockerCli.DockerEndpoint().TLSData != nil {
return errors.Errorf("could not create a builder instance with TLS data loaded from environment. Please use `docker context create <context-name>` to create a context for current environment and then create a builder instance with `docker buildx create <context-name>`")
}

ep, err = storeutil.GetCurrentEndpoint(dockerCli)
if err != nil {
return err
}
}

if in.driver == "kubernetes" {
// naming endpoint to make --append works
ep = (&url.URL{
Scheme: in.driver,
Path: "/" + in.name,
RawQuery: (&url.Values{
"deployment": {in.nodeName},
"kubeconfig": {os.Getenv("KUBECONFIG")},
}).Encode(),
}).String()
setEp = false
}

m, err := csvToMap(in.driverOpts)
if err != nil {
return err
}
if err := ng.Update(in.nodeName, ep, in.platform, len(args) > 0, in.actionAppend, flags, in.configFile, m); err != nil {
if err := ng.Update(in.nodeName, ep, in.platform, setEp, in.actionAppend, flags, in.configFile, m); err != nil {
return err
}
}
Expand Down
29 changes: 17 additions & 12 deletions commands/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"os"
"strings"

buildkitclient "github.com/moby/buildkit/client"

"github.com/docker/buildx/build"
"github.com/docker/buildx/driver"
ctxkube "github.com/docker/buildx/driver/kubernetes/context"
Expand Down Expand Up @@ -43,6 +41,18 @@ func validateEndpoint(dockerCli command.Cli, ep string) (string, error) {
return h, nil
}

// validateBuildkitEndpoint validates that endpoint is a valid buildkit host
func validateBuildkitEndpoint(ep string) (string, error) {
endpoint, err := url.Parse(ep)
if err != nil {
return "", errors.Wrapf(err, "failed to parse endpoint %s", ep)
}
if endpoint.Scheme != "tcp" && endpoint.Scheme != "unix" {
return "", errors.Errorf("unrecognized url scheme %s", endpoint.Scheme)
}
return ep, nil
}

// driversForNodeGroup returns drivers for a nodegroup instance
func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, contextPathHash string) ([]build.DriverInfo, error) {
eg, _ := errgroup.WithContext(ctx)
Expand All @@ -56,11 +66,12 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N
return nil, errors.Errorf("failed to find driver %q", f)
}
} else {
dockerapi, err := clientForEndpoint(dockerCli, ng.Nodes[0].Endpoint)
ep := ng.Nodes[0].Endpoint
dockerapi, err := clientForEndpoint(dockerCli, ep)
if err != nil {
return nil, err
}
f, err = driver.GetDefaultFactory(ctx, dockerapi, false)
f, err = driver.GetDefaultFactory(ctx, ep, dockerapi, false)
if err != nil {
return nil, err
}
Expand All @@ -83,12 +94,6 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N
dis[i] = di
}()

buildkitAPI, err := buildkitclient.New(ctx, n.Endpoint)
if err != nil {
di.Err = err
return nil
}

dockerapi, err := clientForEndpoint(dockerCli, n.Endpoint)
if err != nil {
di.Err = err
Expand Down Expand Up @@ -127,7 +132,7 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N
}
}

d, err := driver.GetDriver(ctx, "buildx_buildkit_"+n.Name, f, dockerapi, n.Endpoint, buildkitAPI, imageopt.Auth, kcc, n.Flags, n.Files, n.DriverOpts, n.Platforms, contextPathHash, ng.Driver)
d, err := driver.GetDriver(ctx, "buildx_buildkit_"+n.Name, f, n.Endpoint, dockerapi, imageopt.Auth, kcc, n.Flags, n.Files, n.DriverOpts, n.Platforms, contextPathHash)
if err != nil {
di.Err = err
return nil
Expand Down Expand Up @@ -268,7 +273,7 @@ func getDefaultDrivers(ctx context.Context, dockerCli command.Cli, defaultOnly b
return nil, err
}

d, err := driver.GetDriver(ctx, "buildx_buildkit_default", nil, dockerCli.Client(), "", nil, imageopt.Auth, nil, nil, nil, nil, nil, contextPathHash, "")
d, err := driver.GetDriver(ctx, "buildx_buildkit_default", nil, "", dockerCli.Client(), imageopt.Auth, nil, nil, nil, nil, nil, contextPathHash)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion driver/docker-container/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (*factory) Usage() string {
return "docker-container"
}

func (*factory) Priority(ctx context.Context, api dockerclient.APIClient) int {
func (*factory) Priority(ctx context.Context, endpoint string, api dockerclient.APIClient) int {
if api == nil {
return priorityUnsupported
}
Expand Down
2 changes: 1 addition & 1 deletion driver/docker/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func (*factory) Usage() string {
return "docker"
}

func (*factory) Priority(ctx context.Context, api dockerclient.APIClient) int {
func (*factory) Priority(ctx context.Context, endpoint string, api dockerclient.APIClient) int {
if api == nil {
return priorityUnsupported
}
Expand Down
52 changes: 0 additions & 52 deletions driver/env/factory.go

This file was deleted.

2 changes: 1 addition & 1 deletion driver/kubernetes/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (*factory) Usage() string {
return DriverName
}

func (*factory) Priority(ctx context.Context, api dockerclient.APIClient) int {
func (*factory) Priority(ctx context.Context, endpoint string, api dockerclient.APIClient) int {
if api == nil {
return priorityUnsupported
}
Expand Down
Loading

0 comments on commit d7e4aff

Please sign in to comment.