imagebuildah: reorganize stage and per-stage logic #1473
Conversation
When we're not building with multiple layers, add empty layer items in the image's history so that the instructions from the Dockerfile don't get lost. Signed-off-by: Nalin Dahyabhai <[email protected]>
Move the preparation for the first container and the commit of the final image in a single-stage build into the per-stage Execute() function. For multi-stage builds, only create a layer for ADD, COPY, RUN, and the final instruction in a stage, which changes the step counts that we output. Stop removing the final image for intermediate stages, since they're expected to be present as cached images, even if they don't end up as ancestors of the final image, except for single-stage builds, which we document as not populating the cache. Signed-off-by: Nalin Dahyabhai <[email protected]>
|
This should fix #1405. |
There was a problem hiding this comment.
Does this cure containers/podman#2823 which had this as a proposed fix #1466?
|
We shouldn't be leaving unintended containers after #1446, and leaving intermediate images around is consistent with containers/podman#2823 might be the TODO that it added, which this should resolve. |
|
@nalind thanks. I just verified with Docker-ce and they are leaving an image `' behind for this, along with the images from the "FROM" clauses. My #1146 should not be removing those images I also verified that Docker also does not remove the containers, so perhaps that shouldn't be done either? Now for the weird part, when I was looking at this earlier, there were no images with '' tagged but now there are. Has some other new code gotten in that might have fixed that or have I completely lost it? |
|
Before this PR, we'd remove the image that was created for stages other than the last one (i.e., the intermediate stages) when we were finished, which I recently noticed differed from how |
|
LGTM |
|
LGTM |
|
📌 Commit 1892138 has been approved by |
Move the preparation for the first container and the commit of the final image in a single-stage build into the per-stage Execute() function. For multi-stage builds, only create a layer for ADD, COPY, RUN, and the final instruction in a stage, which changes the step counts that we output. Stop removing the final image for intermediate stages, since they're expected to be present as cached images, even if they don't end up as ancestors of the final image, except for single-stage builds, which we document as not populating the cache. Signed-off-by: Nalin Dahyabhai <[email protected]> Closes: #1473 Approved by: rhatdan
|
☀️ Test successful - status-papr, status-travis |
When we're not building with multiple layers, add empty layer items in the image's history so that the instructions from the Dockerfile don't get lost.
Move the preparation for the first container and the commit of the final image in a single-stage build into the per-stage
Execute()function.For multi-stage builds, only create a layer for
ADD,COPY,RUN, and the final instruction in a stage, which changes the number of cache images that we create, so a number of tests needed to have their expectations updated.It also affects the step counts that we output, so a test in the last hunk we change in
bud.batsneeded to be updated to expect the changed value. In the interests of not making this patch bigger, I expect to include the fix and change that back in a later PR.Stop removing the final image for intermediate stages, since they're expected to be present as cached images, even if they don't end up as ancestors of the final image, except for single-stage builds, which we document as not populating the cache.