From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6927266035414335488 X-Received: by 2002:a05:6402:1b01:: with SMTP id by1mr8239858edb.61.1613385113100; Mon, 15 Feb 2021 02:31:53 -0800 (PST) X-BeenThere: isar-users@googlegroups.com Received: by 2002:a05:6402:520e:: with SMTP id s14ls403933edd.3.gmail; Mon, 15 Feb 2021 02:31:52 -0800 (PST) X-Google-Smtp-Source: ABdhPJxqvxca3Ur1UpvEnlbGzpdcBo+rZwmJELZSFWde+6aVLR5ud7RC5bxc2/0w+ZUAgIe+fb0G X-Received: by 2002:a05:6402:c04:: with SMTP id co4mr1276948edb.224.1613385111983; Mon, 15 Feb 2021 02:31:51 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1613385111; cv=none; d=google.com; s=arc-20160816; b=ff5X65Tmf/VeIjlWBktXXgwwWqk+mBLq5a2usYC29OVeBFpaan0wFIt1DEeNOWHAYs QeBx+EIiYTlGKqhk5MHAYStSPHBTTD2RyTKyHojeVKL/q7Znm5jvIp6U8pQM5XQTGL2e ZL+QYJ/VuMsBXSB0VkgMSoVVvCeNVuspt86GfnxrPGwpsUR0rUNUvXXSi44Mo5oizFbU W9NRE1wPkiwxXyAtR18s8h5GqFp037FgaPtjKIAwbILXruw3QO7Sa/HYnoE21Zc+EHjj uULyQMZ4gVP2uGS6kYZqYzpmrPrglJjO5UrjMFu5uuD0xYAquqFa5Chz6f55BChFEiEq 7RjA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:subject; bh=y8KfSOlxW+tsqTanTUmhUEqzjXQFQfcxGhE8ypR+nJI=; b=ewFcmTvrDNOEx5wFv+IRxMiAXPw89uzaSJYU11XzulR5eqNpNXNUIVUBuw6237q2hL CDYHQ9YJRbp+qngYvZrHfZsgdOyWJUBZwykorXw3VlX0eLROqjMCezGw3dNYjdIayFww p2A4gfImZwsmFhbv5lrrQb5cWIlNtrmkfjTd8iFaxG5oKT9HRFzhoXwJgmgox+xvTpjs RB3GXfNyDq1eVKCfg/9sx2mVtNy01XDQ41R+8ZpuI9npAYSauQFACLkwWQPtV++xeaVX 1JHkcRgs5ArcxsitIPCHNe6ZN+jYljDfwB52Jc8pdmUUrB+T9C1RRE0+e3lOuf2oJ3BX stwA== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: domain of jan.kiszka@siemens.com designates 194.138.37.39 as permitted sender) smtp.mailfrom=jan.kiszka@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Return-Path: Received: from lizzard.sbs.de (lizzard.sbs.de. [194.138.37.39]) by gmr-mx.google.com with ESMTPS id z69si494083ede.1.2021.02.15.02.31.51 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 15 Feb 2021 02:31:51 -0800 (PST) Received-SPF: pass (google.com: domain of jan.kiszka@siemens.com designates 194.138.37.39 as permitted sender) client-ip=194.138.37.39; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of jan.kiszka@siemens.com designates 194.138.37.39 as permitted sender) smtp.mailfrom=jan.kiszka@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Received: from mail2.sbs.de (mail2.sbs.de [192.129.41.66]) by lizzard.sbs.de (8.15.2/8.15.2) with ESMTPS id 11FAVp0d006844 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Mon, 15 Feb 2021 11:31:51 +0100 Received: from [139.22.41.241] ([139.22.41.241]) by mail2.sbs.de (8.15.2/8.15.2) with ESMTP id 11FAVppu013401; Mon, 15 Feb 2021 11:31:51 +0100 Subject: Re: [PATCH v3 1/2] images: add support for container images To: "[ext] Silvano Cirujano Cuesta" , isar-users@googlegroups.com References: <20210212085113.11013-1-silvano.cirujano-cuesta@siemens.com> <20210212085113.11013-2-silvano.cirujano-cuesta@siemens.com> <4f5debdf-b83f-9032-be42-00d85d83a22e@siemens.com> <246640e2-1c02-77e9-888e-3194630cea1c@siemens.com> From: Jan Kiszka Message-ID: <0332682f-48f6-01f7-132e-ef719299a06a@siemens.com> Date: Mon, 15 Feb 2021 11:31:51 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <246640e2-1c02-77e9-888e-3194630cea1c@siemens.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-TUID: vf9oBQPq8XXN On 15.02.21 10:46, [ext] Silvano Cirujano Cuesta wrote: > Wouldn't it make sense to put the "containerize_rootfs" function in a separate class and let "image.bbclass" inherit from it? > Sounds reasonable to me, even if we end up with only one function in that class, at least so far. Jan > The current structure that I have come up to seems weird to me, it isn't obvious that "containerize_rootfs" is meant to be reused. > > (-) an additional class for a single function > > (+) better structured code > > Possibilities that seem to fit somehow: > - specific class "container.bbclass", > - specific class "image-container-extension.bbclass" > - existing class already being inherited by "image.bbclass" ("rootfs.bbclass" -is it a rootfs feature?-, "image-postproc-extension.bbclass", "image-tools-extension.bbclass") > and I cannot tell which one fits best. > > Silvano > > On 12/02/2021 19:23, [ext] Silvano Cirujano Cuesta wrote: >> On 12/02/2021 19:06, Jan Kiszka wrote: >>> On 12.02.21 18:46, Silvano Cirujano Cuesta wrote: >>>> 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 :-) >>> Rather /wrt logically separatable steps. >>> >>>>>> Signed-off-by: Silvano Cirujano Cuesta >>>>>> --- >>>>>> 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. >>>> >>> I cannot follow: What is the difference between >>> CONTAINER_FORMATS="docker-daemon" and SDK_FORMATS="docker-daemon" when >>> running inside a kas build container? Both do not work, do they? >> I misunderstood what you meant. >> >> But I got it now, and that's what I meant with the messed up case section. >> >> In the next version the "Am I a container?" is in the function, no need to do it twice. >> >>>>>> + *) >>>>>> + 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. >>>> >>> Looks like cosmetics, not functional issues. >>> >>> But if you dislike the leading whitespaces in the debug logs, make it >>> trailing (prepend rather than append). >>> >>>>>> + 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. >>>> >>> OK, seems we are again on the same page. >>> >>>>>> + ;; >>>>>> + *) >>>>>> + 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 >>>> >>> Jan >> Silvano >> > -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux