Fix ISO location logic in 99_migration

[doccaz]

I noticed that you have a Kiwi appliance to test DMS on your OBS home directory. I branched it and made an LVM version with 4 volumes to test DMS. I then noticed that 99_migration is not finding the ISO file on an LVM root volume and not writing the corresponding entry to GRUB.

I added a test to check if the ISO is found, and only search on the user paths if not found, This commit solves the issue.

The KIWI appliance is located here: https://build.opensuse.org/package/show/home:emendonca:branches:home:marcus.schaefer:dms/suse-migration-test-vm-lvm

The line where I substituted an "echo" by an "ls -1 | tail -1" is just for a situation I found when testing multiple versions of the packages and it ended up returning an invalid value when there's more than one ISO in the directory.

[schaefi]

@doccaz Thanks for looking into this.

With regards to #277 we moved the install location of the live ISO from /usr/share/migration-image to /migration-image. This move impacts the 99_migration code as you already found out. Because of this reason the two packages SLES15-Migration (which packages the live ISO into a package) and the package suse-migration-sle15-activation must be in sync. I updated the activation package in my :dms build later because I also stumbled over the issue you had. Therefore I assume when you branched my project you still got the not-in-sync package state. If you check it again it should be in sync and my tests worked.

I don't think we should add the complexity of two different search methods in the 99_migration code. Instead I suggest that
we bump the version of the SLES15-Migration package and add a proper Requires: SLES15-Migration >= "..." to the spec file of the suse-migration-sle15-activation package

[schaefi]

The line where I substituted an "echo" by an "ls -1 | tail -1" is just for a situation I found when testing multiple versions of the packages and it ended up returning an invalid value when there's more than one ISO in the directory.

I agree with you that the echo code is weak. However, always using the first entry found might lead to unexpected behavior too. I was thinking under which condition you got the situation that there is more than one image available and differently named ?
In any case, covering this issue should be an extra PR.

[schaefi]

Thanks for looking into this. Good catch and we need to fix this but I propose a different direction. Thoughts ?

[doccaz]

The line where I substituted an "echo" by an "ls -1 | tail -1" is just for a situation I found when testing multiple versions of the packages and it ended up returning an invalid value when there's more than one ISO in the directory.

I agree with you that the echo code is weak. However, always using the first entry found might lead to unexpected behavior too. I was thinking under which condition you got the situation that there is more than one image available and differently named ? In any case, covering this issue should be an extra PR.

I agree with you on this one. This was actually a problem caused by me during testing, there's no need to address it in this PR.

[doccaz]

@doccaz Thanks for looking into this.

With regards to #277 we moved the install location of the live ISO from /usr/share/migration-image to /migration-image. This move impacts the 99_migration code as you already found out. Because of this reason the two packages SLES15-Migration (which packages the live ISO into a package) and the package suse-migration-sle15-activation must be in sync. I updated the activation package in my :dms build later because I also stumbled over the issue you had. Therefore I assume when you branched my project you still got the not-in-sync package state. If you check it again it should be in sync and my tests worked.

I only branched "suse-migration-test-vm" to add an LVM partition layout (see here). The rest of the packages are being pulled from your repository automatically through KIWI. I also added SUSEConnect and a couple missing python dependencies to appliance.kiwi.

Here are the included versions in the qcow2 image:

• SLES15-Migration-2.0.40-1
• suse-migration-pre-checks-2.1.1-1
• suse-migration-2.1.1-1.1

I booted the qcow2 image and manually added the .repo file pointing to your repository. Finally, I installed suse-sle15-activation-2.1.1-1.1. I can see that the generated grub.cfg after the grub2-mkconfig is called has a blank 99_migration section.

My image has a VG called "vg_system" with 4 LVM volumes:

linux:~ # lvs
  LV     VG        Attr       LSize Pool Origin Data%  Meta%  Move Log Cpy%Sync Convert
  LVRoot vg_system -wi-ao---- 2,00g                                                    
  lv_opt vg_system -wi-ao---- 1,00g                                                    
  lv_usr vg_system -wi-ao---- 5,00g                                                    
  lv_var vg_system -wi-ao---- 5,00g  

