add onie image install's platform verification #8814

wen587 · 2021-09-22T06:38:04Z

Depends on #8542. Please DO NOT MERGE before it

Why I did it

Provide onie image install verification

How I did it

Add devices folder based on platform_asic and check if current platform inside the list

How to verify it

In onie env, install a bin file which differs the running platform's ASIC. For example;
install sonic-broadcom.bin to mellanox ASIC platform
install sonic-vs.bin to broadcom ASIC platform

Which release branch to backport (provide reason below if selected)

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

…orm list

wen587 · 2021-09-22T06:39:05Z

Based on #8542.
Wait for above PR merge first.

build_image.sh

qiluo-msft · 2021-09-22T08:43:10Z

installer/x86_64/install.sh

@@ -92,6 +92,39 @@ VAR_LOG_SIZE=4096

 [ -r platforms/$onie_platform ] && . platforms/$onie_platform

+# Verify image platform is inside devices list
+# Temporarily set TIMEOUT as 10 seconds
+if [ "$install_env" = "onie" ] && [ -d devices ]; then


onie

Is it possible to run on all the envrionment besides ONIE? #Closed

It could be an option among sonic/onie/build

We have sonic_installer to block wrong platform already. It might be tedious if user input --skip-platform-check and be prompted here the second time to confirm install.

Reasonable. Could you change sonic-utilities code with same logic, checking platform_asic instead of previous image asic.

Or, we could unify the logic here, so my comment will be finally applicable.

build_image.sh

installer/x86_64/install.sh

… aboot image

qiluo-msft · 2021-09-23T07:48:28Z

build_image.sh

+{
+    # Generate asic-specific ONIE device list
+    rm -rf ./installer/x86_64/devices/
+    mkdir -p ./installer/x86_64/devices/


You just need to create an empty file in this folder. No need to remove directory. #Closed

generate_onie_installer_image
I am following the above code to refresh the folder every time. And the folder needs to be created the first time.

qiluo-msft · 2021-09-23T08:01:37Z

#!/bin/sh

I notice there are many CPU architectures besides x86_64. Check installer/*/install.sh.

In reply to: 925587223

Refers to: installer/x86_64/install.sh:1 in e53fc70. [](commit_id = e53fc70, deletion_comment = False)

qiluo-msft · 2021-09-27T11:45:06Z

installer/x86_64/install.sh

+if [ "$install_env" = "onie" ] && [ "$machine" != "generic" ]; then
+    if ! grep -Fxq "$onie_platform" devices/platforms_asic; then
+        INPUT_TIMEOUT=10
+        echo "The image you're trying to install is of a different platform as the running platform"


platform

asic type #Closed

will change

qiluo-msft · 2021-09-27T11:45:09Z

installer/x86_64/install.sh

+if [ "$install_env" = "onie" ] && [ "$machine" != "generic" ]; then
+    if ! grep -Fxq "$onie_platform" devices/platforms_asic; then
+        INPUT_TIMEOUT=10
+        echo "The image you're trying to install is of a different platform as the running platform"


platform

asic #Closed

will change

qiluo-msft · 2021-09-27T11:48:07Z

installer/x86_64/install.sh

+        INPUT_TIMEOUT=10
+        echo "The image you're trying to install is of a different platform as the running platform"
+        while true; do
+            read -t $INPUT_TIMEOUT -r -p "Do you still wish to install this image? [y/n]: " input


$INPUT_TIMEOUT

Actually there is no requirement to timeout. If the script is used by automation, the prompt will be enough to break the normal path (The automation will have its own timeout).

If the script is used by user, it is too rush for them to response in 10 seconds, and without any hint. #Closed

will remove it

qiluo-msft · 2021-09-27T11:53:18Z

build_image.sh

+        cp ./installer/x86_64/devices/platforms_asic .platforms_asic
+        zip -g $OUTPUT_ABOOT_IMAGE .platforms_asic
+        rm .platforms_asic
+    fi


Implement a new function generate_aboot_device_list ? If some logic is common, you can extract into another function like generate_device_list. #Closed

qiluo-msft · 2021-09-30T07:58:56Z

build_image.sh

+    mkdir -p ./installer/x86_64/devices/
+    for d in `find -L ./device  -maxdepth 2 -mindepth 2 -type d`; do
+        if [ -f $d/platform_asic ] && grep -Fxq "$CONFIGURED_PLATFORM" $d/platform_asic; then
+            echo "${d##*/}" >> ./installer/x86_64/devices/platforms_asic;


