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

bud: layer cache is broken with --build-args #2992

Closed
terceiro opened this issue Feb 10, 2021 · 0 comments · Fixed by #2993
Closed

bud: layer cache is broken with --build-args #2992

terceiro opened this issue Feb 10, 2021 · 0 comments · Fixed by #2993

Comments

@terceiro
Copy link
Contributor

terceiro commented Feb 10, 2021

Description

Building images with the same build args does not use the cache. This worked in v0.19.2, and has been broken by 9b29958. #2967 fixes it for builds with no build args, but having build args makes the cache not be used..

Steps to reproduce the issue:

I'm using the following files:

bisect.sh:

set -eu
if [ -f Makefile ]; then
    make bin/buildah
    basedir=$(pwd)
    export PATH=$(pwd)/bin:$PATH
fi
cd $(dirname $0)
set -x
buildah bud --layers=true --build-arg=N=7
buildah bud --layers=true --build-arg=N=7 . | tee /tmp/log
! grep '^7' /tmp/log

Containerfile:

FROM debian
ARG N
RUN seq 1 ${N}

Then from the buildah source tree, I run the bisect script. First, on the master branch from today (0508fba):

buildah (master) $ sh /tmp/foobar/bisect.sh 
GO111MODULE=on go build -mod=vendor -ldflags '-X main.GitCommit=0508fba0 -X main.buildInfo=1612918285 -X main.cniVersion=v0.8.1 ' -gcflags "" -o bin/buildah -tags "seccomp apparmor btrfs_noversion exclude_graphdriver_btrfs " ./cmd/buildah
+ buildah bud --layers=true --build-arg=N=7
STEP 1: FROM debian
STEP 2: ARG N
--> Using cache c352def690165a2c9a87a59bee469509afcdedf4869d66ac1f100d04667f42ad
--> c352def6901
STEP 3: RUN seq 1 ${N}
1
2
3
4
5
6
7
--> Using cache eec66aa2b27ca0195ff64ab5d7d49081e20a1b62e04311bd64dcf875e1e39505
--> eec66aa2b27
eec66aa2b27ca0195ff64ab5d7d49081e20a1b62e04311bd64dcf875e1e39505
+ buildah bud --layers=true --build-arg=N=7 .
+ tee /tmp/log
STEP 1: FROM debian
STEP 2: ARG N
--> Using cache c352def690165a2c9a87a59bee469509afcdedf4869d66ac1f100d04667f42ad
--> c352def6901
STEP 3: RUN seq 1 ${N}
1
2
3
4
5
6
7
--> Using cache eec66aa2b27ca0195ff64ab5d7d49081e20a1b62e04311bd64dcf875e1e39505
--> eec66aa2b27
eec66aa2b27ca0195ff64ab5d7d49081e20a1b62e04311bd64dcf875e1e39505
+ set +x
7
FAIL

Now, against the latest release where this worked (v0.19.2):

buildah (good) $ git describe --tags
v1.19.2
buildah (good) $ sh /tmp/foobar/bisect.sh 
GO111MODULE=on go build -mod=vendor -ldflags '-X main.GitCommit=7b83c4be -X main.buildInfo=1612918406 -X main.cniVersion=v0.7.2-0.20190904153231-83439463f784 ' -gcflags "" -o bin/buildah -tags "seccomp apparmor btrfs_noversion exclude_graphdriver_btrfs " ./cmd/buildah
+ buildah bud --layers=true --build-arg=N=7
STEP 1: FROM debian
STEP 2: ARG N
--> Using cache abaed72f15975cdfc19f9050cdee3aa0c1b653a1e55843a40614f6b9977a3781
--> abaed72f159
STEP 3: RUN seq 1 ${N}
--> Using cache de565e075cbbf490df04094afcf0e612f863ccf819b7b44035870afdbd31b789
--> de565e075cb
de565e075cbbf490df04094afcf0e612f863ccf819b7b44035870afdbd31b789
+ buildah bud --layers=true --build-arg=N=7 .
+ tee /tmp/log
STEP 1: FROM debian
STEP 2: ARG N
--> Using cache abaed72f15975cdfc19f9050cdee3aa0c1b653a1e55843a40614f6b9977a3781
--> abaed72f159
STEP 3: RUN seq 1 ${N}
--> Using cache de565e075cbbf490df04094afcf0e612f863ccf819b7b44035870afdbd31b789
--> de565e075cb
de565e075cbbf490df04094afcf0e612f863ccf819b7b44035870afdbd31b789
+ set +x

(the later run reuses the layers created by the one from master and cached, so even the first build already does not execute the RUN instruction). But if I change the 7 in the script to something else, say 8, it works as intended:

buildah (good) $ git describe --tags
v1.19.2
buildah (good) $ sh /tmp/foobar/bisect.sh 
GO111MODULE=on go build -mod=vendor -ldflags '-X main.GitCommit=7b83c4be -X main.buildInfo=1612918557 -X main.cniVersion=v0.7.2-0.20190904153231-83439463f784 ' -gcflags "" -o bin/buildah -tags "seccomp apparmor btrfs_noversion exclude_graphdriver_btrfs " ./cmd/buildah
+ buildah bud --layers=true --build-arg=N=8
STEP 1: FROM debian
STEP 2: ARG N
--> Using cache abaed72f15975cdfc19f9050cdee3aa0c1b653a1e55843a40614f6b9977a3781
--> abaed72f159
STEP 3: RUN seq 1 ${N}
1
2
3
4
5
6
7
8
STEP 4: COMMIT
--> 19592c29913
19592c29913abb10d85d8c1086d060520bdfc415eed64272b33a4c425698dcd1
+ buildah bud --layers=true --build-arg=N=8 .
+ tee /tmp/log
STEP 1: FROM debian
STEP 2: ARG N
--> Using cache abaed72f15975cdfc19f9050cdee3aa0c1b653a1e55843a40614f6b9977a3781
--> abaed72f159
STEP 3: RUN seq 1 ${N}
--> Using cache 19592c29913abb10d85d8c1086d060520bdfc415eed64272b33a4c425698dcd1
--> 19592c29913
19592c29913abb10d85d8c1086d060520bdfc415eed64272b33a4c425698dcd1
+ grep ^8 /tmp/log

cc @umohnani8

terceiro added a commit to terceiro/buildah that referenced this issue Feb 10, 2021
This fixes a regression introduced in
9b29958. Adding the build args
explicitly in getCreatedBy() is enough to force a rebuild when build
args change.

On the other hand, if you run ib.Run() whenever build args is not empty,
you re-run the entire build even when using the same build args as
previous builds.

Closes containers#2992

Signed-off-by: Antonio Terceiro <[email protected]>
terceiro added a commit to terceiro/buildah that referenced this issue Feb 10, 2021
This fixes a regression introduced in
9b29958. Adding the build args
explicitly in getCreatedBy() is enough to force a rebuild when build
args change.

On the other hand, if you run ib.Run() whenever build args is not empty,
you re-run the entire build even when using the same build args as
previous builds.

Closes containers#2992

Signed-off-by: Antonio Terceiro <[email protected]>
terceiro added a commit to terceiro/buildah that referenced this issue Feb 10, 2021
This fixes a regression introduced in
9b29958.

ib.Run() is only really needed in the ARG step. On all the other steps,
it can cause potentially expensive commands to be executed unecessarily.

Closes containers#2992

Signed-off-by: Antonio Terceiro <[email protected]>
Signed-off-by: Urvashi Mohnani <[email protected]>
rhatdan pushed a commit to rhatdan/buildah that referenced this issue Feb 11, 2021
This fixes a regression introduced in
9b29958.

ib.Run() is only really needed in the ARG step. On all the other steps,
it can cause potentially expensive commands to be executed unecessarily.

Closes containers#2992

Signed-off-by: Antonio Terceiro <[email protected]>
Signed-off-by: Urvashi Mohnani <[email protected]>
umohnani8 pushed a commit to umohnani8/buildah that referenced this issue Feb 11, 2021
This fixes a regression introduced in
9b29958.

ib.Run() is only really needed in the ARG step. On all the other steps,
it can cause potentially expensive commands to be executed unecessarily.

Closes containers#2992

Signed-off-by: Antonio Terceiro <[email protected]>
Signed-off-by: Urvashi Mohnani <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant