From 17d43698666673ec3e94b35d201864e27371f83c Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 16 Aug 2022 10:58:23 +0100 Subject: [PATCH] create: improve interface when attempting to create docker driver Previously, the help information for buildx indicated that users could create a new instance of the docker driver - which is explicitly something we don't support, driver of this form are automatically derived from the available list of docker contexts. This patch ensures that don't have AllowsInstance set will not appear in the help text, and additionally provide a new more specific error message instead of the generic "failed to find driver". This should help point users in the correct direction. Signed-off-by: Justin Chadwell --- commands/create.go | 6 +++--- commands/util.go | 5 +++-- docs/reference/buildx_create.md | 2 +- driver/manager.go | 17 ++++++++++------- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/commands/create.go b/commands/create.go index 074b309e..cc0af47c 100644 --- a/commands/create.go +++ b/commands/create.go @@ -135,8 +135,8 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { } } - if driver.GetFactory(driverName, true) == nil { - return errors.Errorf("failed to find driver %q", driverName) + if _, err := driver.GetFactory(driverName, true); err != nil { + return err } ngOriginal := ng @@ -282,7 +282,7 @@ func createCmd(dockerCli command.Cli) *cobra.Command { var options createOptions var drivers bytes.Buffer - for _, d := range driver.GetFactories() { + for _, d := range driver.GetFactories(true) { if len(drivers.String()) > 0 { drivers.WriteString(", ") } diff --git a/commands/util.go b/commands/util.go index 2239992a..266887ab 100644 --- a/commands/util.go +++ b/commands/util.go @@ -60,8 +60,9 @@ func driversForNodeGroup(ctx context.Context, dockerCli command.Cli, ng *store.N var f driver.Factory if ng.Driver != "" { - f = driver.GetFactory(ng.Driver, true) - if f == nil { + var err error + f, err = driver.GetFactory(ng.Driver, true) + if err != nil { return nil, errors.Errorf("failed to find driver %q", f) } } else { diff --git a/docs/reference/buildx_create.md b/docs/reference/buildx_create.md index faacc8e3..6f57e465 100644 --- a/docs/reference/buildx_create.md +++ b/docs/reference/buildx_create.md @@ -15,7 +15,7 @@ Create a new builder instance | `--bootstrap` | | | Boot builder after creation | | [`--buildkitd-flags`](#buildkitd-flags) | `string` | | Flags for buildkitd daemon | | [`--config`](#config) | `string` | | BuildKit config file | -| [`--driver`](#driver) | `string` | | Driver to use (available: `docker`, `docker-container`, `kubernetes`, `remote`) | +| [`--driver`](#driver) | `string` | | Driver to use (available: `docker-container`, `kubernetes`, `remote`) | | [`--driver-opt`](#driver-opt) | `stringArray` | | Options for the driver | | [`--leave`](#leave) | | | Remove a node from builder instead of changing it | | [`--name`](#name) | `string` | | Builder instance name | diff --git a/driver/manager.go b/driver/manager.go index 704b1228..37436a22 100644 --- a/driver/manager.go +++ b/driver/manager.go @@ -92,16 +92,16 @@ func GetDefaultFactory(ctx context.Context, ep string, c dockerclient.APIClient, return dd[0].f, nil } -func GetFactory(name string, instanceRequired bool) Factory { +func GetFactory(name string, instanceRequired bool) (Factory, error) { for _, f := range drivers { - if instanceRequired && !f.AllowsInstances() { - continue - } if f.Name() == name { - return f + if instanceRequired && !f.AllowsInstances() { + return nil, errors.Errorf("additional instances of driver %q cannot be created", name) + } + return f, nil } } - return nil + return nil, errors.Errorf("failed to find driver %q", name) } func GetDriver(ctx context.Context, name string, f Factory, endpointAddr string, api dockerclient.APIClient, auth Auth, kcc KubeClientConfig, flags []string, files map[string][]byte, do map[string]string, platforms []specs.Platform, contextPathHash string) (Driver, error) { @@ -131,9 +131,12 @@ func GetDriver(ctx context.Context, name string, f Factory, endpointAddr string, return &cachedDriver{Driver: d}, nil } -func GetFactories() []Factory { +func GetFactories(instanceRequired bool) []Factory { ds := make([]Factory, 0, len(drivers)) for _, d := range drivers { + if instanceRequired && !d.AllowsInstances() { + continue + } ds = append(ds, d) } sort.Slice(ds, func(i, j int) bool {