./installer/x86_64/devices/platforms_asic

This filename could be replace by a function parameter, and onie/aboot will call the function with different argument.

Then you don't need the extra step cp ./installer/x86_64/devices/platforms_asic .platforms_asic #Closed

wen587 · 2021-10-09T02:29:04Z

Ready to be merged.

qiluo-msft · 2021-10-09T05:06:55Z

build_image.sh

@@ -170,6 +184,13 @@ elif [ "$IMAGE_TYPE" = "aboot" ]; then
    zip -g $ABOOT_BOOT_IMAGE version
    rm version

+    generate_device_list ".platforms_asic"
+    # Check file existence in case of general build


Check file existence in case of general build

What do you mean by "general build"? Is this file always existing after previous generating function? #Closed

I think the file doesn't exist in some case. For example, if the TARGET_MACHINE was set to 'general', it won't match any platform_asic. I will bypass the check here.

qiluo-msft · 2021-10-09T05:10:30Z

build_image.sh

+    local platforms_asic=$1
+    for d in `find -L ./device  -maxdepth 2 -mindepth 2 -type d`; do
+        if [ -f $d/platform_asic ] && grep -Fxq "$CONFIGURED_PLATFORM" $d/platform_asic; then
+            echo "${d##*/}" >> "$platforms_asic";


echo "${d##*/}" >> "$platforms_asic";

You need to clean the file at the begining in case the file is not empty. No need to rm before this function call.
If there is no platforms using CONFIGURED_PLATFORM asic, this function should generated an empty file. #Closed

In case of general build, sonic installer verification would bypass because $platforms_asic file is not there. But if it is a empty file, it will fail because no platform is in the file. I guess general build means we could install it to any asic.
CONFIGURED_PLATFORM is general by default.
Check out here. general

Thanks for the generic fix!

Still:
clean the file at the begining in case the file is not empty. No need to rm before this function call.
If there is no platforms using CONFIGURED_PLATFORM asic, this function should generated an empty file.

Proposed code

# Generate asic-specific device list generate_device_list() { local platforms_asic=$1 # Create an empty function, and later append to it echo -n > "$platforms_asic" for d in `find -L ./device -maxdepth 2 -mindepth 2 -type d`; do if [ -f $d/platform_asic ]; then if [ "$CONFIGURED_PLATFORM" = "generic" ] || grep -Fxq "$CONFIGURED_PLATFORM" $d/platform_asic; then echo "${d##*/}" >> "$platforms_asic"; fi; fi; done }

Otherwise, you need add rm .platforms_asic in the aboot branch, which is actually best cleaned inside this function.

wen587 · 2021-10-09T11:35:16Z

/azp run

azure-pipelines · 2021-10-09T11:35:35Z

Azure Pipelines successfully started running 1 pipeline(s).

wen587 · 2021-10-11T08:22:01Z

/azp run

azure-pipelines · 2021-10-11T08:22:26Z

Azure Pipelines successfully started running 1 pipeline(s).

2. resolve comment

TACappleman · 2021-12-15T10:45:46Z

@wen587 is there a way to bypass this check (or respond to it) in automation, e.g. if it's hit in a Jenkins run?

wen587 · 2021-12-15T11:49:55Z

@wen587 is there a way to bypass this check (or respond to it) in automation, e.g. if it's hit in a Jenkins run?

If you are using sonic-installer install, you can use sonic-installer install -f to bypass the check and force install.
If you are using onie install, a char of Y or y need to be responded if the platform does not match.

This reverts commit ba2968d.

add onie install check if current platform not in images target platf…

a81ccbc

…orm list

wen587 requested a review from qiluo-msft September 22, 2021 06:53

wen587 marked this pull request as ready for review September 22, 2021 06:57

wen587 requested a review from lguohan as a code owner September 22, 2021 06:57