From 309c49413cec5576d0cb4b45aefff0a371eb1e79 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 12 Jul 2022 13:10:05 +0100 Subject: [PATCH 1/2] buildx: log errors in initializing builders Previously, errors within the driver config would not be reported to the user until they tried to use the driver, even though they are easily accessible from the node group info. This patch reports these errors (but will not fail because of them, since the data is already saved) - this should help improve debuggability of some of the more complex drivers, and prevent error messages being suppressed. Signed-off-by: Justin Chadwell --- commands/create.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/commands/create.go b/commands/create.go index 2a9c1296a08..9238551c949 100644 --- a/commands/create.go +++ b/commands/create.go @@ -242,6 +242,11 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { if err = loadNodeGroupData(timeoutCtx, dockerCli, ngi); err != nil { return err } + for _, info := range ngi.drivers { + if err := info.di.Err; err != nil { + logrus.Errorf("failed to initialize builder %s (%s): %s", ng.Name, info.di.Name, err) + } + } if in.bootstrap { if _, err = boot(ctx, ngi); err != nil { From 86825a95ce655383f9eec8b4d7637ccc6ad5a2dd Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 27 Jul 2022 11:48:52 +0100 Subject: [PATCH 2/2] buildx: rollback configuration if create fails This builds on the added warnings from initialized builders, now erroring the command, and additionally attempting to revert to the previous configuration. To preserve the previous configuration, we add a deep Copy() function to the NodeGroup and Node so that we can easily restore it later if we encounter a failure. Signed-off-by: Justin Chadwell --- commands/create.go | 37 ++++++++++++++++++++++++++----------- store/nodegroup.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/commands/create.go b/commands/create.go index 9238551c949..87b53a806be 100644 --- a/commands/create.go +++ b/commands/create.go @@ -134,6 +134,11 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { } } + ngOriginal := ng + if ngOriginal != nil { + ngOriginal = ngOriginal.Copy() + } + if ng == nil { ng = &store.NodeGroup{ Name: name, @@ -224,16 +229,6 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { return err } - if in.use && ep != "" { - current, err := storeutil.GetCurrentEndpoint(dockerCli) - if err != nil { - return err - } - if err := txn.SetCurrent(current, ng.Name, false, false); err != nil { - return err - } - } - ngi := &nginfo{ng: ng} timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second) @@ -244,7 +239,27 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { } for _, info := range ngi.drivers { if err := info.di.Err; err != nil { - logrus.Errorf("failed to initialize builder %s (%s): %s", ng.Name, info.di.Name, err) + err := errors.Errorf("failed to initialize builder %s (%s): %s", ng.Name, info.di.Name, err) + var err2 error + if ngOriginal == nil { + err2 = txn.Remove(ng.Name) + } else { + err2 = txn.Save(ngOriginal) + } + if err2 != nil { + logrus.Warnf("Could not rollback to previous state: %s", err2) + } + return err + } + } + + if in.use && ep != "" { + current, err := storeutil.GetCurrentEndpoint(dockerCli) + if err != nil { + return err + } + if err := txn.SetCurrent(current, ng.Name, false, false); err != nil { + return err } } diff --git a/store/nodegroup.go b/store/nodegroup.go index c9f97a67228..b6288e0981a 100644 --- a/store/nodegroup.go +++ b/store/nodegroup.go @@ -110,6 +110,44 @@ func (ng *NodeGroup) Update(name, endpoint string, platforms []string, endpoints return nil } +func (ng *NodeGroup) Copy() *NodeGroup { + nodes := make([]Node, len(ng.Nodes)) + for i, node := range ng.Nodes { + nodes[i] = *node.Copy() + } + return &NodeGroup{ + Name: ng.Name, + Driver: ng.Driver, + Nodes: nodes, + Dynamic: ng.Dynamic, + } +} + +func (n *Node) Copy() *Node { + platforms := []specs.Platform{} + copy(platforms, n.Platforms) + flags := []string{} + copy(flags, n.Flags) + driverOpts := map[string]string{} + for k, v := range n.DriverOpts { + driverOpts[k] = v + } + files := map[string][]byte{} + for k, v := range n.Files { + vv := []byte{} + copy(vv, v) + files[k] = vv + } + return &Node{ + Name: n.Name, + Endpoint: n.Endpoint, + Platforms: platforms, + Flags: flags, + DriverOpts: driverOpts, + Files: files, + } +} + func (ng *NodeGroup) validateDuplicates(ep string, idx int) error { i := 0 for _, n := range ng.Nodes {