If I run the script manually with sh -x, I see this:

+ '[' -z '' ']'
+ OS=Migration
++ echo /migration-image/SLES15-Migration.x86_64-2.0.40-Build1.8.iso
+ migration_iso=/migration-image/SLES15-Migration.x86_64-2.0.40-Build1.8.iso
++ grep -E '/$'
++ lsblk -p -n -r -o NAME,MOUNTPOINT
++ uniq
++ cut -f1 '-d '
+ root_device=/dev/mapper/vg_system-LVRoot
+ image_fs_device=/dev/mapper/vg_system-LVRoot
+ index=4
+ for path in '"/usr/share"' '"/usr"'
+ mountpoint -q /usr/share
+ index=3
+ for path in '"/usr/share"' '"/usr"'
+ mountpoint -q /usr
++ lsblk -p -n -r -o NAME,MOUNTPOINT
++ grep -E '/usr$'
++ cut -f1 '-d '
++ uniq
+ image_fs_device=/dev/mapper/vg_system-lv_usr
++ echo /migration-image/SLES15-Migration.x86_64-2.0.40-Build1.8.iso
++ cut -f3- -d/
+ migration_iso=/SLES15-Migration.x86_64-2.0.40-Build1.8.iso
+ break
++ blkid -s UUID -o value /dev/mapper/vg_system-lv_usr
+ image_fs_uuid=a992a0f9-2d23-4ef4-b3c1-2b25c8604ac9
++ blkid -s TYPE -o value /dev/mapper/vg_system-lv_usr
+ image_fs_type=ext3

The migration_iso variable starts with the correct value (/migration-image/SLES15-Migraiton*.iso), then it gets overwritten inside the for loop, because there IS an active volume for /usr but it DOES NOT contain the iso. In the end the wrong value gets assigned (/SLES15-Migration*.iso). That's why I suggested the extra file check.

I don't think we should add the complexity of two different search methods in the 99_migration code. Instead I suggest that we bump the version of the SLES15-Migration package and add a proper Requires: SLES15-Migration >= "..." to the spec file of the suse-migration-sle15-activation package

That's a valid way of using the correct version, I agree.

[schaefi]

@doccaz Oh now I see it. Thanks much for the trace output. Actually as we have now moved the location of the ISO to "/" instead of "/usr/share" the following code in 99_migration can be imho dropped because /migration-image will always be on the root device. Thus I suggest to drop

# check if location of migration_iso is not on the root_device
image_fs_device=${root_device}
index=4
for path in "/usr/share" "/usr";do
    if mountpoint -q ${path};then
        image_fs_device=$(
            lsblk -p -n -r -o NAME,MOUNTPOINT |\
            grep -E "${path}$" | uniq | cut -f1 -d" "
        )
        migration_iso=/$(echo "${migration_iso}" | cut -f"${index}"- -d/)
        break
    fi
    index=$((index -1))
done

completely

Thoughts ?

[schaefi]

@doccaz That's a valid way of using the correct version, I agree.

I provided #305 to address this part

[doccaz]

Here you go.

[doccaz]

Here you go.

[schaefi]

@doccaz Great thanks 👍 it looks good to me now, can you squash your commits into one ? then we are ready for merge. Thanks much

[schaefi]

@doccaz maybe if you have a minute, can you do me a favor and check #305 if it makes sense to you. Thanks again

[doccaz]

@doccaz maybe if you have a minute, can you do me a favor and check #305 if it makes sense to you. Thanks again

Yes, that makes sense and would avoid using the wrong image.

[schaefi]

@doccaz I have merged your and my PR and submitted the changes to:

• https://build.opensuse.org/project/show/home:marcus.schaefer:dms

I'm running some tests once everything as built. So far there are no release plans for the DMS to the SUSE namespace. I plan a refresh during the next three months or on demand if there is a prio bug coming up.