builder: extra initi error handling

* Return errors from creating the `NodeGroup` * Ensure that `b.NodeGroup != nil` before reading from it during validation Signed-off-by: Milas Bowman <milas.bowman@docker.com>
docker · Apr 11, 2023 · 60f3d87 · 60f3d87
1 parent 672eeed
commit 60f3d87
Showing 1 changed file with 57 additions and 45 deletions.
diff --git a/builder/builder.go b/builder/builder.go
@@ -6,15 +6,16 @@ import (
 	"sort"
 	"sync"
 
+	"github.com/docker/cli/cli/command"
+	"github.com/pkg/errors"
+	"golang.org/x/sync/errgroup"
+
 	"github.com/docker/buildx/driver"
 	"github.com/docker/buildx/store"
 	"github.com/docker/buildx/store/storeutil"
 	"github.com/docker/buildx/util/dockerutil"
 	"github.com/docker/buildx/util/imagetools"
 	"github.com/docker/buildx/util/progress"
-	"github.com/docker/cli/cli/command"
-	"github.com/pkg/errors"
-	"golang.org/x/sync/errgroup"
 )
 
 // Builder represents an active builder object
@@ -97,6 +98,9 @@ func New(dockerCli command.Cli, opts ...Option) (_ *Builder, err error) {
 			return nil, err
 		}
 	}
+	if err != nil {
+		return nil, err
+	}
 	if b.opts.validate {
 		if err = b.Validate(); err != nil {
 			return nil, err
@@ -108,7 +112,7 @@ func New(dockerCli command.Cli, opts ...Option) (_ *Builder, err error) {
 
 // Validate validates builder context
 func (b *Builder) Validate() error {
-	if b.NodeGroup.DockerContext {
+	if b.NodeGroup != nil && b.NodeGroup.DockerContext {
 		list, err := b.opts.dockerCli.ContextStore().List()
 		if err != nil {
 			return err
@@ -166,14 +170,16 @@ func (b *Builder) Boot(ctx context.Context) (bool, error) {
 	eg, _ := errgroup.WithContext(ctx)
 	for _, idx := range toBoot {
 		func(idx int) {
-			eg.Go(func() error {
-				pw := progress.WithPrefix(printer, b.NodeGroup.Nodes[idx].Name, len(toBoot) > 1)
-				_, err := driver.Boot(ctx, baseCtx, b.nodes[idx].Driver, pw)
-				if err != nil {
-					b.nodes[idx].Err = err
-				}
-				return nil
-			})
+			eg.Go(
+				func() error {
+					pw := progress.WithPrefix(printer, b.NodeGroup.Nodes[idx].Name, len(toBoot) > 1)
+					_, err := driver.Boot(ctx, baseCtx, b.nodes[idx].Driver, pw)
+					if err != nil {
+						b.nodes[idx].Err = err
+					}
+					return nil
+				},
+			)
 		}(idx)
 	}
 
@@ -208,35 +214,37 @@ type driverFactory struct {
 
 // Factory returns the driver factory.
 func (b *Builder) Factory(ctx context.Context) (_ driver.Factory, err error) {
-	b.driverFactory.once.Do(func() {
-		if b.Driver != "" {
-			b.driverFactory.Factory, err = driver.GetFactory(b.Driver, true)
-			if err != nil {
-				return
-			}
-		} else {
-			// empty driver means nodegroup was implicitly created as a default
-			// driver for a docker context and allows falling back to a
-			// docker-container driver for older daemon that doesn't support
-			// buildkit (< 18.06).
-			ep := b.NodeGroup.Nodes[0].Endpoint
-			var dockerapi *dockerutil.ClientAPI
-			dockerapi, err = dockerutil.NewClientAPI(b.opts.dockerCli, b.NodeGroup.Nodes[0].Endpoint)
-			if err != nil {
-				return
-			}
-			// check if endpoint is healthy is needed to determine the driver type.
-			// if this fails then can't continue with driver selection.
-			if _, err = dockerapi.Ping(ctx); err != nil {
-				return
-			}
-			b.driverFactory.Factory, err = driver.GetDefaultFactory(ctx, ep, dockerapi, false)
-			if err != nil {
-				return
+	b.driverFactory.once.Do(
+		func() {
+			if b.Driver != "" {
+				b.driverFactory.Factory, err = driver.GetFactory(b.Driver, true)
+				if err != nil {
+					return
+				}
+			} else {
+				// empty driver means nodegroup was implicitly created as a default
+				// driver for a docker context and allows falling back to a
+				// docker-container driver for older daemon that doesn't support
+				// buildkit (< 18.06).
+				ep := b.NodeGroup.Nodes[0].Endpoint
+				var dockerapi *dockerutil.ClientAPI
+				dockerapi, err = dockerutil.NewClientAPI(b.opts.dockerCli, b.NodeGroup.Nodes[0].Endpoint)
+				if err != nil {
+					return
+				}
+				// check if endpoint is healthy is needed to determine the driver type.
+				// if this fails then can't continue with driver selection.
+				if _, err = dockerapi.Ping(ctx); err != nil {
+					return
+				}
+				b.driverFactory.Factory, err = driver.GetDefaultFactory(ctx, ep, dockerapi, false)
+				if err != nil {
+					return
+				}
+				b.Driver = b.driverFactory.Factory.Name()
 			}
-			b.Driver = b.driverFactory.Factory.Name()
-		}
-	})
+		},
+	)
 	return b.driverFactory.Factory, err
 }
 
@@ -250,7 +258,8 @@ func GetBuilders(dockerCli command.Cli, txn *store.Txn) ([]*Builder, error) {
 	builders := make([]*Builder, len(storeng))
 	seen := make(map[string]struct{})
 	for i, ng := range storeng {
-		b, err := New(dockerCli,
+		b, err := New(
+			dockerCli,
 			WithName(ng.Name),
 			WithStore(txn),
 			WithSkippedValidation(),
@@ -266,9 +275,11 @@ func GetBuilders(dockerCli command.Cli, txn *store.Txn) ([]*Builder, error) {
 	if err != nil {
 		return nil, err
 	}
-	sort.Slice(contexts, func(i, j int) bool {
-		return contexts[i].Name < contexts[j].Name
-	})
+	sort.Slice(
+		contexts, func(i, j int) bool {
+			return contexts[i].Name < contexts[j].Name
+		},
+	)
 
 	for _, c := range contexts {
 		// if a context has the same name as an instance from the store, do not
@@ -277,7 +288,8 @@ func GetBuilders(dockerCli command.Cli, txn *store.Txn) ([]*Builder, error) {
 		if _, ok := seen[c.Name]; ok {
 			continue
 		}
-		b, err := New(dockerCli,
+		b, err := New(
+			dockerCli,
 			WithName(c.Name),
 			WithStore(txn),
 			WithSkippedValidation(),