Skip to content

Add squash-all, fix squash option in build#4215

Merged
openshift-merge-robot merged 1 commit intocontainers:masterfrom
TomSweeneyRedHat:dev/tsweeney/fixsquash
Oct 15, 2019
Merged

Add squash-all, fix squash option in build#4215
openshift-merge-robot merged 1 commit intocontainers:masterfrom
TomSweeneyRedHat:dev/tsweeney/fixsquash

Conversation

@TomSweeneyRedHat
Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat commented Oct 8, 2019

Translate the podman build --squash command to podman build --layers=false which
has the same functionality as docker build --squash. Add a new option --squash-all
which will squash all layers into one. This will be translated to buildah bud --squash
for the buildah bud api.

Also allow only one option, squash, layers or squash--all to be used per build command.

Fixes: containers/buildah#1234

Signed-off-by: TomSweeneyRedHat tsweeney@redhat.com

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M labels Oct 8, 2019
@TomSweeneyRedHat
Copy link
Member Author

I neglected to update the man page and bash completions, will tend to those once I get general consensus for this approach.

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TomSweeneyRedHat

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 8, 2019
@TomSweeneyRedHat TomSweeneyRedHat changed the title WIP - Add squash-all, fix squash Add squash-all, fix squash option in build Oct 8, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 8, 2019
@TomSweeneyRedHat
Copy link
Member Author

I've updated with @rhatdan's suggestion to have the squash-all option defined here rather than in buildahcli like the other options. I've also updated bash completions and the man pages. Assuming happy tests and LGTM's, this is good to go.

@TomSweeneyRedHat
Copy link
Member Author

Testing:

# touch alpinetest.tgz # or better yet create a tgz file with a few files in it.

# cat ~/Dockerfile.squash-a
FROM busybox:latest
ADD alpinetest.tgz /data

# cat ~/Dockerfile.squash-b
FROM test-squash-a:latest
RUN rm -rf /data

# cat ~/Dockerfile.squash-c
FROM busybox:latest
ADD alpinetest.tgz /data
RUN rm -rf /data

# podman build -f ~/Dockerfile.squash-a -t test-squash-a:latest .
STEP 1: FROM busybox:latest
STEP 2: ADD alpinetest.tgz /data
--> Using cache c54637689450d9516e6f2e497bb78f663405d9b609882ec89f4b481e84395eba
STEP 3: COMMIT test-squash-a:latest
c54637689450d9516e6f2e497bb78f663405d9b609882ec89f4b481e84395eba

# podman build -f ~/Dockerfile.squash-b --squash -t test-squash-b:latest .
STEP 1: FROM test-squash-a:latest
STEP 2: RUN rm -rf /data
STEP 3: COMMIT test-squash-b:latest
Getting image source signatures
Copying blob 6c0ea40aef9d skipped: already exists
Copying blob 2f2b43ce2ffb skipped: already exists
Copying blob 2f27bd025155 done
Copying config bb31c226ee done
Writing manifest to image destination
Storing signatures
bb31c226ee4317cbecd41747500e58c92c2f00c13df4e591d20571974a8a9109

# podman build -f ~/Dockerfile.squash-c --squash -t test-squash-c:latest .
STEP 1: FROM busybox:latest
STEP 2: ADD alpinetest.tgz /data
STEP 3: RUN rm -rf /data
STEP 4: COMMIT test-squash-c:latest
Getting image source signatures
Copying blob 6c0ea40aef9d skipped: already exists
Copying blob cb21ed953734 done
Copying config 7ccbafc877 done
Writing manifest to image destination
Storing signatures
7ccbafc877f09c403f455918e97bca9bad7895719baf5e43177fbd555202768f

# podman build -f ~/Dockerfile.squash-c --squash-all -t test-squash-d:latest .
STEP 1: FROM busybox:latest
STEP 2: ADD alpinetest.tgz /data
STEP 3: RUN rm -rf /data
STEP 4: COMMIT test-squash-d:latest
Getting image source signatures
Copying blob 2e9938a66ae2 done
Copying config 968b309097 done
Writing manifest to image destination
Storing signatures
968b3090976361347cd9e40ee7e8d98c1cba9692524c94abce6d7f3101463c5a

# podman inspect --format "{{.RootFS.Layers}}" test-squash-a
[sha256:6c0ea40aef9d2795f922f4e8642f0cd9ffb9404e6f3214693a1fd45489f38b44 sha256:2f2b43ce2ffb410197ef7c43d96e132a1d12a24519f1fc0eb3598afd9c912ccd]

# podman inspect --format "{{.RootFS.Layers}}" test-squash-b
[sha256:6c0ea40aef9d2795f922f4e8642f0cd9ffb9404e6f3214693a1fd45489f38b44 sha256:2f2b43ce2ffb410197ef7c43d96e132a1d12a24519f1fc0eb3598afd9c912ccd sha256:5a47f575b5d22f740a0689eb1e72ba71fb1491e22a5740bef87023c64c7605e6]

# podman inspect --format "{{.RootFS.Layers}}" test-squash-c
[sha256:6c0ea40aef9d2795f922f4e8642f0cd9ffb9404e6f3214693a1fd45489f38b44 sha256:46f19664b40a2e34c2cab5d6ead1fefcba940c4bbda55ae742206cef82edebbd]

# podman inspect --format "{{.RootFS.Layers}}" test-squash-d
[sha256:c84a24cbfc61792badfacf0d6831d3ec5fa406f0a93cb0ad7b6e9b15bbb6ed54]

@rhatdan
Copy link
Member

rhatdan commented Oct 9, 2019

LGTM

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably #4201) made this pull request unmergeable. Please resolve the merge conflicts.

Translate the podman build --squash command to podman build --layers=false which
has the same functionality as docker build --squash. Add a new option --squash-all
which will squash all layers into one. This will be translated to buildah bud --squash
for the buildah bud api.

Also allow only one option, squash, layers or squash--all to be used per build command.

Fixes: containers/buildah#1234

Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
@TomSweeneyRedHat
Copy link
Member Author

Somethings wrong with the CI tests on this, they're all green...

@rhatdan
Copy link
Member

rhatdan commented Oct 14, 2019

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

The changes LGTM but I'd love us to have some tests as we could regress (for whatever) reason in Podman while Buildah continues working. Simple tests could be to use the different flags and use podman-inspect to make sure we have the expected amount of layers (e.g., podman inspect f945ea07f224 -f "{{len .RootFS.Layers}}"). We also should test that invalid combinations of the flags yield errors.

@rhatdan
Copy link
Member

rhatdan commented Oct 15, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2019
@openshift-merge-robot openshift-merge-robot merged commit 9358025 into containers:master Oct 15, 2019
@vrothberg
Copy link
Member

Eeek. I didn't ack this PR. Without tests we are likely to silently regress. @TomSweeneyRedHat, mind opening another PR to add some tests?

@TomSweeneyRedHat TomSweeneyRedHat deleted the dev/tsweeney/fixsquash branch January 8, 2020 18:39
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

podman build --squash also squashes layers of base container

6 participants