SONiC installer - check new image type before checking installed version and small improvements

[vaibhavhd]

- What I did
SONiC master and 201911 images check the OS version of the new image before checking new image type (ABOOT/ONIE).
Ideally, image type should be verified first and then proceed with checking the OS version along with setting default OS version.

Fixes #1195
Before: (note that type bin is incompatible on aboot platform. But the installation did not give an error:

admin@abc-7060:~$ sudo sonic_installer install -y http://1.1.1.1/sonic-broadcom.bin
Downloading image...
...99%, 756 MB, 48399 KB/s, 0 seconds left...   Image SONiC-OS-20191130.51 is already installed. Setting it as default...
Command: sync;sync;sync

Command: sleep 3

Done
admin@abc-7060:~$

Now:

admin@str2-7060cx-32s-29:~$ sudo sonic_installer install -yhttp://1.1.1.1/sonic-broadcom.bin
Downloading image...
...95%, 724 MB, 57044 KB/s, 0 seconds left...   Image file 'http://1.1.1.1/sonic-broadcom.bin' is of a different type than running image.
If you are sure you want to install this image, use -f|--force.
Aborting...
Aborted!

- How I did it
Check compatibility of new image type before verifying the installed version.
Fix error message.

- How to verify it
Verified on a aboot platform and the incompatible image installation fails with an error instead of a success message.

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

[jleveque]

Is there a problem with the current behavior? If a image with the same version as the image file trying to be installed, the image is not installed, but that image version is set to the new default version. Thus, it doesn't matter if the image type is incorrect, it will not be installed because that version is already installed. What was the issue?

[vaibhavhd]

Is there a problem with the current behavior? If a image with the same version as the image file trying to be installed, the image is not installed, but that image version is set to the new default version. Thus, it doesn't matter if the image type is incorrect, it will not be installed because that version is already installed. What was the issue?

In the sequence of checks before the installation, shouldn't the type verification happen before the OS version verification?
I am running an upgrade test, where if I am sending a bin image to be installed on Arista, it should fail, but the test passes, as the current behavior first checks OS version and then returns success.
Shouldn't the user be warned that the image type that is to be installed is not meant for this platform?

[jleveque]

In the sequence of checks before the installation, shouldn't the type verification happen before the OS version verification?

It wouldn't hurt, and arguably may be more correct. My original intention was to first check if the image version is already installed, because if the image version is already installed, it doesn't matter if the image type is incorrect, because we don't need to install the image. However, we will ensure that image is set to the default version, thus the end result is that the image is installed (because it already was) and is set to the default. Technically the same result as a successful installation, so I don't consider it a failure.

I am running an upgrade test, where if I am sending a bin image to be installed on Arista, it should fail, but the test passes, as the current behavior first checks OS version and then returns success.
Shouldn't the user be warned that the image type that is to be installed is not meant for this platform?

If the version of the image isn't already installed on the device, the operation will error out. For the test, are you trying to install the same version image which is already installed?

[vaibhavhd]

In the sequence of checks before the installation, shouldn't the type verification happen before the OS version verification?

It wouldn't hurt, and arguably may be more correct. My original intention was to first check if the image version is already installed, because if the image version is already installed, it doesn't matter if the image type is incorrect, because we don't need to install the image. However, we will ensure that image is set to the default version, thus the end result is that the image is installed (because it already was) and is set to the default. Technically the same result as a successful installation, so I don't consider it a failure.

This is interesting for me. I believed that we will error out no-matter-what if a command is executed even to attempt to install an ONIE image on ABOOT device.

I am running an upgrade test, where if I am sending a bin image to be installed on Arista, it should fail, but the test passes, as the current behavior first checks OS version and then returns success.
Shouldn't the user be warned that the image type that is to be installed is not meant for this platform?

If the version of the image isn't already installed on the device, the operation will error out. For the test, are you trying to install the same version image which is already installed?

I see. Yes, I tried installing same version but different type. I know that with different version AND type, the installer will error out ( as expected).

I can close the issue, and undo changes here. But still the message needs to be corrected where string substitution does not happen as expected, and hence we get the below:
Image file '{}' is of a different type than running image.

[jleveque]

For now, can you open a small PR to fix the string formatting?

[vaibhavhd]

For now, can you open a small PR to fix the string formatting?

Sure, created a new PR for that - #1197