Merge pull request #3055 from tonistiigi/history-queryrecord

history: generalize query loading

Author: crazy-max

commands/history/inspect.go

@@ -173,17 +173,7 @@ func runInspect(ctx context.Context, dockerCli command.Cli, opts inspectOptions)
 		}
 	}
 
-	var offset *int
-	if strings.HasPrefix(opts.ref, "^") {
-		off, err := strconv.Atoi(opts.ref[1:])
-		if err != nil {
-			return errors.Wrapf(err, "invalid offset %q", opts.ref)
-		}
-		offset = &off
-		opts.ref = ""
-	}
-
-	recs, err := queryRecords(ctx, opts.ref, nodes)
+	recs, err := queryRecords(ctx, opts.ref, nodes, nil)
 	if err != nil {
 		return err
 	}
@@ -195,26 +185,7 @@ func runInspect(ctx context.Context, dockerCli command.Cli, opts inspectOptions)
 		return errors.Errorf("no record found for ref %q", opts.ref)
 	}
 
-	var rec *historyRecord
-	if opts.ref == "" {
-		slices.SortFunc(recs, func(a, b historyRecord) int {
-			return b.CreatedAt.AsTime().Compare(a.CreatedAt.AsTime())
-		})
-		for _, r := range recs {
-			if offset != nil {
-				if *offset > 0 {
-					*offset--
-					continue
-				}
-			}
-			rec = &r
-			break
-		}
-		if offset != nil && *offset > 0 {
-			return errors.Errorf("no completed build found with offset %d", *offset)
-		}
-	}
-
+	rec := &recs[0]
 	c, err := rec.node.Driver.Client(ctx)
 	if err != nil {
 		return err

commands/history/inspect_attachment.go

@@ -3,7 +3,6 @@ package history
 import (
 	"context"
 	"io"
-	"slices"
 
 	"github.com/containerd/containerd/v2/core/content/proxy"
 	"github.com/containerd/platforms"
@@ -42,7 +41,7 @@ func runAttachment(ctx context.Context, dockerCli command.Cli, opts attachmentOp
 		}
 	}
 
-	recs, err := queryRecords(ctx, opts.ref, nodes)
+	recs, err := queryRecords(ctx, opts.ref, nodes, nil)
 	if err != nil {
 		return err
 	}
@@ -54,12 +53,6 @@ func runAttachment(ctx context.Context, dockerCli command.Cli, opts attachmentOp
 		return errors.Errorf("no record found for ref %q", opts.ref)
 	}
 
-	if opts.ref == "" {
-		slices.SortFunc(recs, func(a, b historyRecord) int {
-			return b.CreatedAt.AsTime().Compare(a.CreatedAt.AsTime())
-		})
-	}
-
 	rec := &recs[0]
 
 	c, err := rec.node.Driver.Client(ctx)

commands/history/logs.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"io"
 	"os"
-	"slices"
 
 	"github.com/docker/buildx/builder"
 	"github.com/docker/buildx/util/cobrautil/completion"
@@ -39,7 +38,7 @@ func runLogs(ctx context.Context, dockerCli command.Cli, opts logsOptions) error
 		}
 	}
 
-	recs, err := queryRecords(ctx, opts.ref, nodes)
+	recs, err := queryRecords(ctx, opts.ref, nodes, nil)
 	if err != nil {
 		return err
 	}
@@ -51,12 +50,6 @@ func runLogs(ctx context.Context, dockerCli command.Cli, opts logsOptions) error
 		return errors.Errorf("no record found for ref %q", opts.ref)
 	}
 
-	if opts.ref == "" {
-		slices.SortFunc(recs, func(a, b historyRecord) int {
-			return b.CreatedAt.AsTime().Compare(a.CreatedAt.AsTime())
-		})
-	}
-
 	rec := &recs[0]
 	c, err := rec.node.Driver.Client(ctx)
 	if err != nil {

commands/history/ls.go

@@ -56,7 +56,7 @@ func runLs(ctx context.Context, dockerCli command.Cli, opts lsOptions) error {
 		}
 	}
 
-	out, err := queryRecords(ctx, "", nodes)
+	out, err := queryRecords(ctx, "", nodes, nil)
 	if err != nil {
 		return err
 	}

commands/history/open.go

@@ -3,7 +3,6 @@ package history
 import (
 	"context"
 	"fmt"
-	"slices"
 
 	"github.com/docker/buildx/builder"
 	"github.com/docker/buildx/util/cobrautil/completion"
@@ -35,7 +34,7 @@ func runOpen(ctx context.Context, dockerCli command.Cli, opts openOptions) error
 		}
 	}
 
-	recs, err := queryRecords(ctx, opts.ref, nodes)
+	recs, err := queryRecords(ctx, opts.ref, nodes, nil)
 	if err != nil {
 		return err
 	}
@@ -47,12 +46,6 @@ func runOpen(ctx context.Context, dockerCli command.Cli, opts openOptions) error
 		return errors.Errorf("no record found for ref %q", opts.ref)
 	}
 
