Re: [RFC PATCH 0/1] ci: Speed up container stage

[Fabiano Rosas <farosas@suse.de>]

Daniel P. Berrangé <berrange@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.