Re: [PATCH v3 1/2] images: add support for container images

[Silvano Cirujano Cuesta <silvano.cirujano-cuesta@siemens.com>]

On 12/02/2021 18:10, Jan Kiszka wrote:
> On 12.02.21 09:51, [ext] Silvano Cirujano Cuesta wrote:
>> Add support for creation of container images with the build root
>> filesystems.
>>
>> Extend also task "populate_sdk" to support the creation of a container image
>> containing the SDK.
> Should be done in to steps: container-img.bbclass frirst, and then a
> patch to use it for the SDK as well.

Ok. There are some many different tastes WRT to big vs small commits :-)

>
>> Signed-off-by: Silvano Cirujano Cuesta <silvano.cirujano-cuesta@siemens.com>
>> ---
>>  meta/classes/container-img.bbclass       | 88 ++++++++++++++++++++++++
>>  meta/classes/image-sdk-extension.bbclass | 51 ++++++++++++--
>>  meta/classes/image.bbclass               |  1 +
>>  3 files changed, 133 insertions(+), 7 deletions(-)
>>  create mode 100644 meta/classes/container-img.bbclass
>>
>> diff --git a/meta/classes/container-img.bbclass b/meta/classes/container-img.bbclass
>> new file mode 100644
>> index 0000000..35c7bbc
>> --- /dev/null
>> +++ b/meta/classes/container-img.bbclass
>> @@ -0,0 +1,88 @@
>> +# This software is a part of ISAR.
>> +# Copyright (C) Siemens AG, 2021
>> +#
>> +# SPDX-License-Identifier: MIT
>> +#
>> +# This class provides the tasks 'containerize_rootfs' and 'containerize_sdk'
> Nope, it now only provides the former.
Yes, you're right, will fix it.
>
>> +# to create container images containing the target rootfs and the SDK
>> +# respectively.
>> +
>> +CONTAINER_FORMATS ?= "docker-archive"
>> +
>> +containerize_rootfs() {
>> +    local cmd="/bin/dash"
>> +    local empty_tag="empty"
>> +    local full_tag="latest"
>> +    local oci_img_dir="${WORKDIR}/oci-image"
>> +    local rootfs="$1"
>> +    local rootfs_id="$2"
>> +    local container_formats="$3"
>> +
>> +    # prepare OCI container image skeleton
>> +    bbdebug 1 "prepare OCI container image skeleton"
>> +    rm -rf "${oci_img_dir}"
>> +    sudo umoci init --layout "${oci_img_dir}"
>> +    sudo umoci new --image "${oci_img_dir}:${empty_tag}"
>> +    sudo umoci config --image "${oci_img_dir}:${empty_tag}" \
>> +        --config.cmd="${cmd}"
>> +    sudo umoci unpack --image "${oci_img_dir}:${empty_tag}" \
>> +        "${oci_img_dir}_unpacked"
>> +
>> +    # add root filesystem as the flesh of the skeleton
>> +    sudo cp -a "${rootfs}"/* "${oci_img_dir}_unpacked/rootfs/"
>> +
>> +    # pack container image
>> +    bbdebug 1 "pack container image"
>> +    sudo umoci repack --image "${oci_img_dir}:${full_tag}" \
>> +        "${oci_img_dir}_unpacked"
>> +    sudo umoci remove --image "${oci_img_dir}:${empty_tag}"
>> +    sudo rm -rf "${oci_img_dir}_unpacked"
>> +
>> +    # no root needed anymore
>> +    sudo chown --recursive $(id -u):$(id -g) "${oci_img_dir}"
>> +
>> +    # convert the OCI container image to the desired format
>> +    image_name="isar-${rootfs_id}"
>> +    for image_type in ${CONTAINER_FORMATS} ; do
>> +        image_archive="${DEPLOY_DIR_IMAGE}/${rootfs_id}-${image_type}.tar"
>> +        bbdebug 1 "Creating container image type: ${image_type}"
>> +        case "${image_type}" in
>> +            "docker-archive" | "oci-archive")
>> +                if [ "${image_type}" = "oci-archive" ] ; then
>> +                    target="${image_type}:${image_archive}:latest"
>> +                else
>> +                    target="${image_type}:${image_archive}:${image_name}:latest"
>> +                fi
>> +                rm -f "${image_archive}" "${image_archive}.xz"
>> +                bbdebug 2 "Converting OCI image to ${image_type}"
>> +                skopeo --insecure-policy copy \
>> +                    "oci:${oci_img_dir}:${full_tag}" "${target}"
>> +                bbdebug 2 "Compressing image"
>> +                xz -T0 "${image_archive}"
>> +                ;;
>> +            "oci")
>> +                tar --create --xz --directory "${oci_img_dir}" \
>> +                    --file "${image_archive}.xz" .
>> +                ;;
>> +            "docker-daemon" | "containers-storage")
>> +                skopeo --insecure-policy copy \
>> +                    "oci:${oci_img_dir}:${full_tag}" \
>> +                    "${image_type}:${image_name}:latest"
>> +                ;;
> Missing check for "Am I in a container?", like in the SDK. Maybe move
> that test here and share.

Not needed, since the usage of IMAGE_TYPE is fixing already to container type.

In the case of the SDK the same task is provides the non-containerized format tar-xz.

>
>> +            *)
>> +                die "Unsupported format for containerize_rootfs: ${image_type}"
>> +                ;;
>> +        esac
>> +    done
>> +}
>> +
>> +do_container_image[stamp-extra-info] = "${DISTRO}-${MACHINE}"
>> +do_container_image[vardeps] += "CONTAINER_FORMATS"
>> +do_container_image(){
>> +    rootfs_id="${DISTRO}-${DISTRO_ARCH}"
>> +
>> +    bbnote "Generate container image in these formats: ${CONTAINER_FORMATS}"
> Probabably more "bbdebug"? Unsure. But we aren't using bbnote in the
> core so far. Nor bbdebug, though.

At least bbdebug is IMO needed for debbuging if goes wrong.

BTW I'm using bbdebug a lot in the containerize_rootfs section because I've missed those kind of traces much too often when trying to debug some issues on ISAR recipes.

Perhaps we should have more debug verbosity in the logs to ease debugging...

>
>> +    containerize_rootfs "${IMAGE_ROOTFS}" "${rootfs_id}" "${CONTAINER_FORMATS}"
>> +}
>> +
>> +addtask container_image before do_image after do_image_tools
>> diff --git a/meta/classes/image-sdk-extension.bbclass b/meta/classes/image-sdk-extension.bbclass
>> index a8c708a..63138da 100644
>> --- a/meta/classes/image-sdk-extension.bbclass
>> +++ b/meta/classes/image-sdk-extension.bbclass
>> @@ -6,11 +6,25 @@
>>  # This class extends the image.bbclass to supply the creation of a sdk
>>  
>>  SDK_INCLUDE_ISAR_APT ?= "0"
>> +SDK_FORMATS ?= "tar-xz"
>> +
>> +sdk_tar_xz() {
>> +    # Copy mount_chroot.sh for convenience
>> +    sudo cp ${SCRIPTSDIR}/mount_chroot.sh ${SDKCHROOT_DIR}
>> +
>> +    # Create SDK archive
>> +    cd -P ${SDKCHROOT_DIR}/..
>> +    sudo tar --transform="s|^rootfs|sdk-${DISTRO}-${DISTRO_ARCH}|" \
>> +        -c rootfs | xz -T0 > ${DEPLOY_DIR_IMAGE}/sdk-${DISTRO}-${DISTRO_ARCH}.tar.xz
>> +    bbnote "SDK rootfs available in ${DEPLOY_DIR_IMAGE}/sdk-${DISTRO}-${DISTRO_ARCH}.tar.xz"
>> +}
>>  
>>  do_populate_sdk[stamp-extra-info] = "${DISTRO}-${MACHINE}"
>>  do_populate_sdk[depends] = "sdkchroot:do_build"
>> -do_populate_sdk[vardeps] += "SDK_INCLUDE_ISAR_APT"
>> +do_populate_sdk[vardeps] += "SDK_INCLUDE_ISAR_APT SDK_FORMATS"
>>  do_populate_sdk() {
>> +    local sdk_container_formats=""
>> +
>>      if [ "${SDK_INCLUDE_ISAR_APT}" = "1" ]; then
>>          # Copy isar-apt with deployed Isar packages
>>          sudo cp -Trpfx ${REPO_ISAR_DIR}/${DISTRO} ${SDKCHROOT_DIR}/isar-apt
>> @@ -48,12 +62,35 @@ do_populate_sdk() {
>>          done
>>      done
>>  
>> -    # Copy mount_chroot.sh for convenience
>> -    sudo cp ${SCRIPTSDIR}/mount_chroot.sh ${SDKCHROOT_DIR}
>> +    # separate SDK formats: TAR and container formats
>> +    for sdk_format in ${SDK_FORMATS} ; do
>> +        case ${sdk_format} in
>> +            "tar-xz")
>> +                sdk_tar_xz
>> +                ;;
>> +            "docker-archive" | "oci" | "oci-archive")
>> +                if [ -z "${sdk_container_formats}" ] ; then
> Unneeded, just use the else part unconditionally.

The else part alone adds a heading whitespace. It's being ignored in containerize_rootfs, but it's still messing up some outputs.

Not really useless, but not important (in fact that was my 1st version). I can change it in the next patch series version that I need anyway.

>
>> +                    sdk_container_formats="${sdk_format}"
>> +                else
>> +                    sdk_container_formats="${sdk_container_formats} ${sdk_format}"
>> +                fi
>> +                ;;
>> +            "docker-daemon" | "containers-storage")
>> +                if [ -f /.dockerenv ] || [ -f /run/.containerenv ] ; then
>> +                    die "Adding the SDK container image to a container runtime (${sdk_format}) not supported if running from a container (e.g. 'kas-container')"
>> +                fi
> See above, should likely go into containerize_rootfs().

Right, will fix it.

In fact this case section is really messed up, I have to clean it up completely.

>
>> +                ;;
>> +            *)
>> +                die "unsupported SDK format specified: ${sdk_format}"
>> +                ;;
>> +        esac
>> +    done
>>  
>> -    # Create SDK archive
>> -    cd -P ${SDKCHROOT_DIR}/..
>> -    sudo tar --transform="s|^rootfs|sdk-${DISTRO}-${DISTRO_ARCH}|" \
>> -        -c rootfs | xz -T0 > ${DEPLOY_DIR_IMAGE}/sdk-${DISTRO}-${DISTRO_ARCH}.tar.xz
>> +    # generate the SDK in all the desired container formats
>> +    if [ -n "${sdk_container_formats}" ] ; then
>> +        bbnote "Generating SDK container in ${sdk_container_formats} format"
>> +        containerize_rootfs "${SDKCHROOT_DIR}" "sdk-${DISTRO}-${DISTRO_ARCH}" "${sdk_container_formats}"
>> +    fi
>>  }
>> +
>>  addtask populate_sdk after do_rootfs
>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>> index eddc444..7fb7b7e 100644
>> --- a/meta/classes/image.bbclass
>> +++ b/meta/classes/image.bbclass
>> @@ -76,6 +76,7 @@ inherit image-tools-extension
>>  inherit image-postproc-extension
>>  inherit image-locales-extension
>>  inherit image-account-extension
>> +inherit container-img
>>  
>>  # Extra space for rootfs in MB
>>  ROOTFS_EXTRA ?= "64"
>>
> Jan
Silvano

-- 
Siemens AG, T RDA IOT SES-DE
Corporate Competence Center Embedded Linux