Re: [PATCH 3/4] classes: allow more control over container image name and tag

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

On 28/07/2021 17:22, Henning Schild wrote:
> This patch allows more fine-grained control over how the resulting
> container will be tagged. Where the default name will be PN together
> with DISTRO and ARCH, and tag will be derived from PV and PR

This is a completely meaningful change. My implementation didn't foresee the need for free selection of container name and tag.

The default container name and tag that is patch is providing is also meaningful, but it's breaking the interface (resulting container image has a fixed, predefined name).
I don't have the impression that there are many users of this feature. Therefore we might afford it.
But I don't follow ISAR development close enough to be able to assess what's the impact of this change on the user community.

The alternative is breaking this transition in two steps:
1. Make the container image name and tag configurable, but keep the default backwards compatible (even if yours is more meaningful). And announce the upcoming breaking change.
2. Change the default effectively breaking the interface.

> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  meta/classes/container-img.bbclass             |  4 +---
>  meta/classes/image-container-extension.bbclass | 11 ++++++-----
>  meta/classes/image-sdk-extension.bbclass       |  2 +-
>  3 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/meta/classes/container-img.bbclass b/meta/classes/container-img.bbclass
> index 79ef3e8d756b..9928a58ef53d 100644
> --- a/meta/classes/container-img.bbclass
> +++ b/meta/classes/container-img.bbclass
> @@ -9,10 +9,8 @@
>  do_container_image[stamp-extra-info] = "${DISTRO}-${MACHINE}"
>  do_container_image[vardeps] += "CONTAINER_FORMATS"
>  do_container_image(){
> -    rootfs_id="${DISTRO}-${DISTRO_ARCH}"
> -
>      bbdebug 1 "Generate container image in these formats: ${CONTAINER_FORMATS}"
> -    containerize_rootfs "${IMAGE_ROOTFS}" "${rootfs_id}" "${CONTAINER_FORMATS}"
> +    containerize_rootfs "${IMAGE_ROOTFS}" "${CONTAINER_FORMATS}"
>  }
>  
>  addtask container_image before do_image after do_image_tools
> diff --git a/meta/classes/image-container-extension.bbclass b/meta/classes/image-container-extension.bbclass
> index 0e70ba9c1405..43b67f9d864d 100644
> --- a/meta/classes/image-container-extension.bbclass
> +++ b/meta/classes/image-container-extension.bbclass
> @@ -6,15 +6,16 @@
>  # This class extends the image.bbclass for containerizing the root filesystem.
>  
>  CONTAINER_FORMATS ?= "docker-archive"
> +CONTAINER_IMAGE_NAME ?= "${PN}-${DISTRO}-${DISTRO_ARCH}"
> +CONTAINER_TAG ?= "${PV}-${PR}"
>  
>  containerize_rootfs() {
>      local cmd="/bin/dash"
>      local empty_tag="empty"
> -    local tag="latest"
> +    local tag="${CONTAINER_TAG}"
>      local oci_img_dir="${WORKDIR}/oci-image"
>      local rootfs="$1"
> -    local rootfs_id="$2"
> -    local container_formats="$3"
> +    local container_formats="$2"
>  
>      # prepare OCI container image skeleton
>      bbdebug 1 "prepare OCI container image skeleton"
> @@ -42,9 +43,9 @@ containerize_rootfs() {
>      sudo chown --recursive $(id -u):$(id -g) "${oci_img_dir}"
>  
>      # convert the OCI container image to the desired format
> -    image_name="isar-${rootfs_id}"
> +    image_name="${CONTAINER_IMAGE_NAME}"
>      for image_type in ${CONTAINER_FORMATS} ; do
> -        image_archive="${DEPLOY_DIR_IMAGE}/${rootfs_id}-${image_type}.tar"
> +        image_archive="${DEPLOY_DIR_IMAGE}/${image_name}-${tag}-${image_type}.tar"
>          bbdebug 1 "Creating container image type: ${image_type}"
>          case "${image_type}" in
>              "docker-archive" | "oci-archive")
> diff --git a/meta/classes/image-sdk-extension.bbclass b/meta/classes/image-sdk-extension.bbclass
> index 426b92595554..fa15b588068c 100644
> --- a/meta/classes/image-sdk-extension.bbclass
> +++ b/meta/classes/image-sdk-extension.bbclass
> @@ -80,7 +80,7 @@ do_populate_sdk() {
>      # 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}"
> +        containerize_rootfs "${SDKCHROOT_DIR}" "${sdk_container_formats}"
>      fi
>  }
>  
> 
-- 
  Silvano Cirujano Cuesta
-- 
Siemens AG, T RDA IOT SES-DE
Corporate Competence Center Embedded Linux