[PATCH] support tests against Fedora 35 Cloud Base Image

[Coiby Xu]

Fedora 35 Cloud Base Image has multiple partitions and existing tests
couldn't run due to the following two changes,
    - the last partition is the root partition and the files now resides
      in a folder named root
    - the first partition is the boot partition

This patch address the above changes by mounting {LAST_PARTITION}/root as
to TEMP_ROOT and mount FIRST_PARTITION to TEMP_ROOT/boot. So the test
image can be built successfully.

Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
---
 tests/scripts/image-init-lib.sh | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
index 0a1524b..c1c1c4e 100644
--- a/tests/scripts/image-init-lib.sh
+++ b/tests/scripts/image-init-lib.sh
@@ -23,6 +23,7 @@ is_mounted()
 clean_up()
 {
 	for _mnt in ${MNTS[@]}; do
+		is_mounted $_mnt/root/boot && $SUDO umount -f $_mnt/root/boot
 		is_mounted $_mnt && $SUDO umount -f $_mnt
 	done
 
@@ -81,6 +82,21 @@ get_mountable_dev() {
 	fi
 }
 
+# get the separate boot partition
+# return the first partition as boot partition
+get_mount_boot() {
+        local dev=$1 parts
+
+        $SUDO partprobe $dev && sync
+        parts="$(ls -1 ${dev}p*)"
+        if [ -n "$parts" ]; then
+                if [ $(echo "$parts" | wc -l) -gt 1 ]; then
+                    echo "$parts" | head -1
+                fi
+        fi
+}
+
+
 prepare_loop() {
 	[ -n "$(lsmod | grep "^loop")" ] && return
 
@@ -133,7 +149,7 @@ image_lock()
 # Mount a device, will umount it automatially when shell exits
 mount_image() {
 	local image=$1 fmt
-	local dev mnt mnt_dev
+	local dev mnt mnt_dev boot root
 
 	# Lock the image just in case user run this script in parrel
 	image_lock $image
@@ -166,12 +182,19 @@ mount_image() {
 
 	$SUDO mount $mnt_dev $mnt
 	[ $? -ne 0 ] && perror_exit "failed to mount device
'$mnt_dev'"
+	boot=$(get_mount_boot "$dev")
+	if [[ -n "$boot" ]]; then
+		root=$(get_image_mount_root $image)
+		$SUDO mount $boot $root/boot
+		[ $? -ne 0 ] && perror_exit "failed to mount the bootable partition for
device '$mnt_dev'"
+	fi
 }
 
 get_image_mount_root() {
 	local image=$1
 	local root=${MNTS[$image]}
 
+	[ -d "$root/root" ] && root=$root/root
 	echo $root
 
 	if [ -z "$root" ]; then
@@ -213,7 +236,7 @@ run_in_image() {
 
 inst_in_image() {
 	local image=$1 src=$2 dst=$3
-	local root=${MNTS[$image]}
+	local root=$(get_image_mount_root $1)
 
 	$SUDO cp $src $root/$dst
 }
-- 
2.31.1

[Kairui Song]

On Thu, Oct 14, 2021 at 3:52 PM Coiby Xu <coxu(a)redhat.com&gt; wrote:
>  Fedora 35 Cloud Base Image has multiple partitions and existing tests
>  couldn't run due to the following two changes,
>      - the last partition is the root partition and the files now resides
>        in a folder named root
>      - the first partition is the boot partition
>
>  This patch address the above changes by mounting {LAST_PARTITION}/root as
>  to TEMP_ROOT and mount FIRST_PARTITION to TEMP_ROOT/boot. So the test
>  image can be built successfully.
>
>  Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
>  ---
>   tests/scripts/image-init-lib.sh | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
>
>  diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
>  index 0a1524b..c1c1c4e 100644
>  --- a/tests/scripts/image-init-lib.sh
>  +++ b/tests/scripts/image-init-lib.sh
>  @@ -23,6 +23,7 @@ is_mounted()
>   clean_up()
>   {
>          for _mnt in ${MNTS[@]}; do
>  +               is_mounted $_mnt/root/boot && $SUDO umount -f $_mnt/root/boot
>                  is_mounted $_mnt && $SUDO umount -f $_mnt
>          done
>
>  @@ -81,6 +82,21 @@ get_mountable_dev() {
>          fi
>   }
>
>  +# get the separate boot partition
>  +# return the first partition as boot partition
>  +get_mount_boot() {
>  +        local dev=$1 parts
>  +
>  +        $SUDO partprobe $dev && sync
>  +        parts="$(ls -1 ${dev}p*)"
>  +        if [ -n "$parts" ]; then
>  +                if [ $(echo "$parts" | wc -l) -gt 1 ]; then
>  +                    echo "$parts" | head -1
>  +                fi
>  +        fi
>  +}
>  +
>  +
>   prepare_loop() {
>          [ -n "$(lsmod | grep "^loop")" ] && return
>
>  @@ -133,7 +149,7 @@ image_lock()
>   # Mount a device, will umount it automatially when shell exits
>   mount_image() {
>          local image=$1 fmt
>  -       local dev mnt mnt_dev
>  +       local dev mnt mnt_dev boot root
>
>          # Lock the image just in case user run this script in parrel
>          image_lock $image
>  @@ -166,12 +182,19 @@ mount_image() {
>
>          $SUDO mount $mnt_dev $mnt
>          [ $? -ne 0 ] && perror_exit "failed to mount device
> '$mnt_dev'"
>  +       boot=$(get_mount_boot "$dev")
>  +       if [[ -n "$boot" ]]; then
>  +               root=$(get_image_mount_root $image)
>  +               $SUDO mount $boot $root/boot
>  +               [ $? -ne 0 ] && perror_exit "failed to mount the bootable
> partition for device '$mnt_dev'"
>  +       fi
>   }
>
>   get_image_mount_root() {
>          local image=$1
>          local root=${MNTS[$image]}
>
>  +       [ -d "$root/root" ] && root=$root/root 
It seems this line is hackish and broken, at least it breaks the
exiting test on F34 images. $root/root always exists, which is for the
root user's home folder.

I think here we can do a more proper detection, like detect
/etc/fedora-release file location, this file should always present on
Fedora images, and maybe a bit more sanity check with /usr/bin/sh file
which is also always mandatory.

>          echo $root
>
>          if [ -z "$root" ]; then
>  @@ -213,7 +236,7 @@ run_in_image() {
>
>   inst_in_image() {
>          local image=$1 src=$2 dst=$3
>  -       local root=${MNTS[$image]}
>  +       local root=$(get_image_mount_root $1)
>
>          $SUDO cp $src $root/$dst
>   }
>  --
>  2.31.1
>  _______________________________________________
>  kexec mailing list -- kexec(a)lists.fedoraproject.org
>  To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]
>  Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure


-- 
Best Regards,
Kairui Song

[Coiby Xu]

On Fri, Oct 15, 2021 at 07:41:45PM +0800, Kairui Song wrote:
> On Thu, Oct 14, 2021 at 3:52 PM Coiby Xu <coxu(a)redhat.com&gt;
> wrote:
> >
> > Fedora 35 Cloud Base Image has multiple partitions and existing tests
> > couldn't run due to the following two changes,
> >     - the last partition is the root partition and the files now resides
> >       in a folder named root
> >     - the first partition is the boot partition
> >
> > This patch address the above changes by mounting {LAST_PARTITION}/root as
> > to TEMP_ROOT and mount FIRST_PARTITION to TEMP_ROOT/boot. So the test
> > image can be built successfully.
> >
> > Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
> > ---
> >  tests/scripts/image-init-lib.sh | 27 +++++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
> > index 0a1524b..c1c1c4e 100644
> > --- a/tests/scripts/image-init-lib.sh
> > +++ b/tests/scripts/image-init-lib.sh
> > @@ -23,6 +23,7 @@ is_mounted()
> >  clean_up()
> >  {
> >         for _mnt in ${MNTS[@]}; do
> > +               is_mounted $_mnt/root/boot && $SUDO umount -f
> $_mnt/root/boot
> >                 is_mounted $_mnt && $SUDO umount -f $_mnt
> >         done
> >
> > @@ -81,6 +82,21 @@ get_mountable_dev() {
> >         fi
> >  }
> >
> > +# get the separate boot partition
> > +# return the first partition as boot partition
> > +get_mount_boot() {
> > +        local dev=$1 parts
> > +
> > +        $SUDO partprobe $dev && sync
> > +        parts="$(ls -1 ${dev}p*)"
> > +        if [ -n "$parts" ]; then
> > +                if [ $(echo "$parts" | wc -l) -gt 1 ]; then
> > +                    echo "$parts" | head -1
> > +                fi
> > +        fi
> > +}
> > +
> > +
> >  prepare_loop() {
> >         [ -n "$(lsmod | grep "^loop")" ] && return
> >
> > @@ -133,7 +149,7 @@ image_lock()
> >  # Mount a device, will umount it automatially when shell exits
> >  mount_image() {
> >         local image=$1 fmt
> > -       local dev mnt mnt_dev
> > +       local dev mnt mnt_dev boot root
> >
> >         # Lock the image just in case user run this script in parrel
> >         image_lock $image
> > @@ -166,12 +182,19 @@ mount_image() {
> >
> >         $SUDO mount $mnt_dev $mnt
> >         [ $? -ne 0 ] && perror_exit "failed to mount device
> '$mnt_dev'"
> > +       boot=$(get_mount_boot "$dev")
> > +       if [[ -n "$boot" ]]; then
> > +               root=$(get_image_mount_root $image)
> > +               $SUDO mount $boot $root/boot
> > +               [ $? -ne 0 ] && perror_exit "failed to mount the
> bootable partition for device '$mnt_dev'"
> > +       fi
> >  }
> >
> >  get_image_mount_root() {
> >         local image=$1
> >         local root=${MNTS[$image]}
> >
> > +       [ -d "$root/root" ] && root=$root/root
>
> It seems this line is hackish and broken, at least it breaks the
> exiting test on F34 images. $root/root always exists, which is for the
> root user's home folder. 
Sorry it should be "-d $root/root/root". This is what I mean by "the last 
partition is the root partition and the files now resides in a folder 
named root", i.e. the root partition has the following file structure,
.
├── home
└── root
     ├── bin -> usr/bin
     ├── boot
     ├── dev
     ├── etc
     ├── home
  
By comparison, the file structure for 34 is as follows,
.
├── bin -> usr/bin
├── boot
├── dev
├── etc
├── home

> I think here we can do a more proper detection, like detect
> /etc/fedora-release file location, this file should always present on
> Fedora images, and maybe a bit more sanity check with /usr/bin/sh file
> which is also always mandatory. 
I will include the comparison of file structure in the commit log. Do
you think other detection is needed?

> >         echo $root
> >
> >         if [ -z "$root" ]; then
> > @@ -213,7 +236,7 @@ run_in_image() {
> >
> >  inst_in_image() {
> >         local image=$1 src=$2 dst=$3
> > -       local root=${MNTS[$image]}
> > +       local root=$(get_image_mount_root $1)
> >
> >         $SUDO cp $src $root/$dst
> >  }
> > --
> > 2.31.1
> > _______________________________________________
> > kexec mailing list -- kexec(a)lists.fedoraproject.org
> > To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
> > Fedora Code of Conduct:
> https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]
> > Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
>
>
>
> -- 
> Best Regards,
> Kairui Song
> _______________________________________________
> kexec mailing list -- kexec(a)lists.fedoraproject.org
> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]
> Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
-- 
Best regards,
Coiby

[Kairui Song]

On Mon, Oct 18, 2021 at 9:40 AM Coiby Xu <coxu(a)redhat.com&gt; wrote:
>  On Fri, Oct 15, 2021 at 07:41:45PM +0800, Kairui Song wrote:
>  >On Thu, Oct 14, 2021 at 3:52 PM Coiby Xu <coxu(a)redhat.com&gt; wrote:
>  >>
>  >> Fedora 35 Cloud Base Image has multiple partitions and existing tests
>  >> couldn't run due to the following two changes,
>  >>     - the last partition is the root partition and the files now resides
>  >>       in a folder named root
>  >>     - the first partition is the boot partition
>  >>
>  >> This patch address the above changes by mounting {LAST_PARTITION}/root as
>  >> to TEMP_ROOT and mount FIRST_PARTITION to TEMP_ROOT/boot. So the test
>  >> image can be built successfully.
>  >>
>  >> Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
>  >> ---
>  >>  tests/scripts/image-init-lib.sh | 27 +++++++++++++++++++++++++--
>  >>  1 file changed, 25 insertions(+), 2 deletions(-)
>  >>
>  >> diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
>  >> index 0a1524b..c1c1c4e 100644
>  >> --- a/tests/scripts/image-init-lib.sh
>  >> +++ b/tests/scripts/image-init-lib.sh
>  >> @@ -23,6 +23,7 @@ is_mounted()
>  >>  clean_up()
>  >>  {
>  >>         for _mnt in ${MNTS[@]}; do
>  >> +               is_mounted $_mnt/root/boot && $SUDO umount -f
> $_mnt/root/boot
>  >>                 is_mounted $_mnt && $SUDO umount -f $_mnt
>  >>         done
>  >>
>  >> @@ -81,6 +82,21 @@ get_mountable_dev() {
>  >>         fi
>  >>  }
>  >>
>  >> +# get the separate boot partition
>  >> +# return the first partition as boot partition
>  >> +get_mount_boot() {
>  >> +        local dev=$1 parts
>  >> +
>  >> +        $SUDO partprobe $dev && sync
>  >> +        parts="$(ls -1 ${dev}p*)"
>  >> +        if [ -n "$parts" ]; then
>  >> +                if [ $(echo "$parts" | wc -l) -gt 1 ]; then
>  >> +                    echo "$parts" | head -1
>  >> +                fi
>  >> +        fi
>  >> +}
>  >> +
>  >> +
>  >>  prepare_loop() {
>  >>         [ -n "$(lsmod | grep "^loop")" ] && return
>  >>
>  >> @@ -133,7 +149,7 @@ image_lock()
>  >>  # Mount a device, will umount it automatially when shell exits
>  >>  mount_image() {
>  >>         local image=$1 fmt
>  >> -       local dev mnt mnt_dev
>  >> +       local dev mnt mnt_dev boot root
>  >>
>  >>         # Lock the image just in case user run this script in parrel
>  >>         image_lock $image
>  >> @@ -166,12 +182,19 @@ mount_image() {
>  >>
>  >>         $SUDO mount $mnt_dev $mnt
>  >>         [ $? -ne 0 ] && perror_exit "failed to mount device
> '$mnt_dev'"
>  >> +       boot=$(get_mount_boot "$dev")
>  >> +       if [[ -n "$boot" ]]; then
>  >> +               root=$(get_image_mount_root $image)
>  >> +               $SUDO mount $boot $root/boot
>  >> +               [ $? -ne 0 ] && perror_exit "failed to mount the
> bootable partition for device '$mnt_dev'"
>  >> +       fi
>  >>  }
>  >>
>  >>  get_image_mount_root() {
>  >>         local image=$1
>  >>         local root=${MNTS[$image]}
>  >>
>  >> +       [ -d "$root/root" ] && root=$root/root
>  >
>  >It seems this line is hackish and broken, at least it breaks the
>  >exiting test on F34 images. $root/root always exists, which is for the
>  >root user's home folder.
>
>  Sorry it should be "-d $root/root/root". This is what I mean by "the last
>  partition is the root partition and the files now resides in a folder
>  named root", i.e. the root partition has the following file structure,
>  .
>  ├── home
>  └── root
>       ├── bin -> usr/bin
>       ├── boot
>       ├── dev
>       ├── etc
>       ├── home
>
>  By comparison, the file structure for 34 is as follows,
>  .
>  ├── bin -> usr/bin
>  ├── boot
>  ├── dev
>  ├── etc
>  ├── home
>
>  >
>  >I think here we can do a more proper detection, like detect
>  >/etc/fedora-release file location, this file should always present on
>  >Fedora images, and maybe a bit more sanity check with /usr/bin/sh file
>  >which is also always mandatory.
>
>  I will include the comparison of file structure in the commit log. Do
>  you think other detection is needed? 
I think that's good enough, I can help review the next patch.

>  >
>  >>         echo $root
>  >>
>  >>         if [ -z "$root" ]; then
>  >> @@ -213,7 +236,7 @@ run_in_image() {
>  >>
>  >>  inst_in_image() {
>  >>         local image=$1 src=$2 dst=$3
>  >> -       local root=${MNTS[$image]}
>  >> +       local root=$(get_image_mount_root $1)
>  >>
>  >>         $SUDO cp $src $root/$dst
>  >>  }
>  >> --
>  >> 2.31.1
>  >> _______________________________________________
>  >> kexec mailing list -- kexec(a)lists.fedoraproject.org
>  >> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>  >> Fedora Code of Conduct:
> https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  >> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  >> List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]
>  >> Do not reply to spam on the list, report it:
> https://pagure.io/fedora-infrastructure
>  >
>  >
>  >
>  >--
>  >Best Regards,
>  >Kairui Song
>  >_______________________________________________
>  >kexec mailing list -- kexec(a)lists.fedoraproject.org
>  >To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>  >Fedora Code of Conduct:
> https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  >List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  >List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]
>  >Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
>
>  --
>  Best regards,
>  Coiby
>  

-- 
Best Regards,
Kairui Song

[Coiby Xu]

On Mon, Oct 18, 2021 at 07:06:36PM +0800, Kairui Song wrote:
> On Mon, Oct 18, 2021 at 9:40 AM Coiby Xu <coxu(a)redhat.com&gt;
> wrote: 
[...]
> > >
> > >I think here we can do a more proper detection, like detect
> > >/etc/fedora-release file location, this file should always present on
> > >Fedora images, and maybe a bit more sanity check with /usr/bin/sh file
> > >which is also always mandatory.
> >
> > I will include the comparison of file structure in the commit log. Do
> > you think other detection is needed?
>
> I think that's good enough, I can help review the next patch. 
Thanks! I've sent v2.

-- 
Best regards,
Coiby