Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce new errors for unsupported driver behaviors #1188

Merged
merged 3 commits into from
Aug 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 41 additions & 33 deletions commands/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,32 +61,6 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
}
}

buildkitHost := os.Getenv("BUILDKIT_HOST")

driverName := in.driver
if driverName == "" {
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()
}
}

if driver.GetFactory(driverName, true) == nil {
return errors.Errorf("failed to find driver %q", in.driver)
}

txn, release, err := storeutil.GetStore(dockerCli)
if err != nil {
return err
Expand Down Expand Up @@ -121,34 +95,62 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
logrus.Warnf("failed to find %q for append, creating a new instance instead", in.name)
}
if in.actionLeave {
return errors.Errorf("failed to find instance %q for leave", name)
return errors.Errorf("failed to find instance %q for leave", in.name)
}
} else {
return err
}
}

buildkitHost := os.Getenv("BUILDKIT_HOST")

driverName := in.driver
if driverName == "" {
if ng != nil {
driverName = ng.Driver
} else 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()
}
}

if ng != nil {
if in.nodeName == "" && !in.actionAppend {
return errors.Errorf("existing instance for %s but no append mode, specify --node to make changes for existing instances", name)
return errors.Errorf("existing instance for %q but no append mode, specify --node to make changes for existing instances", name)
}
if driverName != ng.Driver {
return errors.Errorf("existing instance for %q but has mismatched driver %q", name, ng.Driver)
}
}

if driver.GetFactory(driverName, true) == nil {
return errors.Errorf("failed to find driver %q", driverName)
}

ngOriginal := ng
if ngOriginal != nil {
ngOriginal = ngOriginal.Copy()
}

if ng == nil {
ng = &store.NodeGroup{
Name: name,
Name: name,
Driver: driverName,
}
}

if ng.Driver == "" || in.driver != "" {
ng.Driver = driverName
}

var flags []string
if in.flags != "" {
flags, err = shlex.Split(in.flags)
Expand All @@ -166,6 +168,9 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
} else {
switch {
case driverName == "kubernetes":
if len(args) > 0 {
logrus.Warnf("kubernetes driver does not support endpoint args %q", args[0])
}
// naming endpoint to make --append works
ep = (&url.URL{
Scheme: driverName,
Expand Down Expand Up @@ -315,6 +320,9 @@ func createCmd(dockerCli command.Cli) *cobra.Command {
}

func csvToMap(in []string) (map[string]string, error) {
if len(in) == 0 {
return nil, nil
}
m := make(map[string]string, len(in))
for _, s := range in {
csvReader := csv.NewReader(strings.NewReader(s))
Expand Down
35 changes: 27 additions & 8 deletions store/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/docker/buildx/util/platformutil"
specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

type NodeGroup struct {
Expand Down Expand Up @@ -59,17 +60,42 @@ func (ng *NodeGroup) Update(name, endpoint string, platforms []string, endpoints
return err
}

var files map[string][]byte
if configFile != "" {
files, err = confutil.LoadConfigFiles(configFile)
if err != nil {
return err
}
}

if i != -1 {
n := ng.Nodes[i]
needsRestart := false
if endpointsSet {
n.Endpoint = endpoint
needsRestart = true
}
if len(platforms) > 0 {
n.Platforms = pp
}
if flags != nil {
n.Flags = flags
needsRestart = true
}
if do != nil {
n.DriverOpts = do
needsRestart = true
}
if configFile != "" {
for k, v := range files {
n.Files[k] = v
}
needsRestart = true
}
if needsRestart {
logrus.Warn("new settings may not be used until builder is restarted")
}

ng.Nodes[i] = n
if err := ng.validateDuplicates(endpoint, i); err != nil {
return err
Expand All @@ -92,14 +118,7 @@ func (ng *NodeGroup) Update(name, endpoint string, platforms []string, endpoints
Platforms: pp,
Flags: flags,
DriverOpts: do,
}

if configFile != "" {
files, err := confutil.LoadConfigFiles(configFile)
if err != nil {
return err
}
n.Files = files
Files: files,
}

ng.Nodes = append(ng.Nodes, n)
Expand Down