ci: Prune only docker images built for current build #11222
Conversation
|
Please set the appropriate release note label. |
|
test-me-please |
Makefile
Outdated
| @@ -226,6 +226,7 @@ docker-image-no-clean: GIT_VERSION | |||
| --build-arg LOCKDEBUG=${LOCKDEBUG} \ | |||
| --build-arg V=${V} \ | |||
| --build-arg LIBNETWORK_PLUGIN=${LIBNETWORK_PLUGIN} \ | |||
| --build-arg CILIUM_SHA=$$(cat GIT_VERSION | cut -d" " -f1 | tr -d '\n') \ | |||
There was a problem hiding this comment.
Could this be set with docker build --label in stead?
There was a problem hiding this comment.
I think you what you are looking for is $(GIT_VERSION), it should be set already. And let's not invent a new name, just call it git_version or cilium_git_version.
I'd just use the whole string, i.e. --label "cilium_git_version=$(GIT_VERSION)", or otherwise --label "cilium_git_version=$(firstword $(GIT_VERSION))" if you prefer to use just the short commit hash.
There was a problem hiding this comment.
We cannot use --label here, because it only labels the final image. We want to label intermediate images too so we know which ones to prune.
Good catch with firstword usage, I didn't know I can use it like that, will change.
There was a problem hiding this comment.
I see, seems suboptimal but acceptable :)
| @@ -12,3 +12,5 @@ docker tag cilium/operator:$2 $1/cilium/operator:$2 | |||
| docker push $1/cilium/cilium:$2 | |||
| docker push $1/cilium/cilium-dev:$2 | |||
| docker push $1/cilium/operator:$2 | |||
|
|
|||
| docker image prune -f --all --filter "label=cilium-sha=$(cat GIT_VERSION | cut -d' ' -f1 | tr -d '\n')" | |||
There was a problem hiding this comment.
cilium_git_version="$(cat GIT_VERSION)"
docker image prune -f --all --filter "label=cilium_git_version=${cilium_git_version%% *}"`
There was a problem hiding this comment.
I couldn't get this to work locally in zsh and bash :/ , it seems like a magic bash variable expansion, but I am not sure how this works.
There was a problem hiding this comment.
Apologies for the typo, I fixed it now for the record, and thanks for accepting this suggestions :)
| @@ -21,4 +21,3 @@ docker push $1/cilium/cilium:$2 | |||
| docker push $1/cilium/cilium-dev:$2 | |||
| docker push $1/cilium/operator:$2 | |||
|
|
|||
| docker image prune -f --all --filter "until=-6h" | |||
There was a problem hiding this comment.
Why not change it to docker image prune --force --filter "until=-6h" , so we keep cleaning the dangling images (surely there will be some of those).
There was a problem hiding this comment.
because this deletes intermediate docker images while other build is running and causes docker build to fail.
There was a problem hiding this comment.
Well, I was thinking that the 6h window should actually prevent that, or does it not work as intended? Maybe we could extend the window?
There was a problem hiding this comment.
So my theory on what is happening here is that
- a build (A) downloads base images for intermediate images and carries on with the build
- 6 hours pass, other build (B) starts
- intermediate images are based on existing over 6h old images
- other build (C) runs "docker image prune" and deletes the base image causing build B to fail
docker apparently doesn't protect images used in multi-stage builds, so we need to work around that.
There was a problem hiding this comment.
Hm... but that's based on having ran this script --all, so it was actually deleting every single image that wasn't used by a container. I think if we remove --all, we would make different observations, however some of what you said would stand anyway.
Anyway, let's just go ahead with your change, and I'd rather spend time discussing how we can evolve some of this.
nebril
force-pushed
the
pr/fix-docker-prune
branch
2 times, most recently
from
e9dc0b4
to
c4e8407
Compare
|
@errordeveloper please re-review |
|
test-me-please |
|
Ginkgo hit #11213 test-focus K8sFQDNTest Restart Cilium validate that FQDN is still working |
|
Conflict in Makefile on |
qmonnet
added
the
dont-merge/needs-rebase
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
nebril
force-pushed
the
pr/fix-docker-prune
branch
from
c4e8407
to
580c030
Compare
|
test-me-please |
|
restart-ginkgo |
|
test-with-kernel |
|
This change only fixes things if other PRs rebase on top of it, so I am going to merge it with GKE build failing |
This PR should fix GKE builds failures happening on docker build stage (we are concurrently building and image and pruning images aggresively, this change makes the build clean up after itself).