| Message ID | 20210728152210.7089-4-henning.schild@siemens.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Allow better control over container tags | expand |
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 > } > >
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. Let us see if anyone else speaks up, in which case we could see about adding a way to stick to the old default. 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. 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 > > } > > > >
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 >>> } >>> >>> >
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 }
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 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(-)