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

build: add options to builder prune #1327

Merged
merged 2 commits into from
Sep 5, 2018

Conversation

tiborvass
Copy link
Collaborator

@tiborvass tiborvass commented Aug 30, 2018

Signed-off-by: Tibor Vass tibor@docker.com

Related to moby/moby#37651

This patch fixes the UX around build cache:

  • Adds new options to builder prune: --filter, --keep-storage, --all, --force. This is needed to to avoid situations where user needs to prune all their cache, and they can thus prune only a subset of their cache.
  • Shows table layout for build cache in system df consistent with the way other types are shown (containers, images, volumes...)

@codecov-io
Copy link

codecov-io commented Aug 30, 2018

Codecov Report

Merging #1327 into master will decrease coverage by 0.18%.
The diff coverage is 17.39%.

@@            Coverage Diff             @@
##           master    #1327      +/-   ##
==========================================
- Coverage   54.82%   54.64%   -0.19%     
==========================================
  Files         292      293       +1     
  Lines       19267    19360      +93     
==========================================
+ Hits        10564    10580      +16     
- Misses       8044     8119      +75     
- Partials      659      661       +2

@tiborvass tiborvass force-pushed the cmd-builder-prune-with-options branch 7 times, most recently from e89e1c3 to 82ee8e1 Compare September 4, 2018 14:54
@tiborvass tiborvass changed the title [WIP] build: add options to builder prune build: add options to builder prune Sep 4, 2018
Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

sort.Slice(buildCache, func(i, j int) bool {
lui, luj := buildCache[i].LastUsedAt, buildCache[j].LastUsedAt
switch {
case lui == luj:
Copy link
Member

Choose a reason for hiding this comment

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

explicit nil check is more readable

case lui.Equal(*luj):
return strings.Compare(buildCache[i].ID, buildCache[j].ID) < 0
}
return lui.Before(*luj)
Copy link
Member

Choose a reason for hiding this comment

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

nit: default:

for _, v := range ctx.BuildCache {
t.Execute(ctx.Output, *v)
if !v.Shared {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't filter in here, but in the summary bytes.

This patch adds --filter, --keep-storage, --all and --force to builder prune.

Signed-off-by: Tibor Vass <tibor@docker.com>
@tiborvass tiborvass force-pushed the cmd-builder-prune-with-options branch 7 times, most recently from 3c46602 to b774911 Compare September 5, 2018 03:30
Signed-off-by: Tibor Vass <tibor@docker.com>
@tiborvass tiborvass force-pushed the cmd-builder-prune-with-options branch from b774911 to ca608c2 Compare September 5, 2018 03:31
Copy link
Contributor

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

SGTM

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.

7 participants