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

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

On 29/07/2021 15:05, Henning Schild wrote:
> Am Thu, 29 Jul 2021 14:50:12 +0200
> schrieb 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.
>  
> I was expecting you to know users that might need to be warned about
> the default name:tag change. But seems you are OK with it.

I don't know any users, I'm OK with it.

> 
> Let us see if anyone else speaks up, in which case we could see about
> adding a way to stick to the old default.

I agree with you that if there are no known users of this feature, we can afford this breaking change.

> 
> Any way i will add an entry to the API-CHANGELOG to point out the
> break. Maybe in v2 or just on top in case this v1 goes through as is.

Keeping the breaking change and adding the API-CHANGELOG entry would be in any case the lowest effort possible.

Silvano

> 
> Henning
> 
>>>
>>> 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