Re: [PATCH 3/3] automation: add a QEMU based x86_64 Dom0/DomU test

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Oct 22, 2021 at 01:05:35PM -0700, Stefano Stabellini wrote:
> On Fri, 22 Oct 2021, Anthony PERARD wrote:
> > On Thu, Oct 21, 2021 at 04:08:39PM -0700, Stefano Stabellini wrote:
> > > diff --git a/automation/scripts/qemu-alpine-x86_64.sh 
> > > b/automation/scripts/qemu-alpine-x86_64.sh
> > > new file mode 100644
> > > index 0000000000..41b05210d6
> > > --- /dev/null
> > > +++ b/automation/scripts/qemu-alpine-x86_64.sh
> > > @@ -0,0 +1,92 @@
> > > +#!/bin/bash
> > > +
> > > +set -ex
> > > +
> > > +apt-get -qy update
> > > +apt-get -qy install --no-install-recommends qemu-system-x86 \
> > > +                                            cpio \
> > > +                                            curl \
> > > +                                            busybox-static
> > 
> > Please, don't install packages during the CI job. If you need new
> > packages, update the container.
> 
> The container used to run this script is the one used for the Xen build:
> automation/build/debian/unstable.dockerfile, AKA
> registry.gitlab.com/xen-project/xen/debian:unstable. We don't have a
> specific container for the sole purpose of running tests.

I've added qemu to our debian:stretch container recently, in order to
run the "qemu-smoke-*" tests without running apt commands. Unless more
recent software are needed, you could base the "qemu-alpine-x86*" test
on debian:stretch which might only then missing cpio and busybox. Update
of the stretch container should go smoothly as it has been done
recently.

> Thus, I could add qemu-system-x86 to
> automation/build/debian/unstable.dockerfile, but then we increase the
> size of the debian unstable build container needlessly for all related
> build jobs.

There is something I'm missing, how is it a problem to have a container
that is a bit bigger? What sort of problem could we have to deal with?
On the other hand, doing more task, installing software, downloading
anything from the internet, makes the job much less reliable. It
increase the risk of a failure do to external factors and it takes more
time to run the test.

> Or we could add one more special container just for running tests, but
> then it is one more container to maintain and keep up-to-date.
> 
> This is why I chose as a compromise to keep the underlying container as
> is, and just apt-get the extra 3-4 packages here. It is the same thing
> we do on ARM: automation/scripts/qemu-alpine-arm64.sh. Also keep in mind
> that this job is run in the "test" step where we have far fewer jobs at
> the moment and the runners are not busy. (It would be different in the
> "build" step where we have many jobs.)

I don't really see any difference between a "test" job and a "build"
jobs, both kind use the same resource/runner.

On that note, they're is something I've learned recently: "test" job
don't even have to wait for all "build" job to complete, they can run in
parallel. We would just need to replace "dependencies" by "needs":
    https://docs.gitlab.com/ee/ci/yaml/index.html#needs
But that could be an improvement for an other time and only a side note
for the patch.

> I am not entirely sure what is the best solution overall, but for this
> series at this stage I would prefer to keep the same strategy used for
> the ARM tests (i.e. reuse the debian unstable build container and
> apt-get the few missing packages.) If we do change the way we do it, I
> would rather change both x86 and ARM at the same time.

I'm pretty sure the best strategy would be to do as little as possible
during a job, download as little as possible and possibly cache as much
as possible and do as much as possible ahead of time. Feel free to
change the Arm test, but I don't think it is necessary to change the Arm
test at the same time as introducing an x86 test.

Cheers,

-- 
Anthony PERARD