-	if opts.ref == "" {
-		slices.SortFunc(recs, func(a, b historyRecord) int {
-			return b.CreatedAt.AsTime().Compare(a.CreatedAt.AsTime())
-		})
-	}
-
 	rec := &recs[0]
 
 	url := desktop.BuildURL(fmt.Sprintf("%s/%s/%s", rec.node.Builder, rec.node.Name, rec.Ref))

commands/history/trace.go

@@ -8,9 +8,6 @@ import (
 	"io"
 	"net"
 	"os"
-	"slices"
-	"strconv"
-	"strings"
 	"time"
 
 	"github.com/containerd/console"
@@ -37,51 +34,20 @@ type traceOptions struct {
 }
 
 func loadTrace(ctx context.Context, ref string, nodes []builder.Node) (string, []byte, error) {
-	var offset *int
-	if strings.HasPrefix(ref, "^") {
-		off, err := strconv.Atoi(ref[1:])
-		if err != nil {
-			return "", nil, errors.Wrapf(err, "invalid offset %q", ref)
-		}
-		offset = &off
-		ref = ""
-	}
-
-	recs, err := queryRecords(ctx, ref, nodes)
+	recs, err := queryRecords(ctx, ref, nodes, &queryOptions{
+		CompletedOnly: true,
+	})
 	if err != nil {
 		return "", nil, err
 	}
 
-	var rec *historyRecord
-
-	if ref == "" {
-		slices.SortFunc(recs, func(a, b historyRecord) int {
-			return b.CreatedAt.AsTime().Compare(a.CreatedAt.AsTime())
-		})
-		for _, r := range recs {
-			if r.CompletedAt != nil {
-				if offset != nil {
-					if *offset > 0 {
-						*offset--
-						continue
-					}
-				}
-				rec = &r
-				break
-			}
-		}
-		if offset != nil && *offset > 0 {
-			return "", nil, errors.Errorf("no completed build found with offset %d", *offset)
-		}
-	} else {
-		rec = &recs[0]
-	}
-	if rec == nil {
+	if len(recs) == 0 {
 		if ref == "" {
 			return "", nil, errors.New("no records found")
 		}
 		return "", nil, errors.Errorf("no record found for ref %q", ref)
 	}
+	rec := &recs[0]
 
 	if rec.CompletedAt == nil {
 		return "", nil, errors.Errorf("build %q is not completed, only completed builds can be traced", rec.Ref)
@@ -103,7 +69,9 @@ func loadTrace(ctx context.Context, ref string, nodes []builder.Node) (string, [
 			return "", nil, err
 		}
 
-		recs, err := queryRecords(ctx, rec.Ref, []builder.Node{*rec.node})
+		recs, err := queryRecords(ctx, rec.Ref, []builder.Node{*rec.node}, &queryOptions{
+			CompletedOnly: true,
+		})
 		if err != nil {
 			return "", nil, err
 		}

commands/history/utils.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"io"
 	"path/filepath"
+	"slices"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -106,10 +108,24 @@ type historyRecord struct {
 	name             string
 }
 
-func queryRecords(ctx context.Context, ref string, nodes []builder.Node) ([]historyRecord, error) {
+type queryOptions struct {
+	CompletedOnly bool
+}
+
+func queryRecords(ctx context.Context, ref string, nodes []builder.Node, opts *queryOptions) ([]historyRecord, error) {
 	var mu sync.Mutex
 	var out []historyRecord
 
+	var offset *int
+	if strings.HasPrefix(ref, "^") {
+		off, err := strconv.Atoi(ref[1:])
+		if err != nil {
+			return nil, errors.Wrapf(err, "invalid offset %q", ref)
+		}
+		offset = &off
+		ref = ""
+	}
+
 	eg, ctx := errgroup.WithContext(ctx)
 	for _, node := range nodes {
 		node := node
@@ -153,6 +169,10 @@ func queryRecords(ctx context.Context, ref string, nodes []builder.Node) ([]hist
 				if he.Type == controlapi.BuildHistoryEventType_DELETED || he.Record == nil {
 					continue
 				}
+				if opts != nil && opts.CompletedOnly && he.Type != controlapi.BuildHistoryEventType_COMPLETE {
+					continue
+				}
+
 				records = append(records, historyRecord{
 					BuildHistoryRecord: he.Record,
 					currentTimestamp:   ts,
@@ -169,6 +189,27 @@ func queryRecords(ctx context.Context, ref string, nodes []builder.Node) ([]hist
 	if err := eg.Wait(); err != nil {
 		return nil, err
 	}
+
+	slices.SortFunc(out, func(a, b historyRecord) int {
+		return b.CreatedAt.AsTime().Compare(a.CreatedAt.AsTime())
+	})
+
+	if offset != nil {
+		var filtered []historyRecord
+		for _, r := range out {
+			if *offset > 0 {
+				*offset--
+				continue
+			}
+			filtered = append(filtered, r)
+			break
+		}
+		if *offset > 0 {
+			return nil, errors.Errorf("no completed build found with offset %d", *offset)
+		}
+		out = filtered
+	}
+
 	return out, nil
 }