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

Add buildx history command #2891

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tonistiigi
Copy link
Member

@tonistiigi tonistiigi commented Jan 7, 2025

implements #2711

These commands allow working with build records
of completed and running builds.

Currently supports:

history ls
history logs
history rm
history open
history inspect
history inspect attachment

docs/reference/buildx_history_rm.md Outdated Show resolved Hide resolved
docs/reference/buildx.md Outdated Show resolved Hide resolved
"golang.org/x/sync/errgroup"
)

func buildName(fattrs map[string]string, ls *localstate.State) string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using the same logic in Docker Desktop. Could we make this one public and use it in DD?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't directly the same because the types are different, so I think it can be figured out in follow-up.

Comment on lines +224 to +232
if c.format.IsTable() {
if c.isTerm {
return desktop.ANSIHyperlink(url, "Open")
}
return ""
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was going to comment about the table that could be quite huge with the link but with this hyperlink this looks great.

commands/history/ls.go Outdated Show resolved Hide resolved
@tonistiigi tonistiigi force-pushed the history-command-initial branch 2 times, most recently from 1f03cd2 to dfdd89d Compare January 8, 2025 05:27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's now also a docker desktop CLI plugin. I think it would make more sense to put this feature in there, e.g. docker desktop open build <id>. WDYT?

lsHeaderDuration = "DURATION"
lsHeaderLink = ""

lsDefaultTableFormat = "table {{.Ref}}\t{{.Name}}\t{{.Status}}\t{{.CreatedAt}}\t{{.Duration}}\t{{.Link}}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have a builder column? At first sight I'm not sure what builder is used:

$ docker buildx history ls
BUILD ID                    NAME                                   STATUS      CREATED AT           DURATION   
6kfisaus77v2lng6uh013mqsg   buildx (binaries)                      Completed   About a minute ago   47.9s      Open
i2bwu6bu90hsgrcwt8ino4pdv   ..ckerfiles/docs.Dockerfile (update)   Completed   5 hours ago          1m  1s     Open
l75hmymzlopcthlmp1onrko60   buildx (binaries)                      Completed   5 hours ago          1m  0s     Open
2dujhidv8awc8b931fx27ok0l   ..ckerfiles/docs.Dockerfile (update)   Completed   5 hours ago          38.4s      Open
ohcjje6ufqa2q1y025yroxs45   ..ckerfiles/docs.Dockerfile (update)   Error       5 hours ago          47.3s      Open
2cd0dp6aez14aort5yqv0qt0s   ..ckerfiles/docs.Dockerfile (update)   Completed   5 hours ago          1m  0s     Open
sbdkqcz8pnpgk5o8hpep7efml   buildx (binaries)                      Completed   5 hours ago          56.4s      Open
1383761vlyc7v5zpvnu04x969   buildx (binaries)                      Completed   5 hours ago          58.2s      Open
usjztin7bie33n53748tme8uw   buildx (binaries)                      Completed   6 hours ago          1m 17s     Open
w6eq2yd1fp000sgcstdwb5kq2   ..ckerfiles/docs.Dockerfile (update)   Completed   8 hours ago          53.8s      Open
tn6je06lbx11w5446qk2mijil   ..ckerfiles/docs.Dockerfile (update)   Error       8 hours ago          46.7s      Open
zlzmiimgbujyoxrexg193j5mm   buildx (binaries)                      Completed   29 hours ago         1m 12s     Open
lapq0qhot3rt889b9zgfz6p3b   buildx (binaries)                      Error       29 hours ago         53.4s      Open
e65kwe3vw61fhzruia0qqd6vw   ..ion/dev.Dockerfile (format-update)   Completed   30 hours ago         25.9s      Open
w6kji8z2tutykbad1dykdvo4j   ..tion/dev.Dockerfile (build-update)   Completed   30 hours ago         34.2s      Open
39p6gvyf0bt1l3lnpr1wmicsl   ..ion/dev.Dockerfile (vendor-update)   Completed   30 hours ago         22.4s      Open
qbgt47jlt98e34in2skv88d1m   ..ion/dev.Dockerfile (format-update)   Completed   30 hours ago         25.2s      Open
0pxk73dru42im1cc6seee9k5o   ..ion/dev.Dockerfile (vendor-update)   Completed   30 hours ago         20.4s      Open

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But they all have the same builder

Copy link
Member

@crazy-max crazy-max Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True but I was thinking of smth similar to what we have done when building #1177 to have a hint of what builder it's coming from

Comment on lines +134 to +153
if _, ok := attrs["context"]; !ok {
if src, ok := attrs["vcs:source"]; ok {
fmt.Fprintf(tw, "VCS Repository:\t%s\n", src)
}
if rev, ok := attrs["vcs:revision"]; ok {
fmt.Fprintf(tw, "VCS Revision:\t%s\n", rev)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could keep the well-known git context format like:

VCS: https://github.com/tonistiigi/buildx#dfdd89d50e60af52129a88b47792c9784470f708

instead of:

VCS Repository: https://github.com/tonistiigi/buildx
VCS Revision:   dfdd89d50e60af52129a88b47792c9784470f708

These commands allow working with build records
of completed and running builds.

Signed-off-by: Tonis Tiigi <[email protected]>
@tonistiigi tonistiigi force-pushed the history-command-initial branch from dfdd89d to 12180f2 Compare January 10, 2025 06:10
@tonistiigi tonistiigi marked this pull request as ready for review January 10, 2025 06:10
@tonistiigi
Copy link
Member Author

Updated with completion of inspect and inspect attachment commands. Trace command has been separated to #2904 .

@tonistiigi tonistiigi force-pushed the history-command-initial branch from 12180f2 to 6489c8f Compare January 10, 2025 07:10
Comment on lines +297 to +310
if len(attachments) > 0 {
fmt.Fprintf(tw, "Attachments:\n")
tw = tabwriter.NewWriter(dockerCli.Out(), 1, 8, 1, '\t', 0)
fmt.Fprintf(tw, "DIGEST\tPLATFORM\tTYPE\n")
for _, a := range attachments {
p := ""
if a.platform != nil {
p = platforms.FormatAll(*a.platform)
}
fmt.Fprintf(tw, "%s\t%s\t%s\n", a.descr.Digest, p, descrType(a.descr))
}
tw.Flush()
fmt.Fprintln(dockerCli.Out())
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice this is what I was looking for on my side

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for this first iteration

As follow-up:

@tonistiigi
Copy link
Member Author

WDYT about #2891 (comment)?

#2891 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants