Daniel P. Berrangé <berra...@redhat.com> writes:
> On Thu, Feb 23, 2023 at 11:21:53AM -0300, Fabiano Rosas wrote:
>> I'm not sure if this was discussed previously, but I noticed we're not
>> pulling the images we push to the registry at every pipeline run.
>>
>> I would expect we don't actually need to rebuild container images at
>> _every_ pipeline run, so I propose we add a "docker pull" to the
>> container templates. We already have that for the docker-edk2|opensbi
>> images.
>>
>> Some containers can take a long time to build (14 mins) and pulling
>> the image first without building can cut the time to about 3
>> mins. With this we can save almost 2h of cumulative CI time per
>> pipeline run:
>
> The docker.py script that we're invoking is already pulling the
> image itself eg to pick a random recent job:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/3806090058
>
> We can see
>
> $ ./tests/docker/docker.py --engine docker build -t "qemu/$NAME" -f
> "tests/docker/dockerfiles/$NAME.docker" -r $CI_REGISTRY/qemu-project/qemu
> 03:54
> Using default tag: latest
> latest: Pulling from qemu-project/qemu/qemu/debian-arm64-cross
> bb263680fed1: Pulling fs layer
> ...snip...
Ah right, so this is different for user's pipelines because the push at
the end of the build goes to the user's registry:
registry.gitlab.com/farosas/qemu/qemu/debian-arm64-cross
So we're fetching from one place and pushing to a different one. That is
why I see the improvement.
> none the less it still went ahead and rebuilt the image from scratch
It seems docker.py does not see that we're trying to build a tag that is
already there. Could this be due to --cache-from being "disabled"?
if ("docker" in self._command and
"TRAVIS" not in os.environ and
"GITLAB_CI" not in os.environ):
os.environ["DOCKER_BUILDKIT"] = "1"
self._buildkit = True
else:
self._buildkit = False
> so something is going wrong here. I don't know why your change adding
> an extra 'docker pull' would have any effect, given we're already
> pulling, so I wonder if that's just coincidental apparent change
> due to the initial state of your fork's container registery.
>
> Whenever I look at this I end up wishing out docker.py didn't exist
> and that we could just directly do
>
> - docker pull "$TAG"
> - docker build --cache-from "$TAG" --tag "$TAG" -f
> "tests/docker/$NAME.docker"
I see that in the docker-openbsi image we do just that.
> as that sould be sufficient to build the image with caching.
>
>> We would need to devise a mechanism (not included here) to force the
>> re-build of the container images when needed, perhaps an environment
>> variable or even a whole new "container build" stage before the
>> "container" stage.
>>
>> What do you think?
>
> We definitely want the rebuild to be cached. So whatever is
> broken in that regard needs fixing, as this used to work AFAIK.
>
>
> Ideally we would skip the container stage entirely for any
> pull request that did NOT include changes to the dockerfile.
Agreed.
