Podman: Regression: 1.2.0: 'build -t' tags even on error

[edsantiago]

# cat Dockerfile
FROM alpine
ENV FOO=bar
RUN /bin/false

# podman build   -t foo .
STEP 1: FROM alpine
STEP 2: MAINTAINER [email protected]
--> fe28c78afc45d14fce6320c9e13ec29fcf0f1826b26402c9560dda55ee815eef
STEP 3: FROM fe28c78afc45d14fce6320c9e13ec29fcf0f1826b26402c9560dda55ee815eef
STEP 4: RUN /bin/false
Error: error building at step {Env:[PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin] Command:run Args:[/bin/false] Flags:[] Attrs:map[] Message:RUN /bin/false Original:RUN /bin/false}: error while running runtime: exit status 1

So far so good, but:

# podman images 
REPOSITORY                                   TAG      IMAGE ID       CREATED         SIZE
localhost/foo                                latest   a3a4692e40b7   5 seconds ago   5.8 MB

(expected: an unnamed <none> image, not a named tag)

Only happens with multiple layers; does not fail with simple FROM alpine\nRUN false.

podman-1.2.0-2.git3bd528e.fc29.x86_64

[mheon]

@TomSweeneyRedHat looks like a buildah issue

[mheon]

@rhatdan We want to hold 1.2.0 until we can cut a 1.2.1 with this?

[TomSweeneyRedHat]

@mheon Just back on-line after my excursion. I'll take a look, I'm going to guess that some of Nalin's recent changes are at the root of this. I wouldn't hold the release for a fix to this, but @rhatdan's call. Certainly something to track and squish quickly though.

[TomSweeneyRedHat]

@edsantiago can you try this using upstream Buildah and buildah bud instead of podman build? That surprisingly seems to be working for me. Perhaps the vendor of Buildah didn't go well?

[edsantiago]

I was afraid you'd ask.

I've just tried buildah-1.7-2.git873f001.fc29.x86_64 and hand-building 9e1967ae9e510a12afbe28aeeb50e0921fc30ec5. Cannot reproduce on either, even with --no-cache=true --rm=true --force-rm=false

[TomSweeneyRedHat]

OK, I'm puzzled, I've gone through all vendoring for runc, imagebuilder, containers/storage, containers/image and buildah and everything seems to match between Buildah and Podman. I'm at a loss as to why this is working in Buildah, but not Podman.

[TomSweeneyRedHat]

And no sooner write that then light dawns on marblehead. Vendoring is not the issue. The reason it works in buildah is --layers==false by default, in podman --layers=true by default. The images isn't left behind if podman build --layers=false -t foo . is used, so something has changed in the layer handling.

[TomSweeneyRedHat]

OK, I've figured out the why, I'm not sure how to fix it yet. In short the ENV command in the Dockerfile is causing the image to be commited when layers are in use. But we're not saving that imageid into our list of images to cleanup in case of error. That list is relatively new functionality. That list is created in the Build() function of imagebuildah/build.go, but we're creating the image down in the Execute() command. I think what will need to be done is the list of images to cleanup needs to be passed also to the Execute() command so the ID can be added there.

Will look in the morning after my eyes our less crossed.

[TomSweeneyRedHat]

FWIW, I think I've a working fix in hand. Need to run through a bunch more testing, but hope to have a PR up by noon.

[TomSweeneyRedHat]

@edsantiago for a sanity check, could you try this again with the latest upstream Buildah? Use buildah bud --layers=true ... instead of podman build. I swear it wasn't working earlier this week, but appears to be now.

[edsantiago]

If I'm doing this correctly, I see the issue in master @ 9e1967ae9e510a12afbe28aeeb50e0921fc30ec5 :

# b bud --layers=true -t foo .
STEP 1: FROM alpine
STEP 2: MAINTAINER [email protected]
--> 1e90412843d1ca0e2fc475cd2245bba20712d1f382a597f8524b68d4f4d94177
STEP 3: FROM 1e90412843d1ca0e2fc475cd2245bba20712d1f382a597f8524b68d4f4d94177
STEP 4: RUN /bin/false
error building at step {Env:[PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin] Command:run Args:[/bin/false] Flags:[] Attrs:map[] Message:RUN /bin/false Original:RUN /bin/false}: error while running runtime: exit status 1
# b images
REPOSITORY                                   TAG      IMAGE ID       CREATED         SIZE
localhost/foo                                latest   1e90412843d1   3 seconds ago   5.8 MB
...

[TomSweeneyRedHat]

One more test please if you could. Can you confirm that it works OK without -t foo please? That's the difference, this afternoon I neglected the -t {name} when I tried it.

[edsantiago]

What does 'works OK' mean? The issue I'm griping about is that -t leaves a tagged, incomplete, incorrect image; I'm not sure I've given any thought to what the behavior should be without -t. (FWIW the behavior I see is that it leaves behind a <none> image, which seems reasonable at first glance).

[TomSweeneyRedHat]

Thanks @edsantiago. What I'd expect to see now without the -t flag is an image listed at <none> <none> and then an alpine image. That's what I think the state should be when running with the -t flag too.

[TomSweeneyRedHat]

grr github auto md convert. <none> <none> got swallowed in my last reply due to a lack of back ticks.

[TomSweeneyRedHat]

OK, this one's for @rhatdan and @nalind. The behavior for this functionality has changed in upstream. The image that was left behind that @edsantiago noted above is no longer there. Now only the alpine image is left behind.

# buildah bud --layers -t tom -f ~/Dockerfile.false .
STEP 1: FROM alpine
STEP 2: ENV FOO=bar
STEP 3: ENV BAR=foo
STEP 4: RUN /bin/false
error building at STEP "RUN /bin/false": error while running runtime: exit status 1
[root@localhost buildah]# buildah images
REPOSITORY                 TAG      IMAGE ID       CREATED      SIZE
docker.io/library/alpine   latest   cdf98d1859c1   2 days ago   5.8 MB

However with Docker-ce a <none> <none>' image is left behind and from what I could gather was equivalent to thelocalhost/fooimage from Ed's example, but tagged with `.

If this is OK as is then we could close this, but I think we probably should match Docker here.

[nalind]

We're no longer committing an image for steps 2 and 3, so it makes sense that we wouldn't see an unnamed intermediate image being left behind when step 4 fails. When we wire in support for committing without creating a layer, which we need anyway, then we'll be able to have images for steps 2 and 3.

[TomSweeneyRedHat]

@edsantiago is this new behavior described by @nalind above OK from your side? If so I'm going to close this and the related issue.

[edsantiago]

@nalind @TomSweeneyRedHat I'm really confused. The behavior I reported in the first comment is still present (a87cf6fef8f84ded410d547aa772b8f942c5f273) and still IMO very wrong: a build that fails should not be tagged. When you try the exact sequence given in my first comment, do you not see the localhost/foo image? Am I doing something wrong?

[TomSweeneyRedHat]

@edsantiago With the latest from upstream, I'm seeing the below. I'm not seeing a 'foo' image there at all. However if you compare against Docker you do see a <none> <none> image in addition to the alpine image. Were you by chance using not the latest from upstream in your test?

# buildah bud --layers=true -t foo -f ~/Dockerfile.false .
STEP 1: FROM alpine
Getting image source signatures
Copying blob bdf0201b3a05 done
Copying config cdf98d1859 done
Writing manifest to image destination
Storing signatures
STEP 2: ENV FOO=bar
STEP 3: ENV BAR=foo
STEP 4: RUN /bin/false
error building at STEP "RUN /bin/false": error while running runtime: exit status 1
[root@localhost ~]# buildah images
REPOSITORY                 TAG      IMAGE ID       CREATED      SIZE
docker.io/library/alpine   latest   cdf98d1859c1   8 days ago   5.8 MB

[edsantiago]

@TomSweeneyRedHat I'm looking at podman, not buildah. I would prefer not to close this issue until podman exhibits the correct behavior.

[TomSweeneyRedHat]

That explains it. Yep, not yet working in Podman, only in Buildah and hasn't been vendored yet. Thanks @edsantiago

[rhatdan]

Lets cut a new version of buildah and get it vendored into Podman, I think it is time for a new buildah.

[TomSweeneyRedHat]

We've a couple of good layer changes going in plus I want to take another hack at that symlink issue. I'll plan to cut a new Buildah early next week, holler if that doesn't work.

[rhatdan]

No that works for me. I want to cut a new containers/storage and containers/image as well and get them vendored into buildah

[baude]

@TomSweeneyRedHat did this get fixed?

[nalind]

The changes to intermediate images and tagging that were planned should all be landed by now. A spot-check with podman 1.3.1 from the Fedora podman-1.3.1-1.git7210727.fc30.x86_64 package appears to confirm this: a failed build doesn't leave behind an intermediate image with the name that was specified for the final image.