diff mbox series

[07/12] CI: Introduce a debian:12-ppc64le container

Message ID 20240711111517.3064810-8-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series CI: part 3 (slimline, and PPC/RISCV fixes) | expand

Commit Message

Andrew Cooper July 11, 2024, 11:15 a.m. UTC 
Make it non-root by default, and set XEN_TARGET_ARCH=ppc64 too.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Juergen Gross <jgross@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>
---
 automation/build/debian/12-ppc64le.dockerfile | 36 +++++++++++++++++++
 automation/scripts/containerize               |  1 +
 2 files changed, 37 insertions(+)
 create mode 100644 automation/build/debian/12-ppc64le.dockerfile

Comments

Anthony PERARD July 11, 2024, 2:41 p.m. UTC | #1 
On Thu, Jul 11, 2024 at 12:15:12PM +0100, Andrew Cooper wrote:
> diff --git a/automation/build/debian/12-ppc64le.dockerfile b/automation/build/debian/12-ppc64le.dockerfile
> new file mode 100644
> index 000000000000..3b311e675ef5
> --- /dev/null
> +++ b/automation/build/debian/12-ppc64le.dockerfile
> @@ -0,0 +1,36 @@
...
> +ENV CROSS_COMPILE=powerpc64le-linux-gnu-
> +ENV XEN_TARGET_ARCH=ppc64

I don't really like these two envvars. The second one is already set in
the env by gitlab, the first one could in theory also come from
gitlab-ci.

But I guess that comment comes from the fact that I don't really know
why we would want a second bookworm containers with only a few packages.

> +
> +RUN <<EOF
> +#!/bin/bash
> +    set -e
> +
> +    useradd --create-home user
> +
> +    apt-get -y update
> +
> +    DEPS=(
> +        # Xen
> +        bison
> +        build-essential
> +        checkpolicy
> +        flex
> +        gcc-powerpc64le-linux-gnu
> +        python3-minimal
> +
> +        # Qemu for test phase
> +        qemu-system-ppc
> +    )
> +
> +    apt-get -y --no-install-recommends install "${DEPS[@]}"
> +    rm -rf /var/lib/apt/lists/*

Other containers do also `apt-get clean` is this not needed? Or maybe
the original containers is setup to not save packages?

/me looking into /var/cache

Looks empty enough, so I guess `apt-get clean` is optional.

Changes looks good enough, so:
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Cheers,
Andrew Cooper July 11, 2024, 3:50 p.m. UTC | #2 
On 11/07/2024 3:41 pm, Anthony PERARD wrote:
> On Thu, Jul 11, 2024 at 12:15:12PM +0100, Andrew Cooper wrote:
>> diff --git a/automation/build/debian/12-ppc64le.dockerfile b/automation/build/debian/12-ppc64le.dockerfile
>> new file mode 100644
>> index 000000000000..3b311e675ef5
>> --- /dev/null
>> +++ b/automation/build/debian/12-ppc64le.dockerfile
>> @@ -0,0 +1,36 @@
> ...
>> +ENV CROSS_COMPILE=powerpc64le-linux-gnu-
>> +ENV XEN_TARGET_ARCH=ppc64
> I don't really like these two envvars. The second one is already set in
> the env by gitlab, the first one could in theory also come from
> gitlab-ci.

The first one is pre-existing.  (It shows as new because enough of the
file changed for it not to be deemed a rename).

The second is something I added, for improved usability.

> But I guess that comment comes from the fact that I don't really know
> why we would want a second bookworm containers with only a few packages.

Right now (for better or worse) - we have single purpose containers for
a single arch.

For PPC and RISCV, where we're only doing a cross-build of the
hypervisor, the dependencies are almost identical.  But the moment we
start doing userspace, it will diverge quickly.

Part of the work I'm doing is trimming the containers down.  Container
size is a relevant factor for how long the CI tests take.  Also, with
smaller containers, the "oops, out of disk space, delete everything"
logic on various runners will trigger less frequently, so we get better
cache utilisation.

We do not IMO want to have containers with a union of all deps of all
architectures.  They'd be giant.

So while we do have single-purpose containers, these ENV vars are
~mandatory for using the container for it's intended purpose, and it's
far more usable for a human when you can just enter the container and
type `make`.
>> +
>> +RUN <<EOF
>> +#!/bin/bash
>> +    set -e
>> +
>> +    useradd --create-home user
>> +
>> +    apt-get -y update
>> +
>> +    DEPS=(
>> +        # Xen
>> +        bison
>> +        build-essential
>> +        checkpolicy
>> +        flex
>> +        gcc-powerpc64le-linux-gnu
>> +        python3-minimal
>> +
>> +        # Qemu for test phase
>> +        qemu-system-ppc
>> +    )
>> +
>> +    apt-get -y --no-install-recommends install "${DEPS[@]}"
>> +    rm -rf /var/lib/apt/lists/*
> Other containers do also `apt-get clean` is this not needed? Or maybe
> the original containers is setup to not save packages?
>
> /me looking into /var/cache
>
> Looks empty enough, so I guess `apt-get clean` is optional.

https://docs.docker.com/build/building/best-practices/ notes that the
official Debian and Ubuntu images automatically clean themselves, so you
don't need to do it manually.

https://github.com/moby/moby/blob/03e2923e42446dbb830c654d0eec323a0b4ef02a/contrib/mkimage/debootstrap#L82-L105

> Changes looks good enough, so:
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks.

~Andrew
diff mbox series

Patch

diff --git a/automation/build/debian/12-ppc64le.dockerfile b/automation/build/debian/12-ppc64le.dockerfile
new file mode 100644
index 000000000000..3b311e675ef5
--- /dev/null
+++ b/automation/build/debian/12-ppc64le.dockerfile
@@ -0,0 +1,36 @@ 
+# syntax=docker/dockerfile:1
+FROM --platform=linux/amd64 debian:bookworm-slim
+LABEL maintainer.name="The Xen Project"
+LABEL maintainer.email="xen-devel@lists.xenproject.org"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV CROSS_COMPILE=powerpc64le-linux-gnu-
+ENV XEN_TARGET_ARCH=ppc64
+
+RUN <<EOF
+#!/bin/bash
+    set -e
+
+    useradd --create-home user
+
+    apt-get -y update
+
+    DEPS=(
+        # Xen
+        bison
+        build-essential
+        checkpolicy
+        flex
+        gcc-powerpc64le-linux-gnu
+        python3-minimal
+
+        # Qemu for test phase
+        qemu-system-ppc
+    )
+
+    apt-get -y --no-install-recommends install "${DEPS[@]}"
+    rm -rf /var/lib/apt/lists/*
+EOF
+
+USER user
+WORKDIR /build
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index e9ad20aa0c80..aa3433f0dc98 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -32,6 +32,7 @@  case "_${CONTAINER}" in
     _fedora) CONTAINER="${BASE}/fedora:29";;
     _focal) CONTAINER="${BASE}/ubuntu:focal" ;;
     _bullseye-ppc64le) CONTAINER="${BASE}/debian:bullseye-ppc64le" ;;
+    _bookworm-ppc64le) CONTAINER="${BASE}/debian:12-ppc64le" ;;
     _buster-gcc-ibt) CONTAINER="${BASE}/debian:buster-gcc-ibt" ;;
     _bookworm|_) CONTAINER="${BASE}/debian:bookworm" ;;
     _bookworm-i386) CONTAINER="${BASE}/debian:bookworm-i386" ;;