[fwutil] Fix Various Bugs Identified in Regression

tests/platform_tests/fwutil/conftest.py

@@ -65,7 +65,7 @@ def host_firmware(localhost, duthost):
 def next_image(duthost, fw_pkg):
 
     # Install next version of sonic
-    current = duthost.shell('sonic_installer list | grep Current | cut -f2 -d " "')['stdout']
+    current = duthost.shell('sonic-installer list | grep Current | cut -f2 -d " "')['stdout']
 
     image = fw_pkg.get("images", {}).keys()
     target = None
@@ -78,9 +78,14 @@ def next_image(duthost, fw_pkg):
         pytest.skip("No suitable image definitions found in config")
 
     logger.info("Installing new image {}".format(target))
-    duthost.copy(src=os.path.join("firmware", fw_pkg["images"][target]), dest=DUT_HOME)
+
+    if fw_pkg["images"][target].startswith("http"):
+        duthost.get_url(url=fw_pkg["images"][target], dest=DUT_HOME)
+    else:
+        duthost.copy(src=os.path.join("firmware", fw_pkg["images"][target]), dest=DUT_HOME)
+
     remote_path = os.path.join(DUT_HOME, os.path.basename(fw_pkg["images"][target]))
-    duthost.command("sonic_installer install -y {}".format(remote_path), module_ignore_errors=True)
+    duthost.command("sonic-installer install -y {}".format(remote_path), module_ignore_errors=True)
 
     # Mount newly installed image
     fs_path = FS_PATH_TEMPLATE.format(target)
@@ -91,9 +96,10 @@ def next_image(duthost, fw_pkg):
 
     logger.info("Attempting to stage test firware onto newly-installed image.")
     try:
-        wait_until(10, 1, 0, check_path_exists, fs_rw)
-
         duthost.command("mkdir -p {}".format(fs_mountpoint))
+        duthost.command("mkdir -p {}".format(fs_rw))
+        duthost.command("mkdir -p {}".format(fs_work))
+
         cmd = "mount -t squashfs {} {}".format(fs_path, fs_mountpoint)
         duthost.command(cmd)
 
@@ -106,11 +112,11 @@ def next_image(duthost, fw_pkg):
         )
         duthost.command(cmd)
     except Exception as e:
+        duthost.command("sonic-installer remove {} -y".format("SONiC-OS-{}".format(target)))
         pytest.fail("Failed to setup next-image.")
-        duthost.command("sonic_installer set-default {}".format(current))
 
     yield overlay_mountpoint
 
     logger.info("Ensuring correct image is set to default boot.")
-    duthost.command("sonic_installer set-default {}".format(current))
+    duthost.command("sonic-installer remove {} -y".format("SONiC-OS-{}".format(target)))
 

tests/platform_tests/fwutil/fwutil_common.py

@@ -1,3 +1,4 @@
+import allure
 import pytest
 import os
 import json
@@ -9,13 +10,15 @@
 
 logger = logging.getLogger(__name__)
 
+TEMP_STATUS_FILE = "/tmp/firmwareupdate/fw_au_status"
+
 WARM_REBOOT = "warm"
 COLD_REBOOT = "cold"
 POWER_CYCLE = "power off"
 FAST_REBOOT = "fast"
 
 DEVICES_PATH="usr/share/sonic/device"
-TIMEOUT=3600
+TIMEOUT=1200
 REBOOT_TYPES = {
     COLD_REBOOT: "reboot",
     WARM_REBOOT: "warm-reboot",
@@ -28,6 +31,11 @@ def find_pattern(lines, pattern):
             return True
     return False
 
+def get_hw_revision(duthost):
+    out = duthost.command("show platform summary")
+    rev_line = out["stdout"].splitlines()[6]
+    return rev_line.split(": ")[1]
+
 def power_cycle(duthost=None, pdu_ctrl=None, delay_time=60):
     if pdu_ctrl is None:
         pytest.skip("No PSU controller for %s, skipping" % duthost.hostname)
@@ -65,7 +73,7 @@ def complete_install(duthost, localhost, boot_type, res, pdu_ctrl, auto_reboot=F
 
         logger.info("Waiting on switch to shutdown...")
         # Wait for ssh flap
-        localhost.wait_for(host=hn, port=22, state='stopped', delay=10, timeout=timeout)
+        localhost.wait_for(host=hn, port=22, state='stopped', delay=1, timeout=timeout)
         logger.info("Letting switch get through ONIE / BIOS before pinging....")
         time.sleep(300)
         logger.info("Waiting on switch to come up....")
@@ -114,18 +122,24 @@ def show_firmware(duthost):
 
     return output_data
 
-def get_install_paths(duthost, fw, versions, chassis):
+def get_install_paths(duthost, fw, versions, chassis, target_component):
     component = fw["chassis"].get(chassis, {})["component"]
     ver = versions["chassis"].get(chassis, {})["component"]
 
     paths = {}
 
+    if target_component is not None:
+        component = {target_component: component[target_component]}
+
     for comp, revs in component.items():
         if comp in ver:
             if revs[0].get("upgrade_only", False) and ver[comp] not in [r["version"] for r in revs]:
                 log.warning("Firmware is upgrade only and existing firmware {} is not present in version list. Skipping {}".format(ver[comp], comp))
                 continue
             for i, rev in enumerate(revs):
+                if "hw_revision" in rev and rev["hw_revision"] != get_hw_revision(duthost):
+                    log.warning("Firmware {} only supports HW Revision {} and this chassis is {}. Skipping".format(rev["version"], rev["hw_revision"], get_hw_revision(duthost)))
+                    continue
                 if rev["version"] != ver[comp]:
                     paths[comp] = rev
                     break
@@ -157,6 +171,9 @@ def generate_config(duthost, cfg, versions):
 def upload_platform(duthost, paths, next_image=None):
     target = next_image if next_image else "/"
 
+    # Clear auto update status file
+    duthost.command("rm -rf {}".format(TEMP_STATUS_FILE))
+
     # Backup the original platform_components.json file
     duthost.fetch(dest=os.path.join("firmware", "platform_components_backup.json"), 
             src=os.path.join(target, DEVICES_PATH, duthost.facts["platform"], "platform_components.json"),
@@ -170,13 +187,12 @@ def upload_platform(duthost, paths, next_image=None):
         os.path.join(target, DEVICES_PATH, duthost.facts["platform"])))
 
     for comp, dat in paths.items():
-        duthost.copy(src=os.path.join("firmware", dat["firmware"]), 
-                dest=os.path.join(target, DEVICES_PATH, duthost.facts["platform"]))
-        if "install" in dat:
-            duthost.copy(src=os.path.join("firmware", dat["install"]["firmware"]), 
+        if dat["firmware"].startswith("http"):
+            duthost.get_url(url=dat["firmware"], 
+                    dest=os.path.join(target, DEVICES_PATH, duthost.facts["platform"]))
+        else:
+            duthost.copy(src=os.path.join("firmware", dat["firmware"]), 
                     dest=os.path.join(target, DEVICES_PATH, duthost.facts["platform"]))
-            logger.info("Copying {} to {}".format(os.path.join("firmware", dat["install"]["firmware"]),
-                os.path.join(target, DEVICES_PATH, duthost.facts["platform"])))
 
 def validate_versions(init, final, config, chassis, boot):
     final = final["chassis"][chassis]["component"]
@@ -189,72 +205,77 @@ def validate_versions(init, final, config, chassis, boot):
     return True
 
 def call_fwutil(duthost, localhost, pdu_ctrl, fw, component=None, next_image=None, boot=None, basepath=None):
+    allure.step("Collect firmware versions")
     logger.info("Calling fwutil with component: {} | next_image: {} | boot: {} | basepath: {}".format(component, next_image, boot, basepath))
     init_versions = show_firmware(duthost)
     logger.info("Initial Versions: {}".format(init_versions))
     chassis = init_versions["chassis"].keys()[0] # Only one chassis
-    paths = get_install_paths(duthost, fw, init_versions, chassis)
+    paths = get_install_paths(duthost, fw, init_versions, chassis, component)
     current = duthost.shell('sonic_installer list | grep Current | cut -f2 -d " "')['stdout']
 
+    allure.step("Upload firmware to DUT")
     generate_config(duthost, paths, init_versions)
     upload_platform(duthost, paths, next_image)
 
+    allure.step("Execute fwutil command")
     command = "fwutil"
     if basepath is not None:
         command += " install"
+        auto_reboot = paths[component].get("force_reboot", False)
     else:
         command += " update"
+        auto_reboot = True
 
     if component is None:
-        command += " all"
+        command += " all fw"
     else:
         if component not in paths:
             pytest.skip("No available firmware to install on {}. Skipping".format(component))
         command += " chassis component {} fw".format(component)
 
     if basepath is not None:
         # Install file is override if API implementation needs a different file for install / update
-        filepath = paths[component]["install"]["firmware"] if "install" in paths[component] else paths[component]["firmware"]
+        filepath = paths[component]["firmware"]
         command += " {}".format(os.path.join(basepath, os.path.basename(filepath)))
 
     if next_image is not None:
         command += " --image={}".format("next" if next_image else "current")
 
     if boot is not None:
         command += " --boot={}".format(boot)
-
-    command += " -y"
+        auto_reboot = False
+    else:
+        command += " -y"
 
     logger.info("Running install command: {}".format(command))
     task, res = duthost.command(command, module_ignore_errors=True, module_async=True)
     boot_type = boot if boot else paths[component]["reboot"][0]
 
-    auto_reboot = False
-    for comp in paths.keys():
-        if "install" in paths[comp] and basepath is not None:
-            if paths[comp]["install"].get("auto_reboot", False): auto_reboot = True
-        else:
-            if paths[comp].get("auto_reboot", False): auto_reboot = True
-
+    allure.step("Perform Neccesary Reboot")
     timeout = max([v.get("timeout", TIMEOUT) for k, v in paths.items()])
     pdu_delay = fw["chassis"][chassis].get("power_cycle_delay", 60)
     complete_install(duthost, localhost, boot_type, res, pdu_ctrl, auto_reboot, current, next_image, timeout, pdu_delay)
 
+    allure.step("Collect Updated Firmware Versions")
+    time.sleep(2) # Give a little bit of time in case of no-op install for mounts to complete
     final_versions = show_firmware(duthost)
-    assert validate_versions(init_versions, final_versions, paths, chassis, boot_type)
+    test_result = validate_versions(init_versions, final_versions, paths, chassis, boot_type)
 
-    duthost.copy(src=os.path.join("firmware", "platform_components_backup.json"), 
-            dest=os.path.join(target, DEVICES_PATH, duthost.facts["platform"], "platform_components.json"))
-    logger.info("Restoring backup platform_components.json to {}".format(
-        os.path.join(DEVICES_PATH, duthost.facts["platform"])))
+    allure.step("Begin Switch Restoration")
+    if next_image is None:
+        duthost.copy(src=os.path.join("firmware", "platform_components_backup.json"), 
+                dest=os.path.join("/", DEVICES_PATH, duthost.facts["platform"], "platform_components.json"))
+        logger.info("Restoring backup platform_components.json to {}".format(
+            os.path.join(DEVICES_PATH, duthost.facts["platform"])))
 
-    update_needed = copy(fw)
+    update_needed = deepcopy(fw)
+    update_needed["chassis"][chassis]["component"] = {}
     for comp in paths.keys():
-        if fw["chassis"][chassis]["component"][comp][0]["version"] == final_versions[comp] or paths[comp]["upgrade_only"]:
-            del update_needed["chassis"][chassis]["component"][comp]
+        if fw["chassis"][chassis]["component"][comp][0]["version"] != final_versions["chassis"][chassis]["component"][comp] and boot in fw["chassis"][chassis]["component"][comp][0]["reboot"] + [None] and not paths[comp].get("upgrade_only", False):
+            update_needed["chassis"][chassis]["component"][comp] = fw["chassis"][chassis]["component"][comp]
     if len(update_needed["chassis"][chassis]["component"].keys()) > 0:
         logger.info("Latest firmware not installed after test. Installing....")
-        call_fwutil(duthost, localhost, pdu_ctrl, update_needed, component, None, boot, basepath)
+        call_fwutil(duthost, localhost, pdu_ctrl, update_needed, component, None, boot, os.path.join("/", DEVICES_PATH, duthost.facts['platform']) if basepath is not None else None)
 
-    return True
+    return test_result
 

tests/platform_tests/fwutil/test_fwutil.py

@@ -21,8 +21,9 @@ def test_fwutil_show(duthost):
 
     show_fw_comp_set = set(versions["chassis"][chassis]["component"].keys())
     platform_comp_set = set(platform_comp["chassis"][chassis]["component"].keys())
+    comp = show_fw_comp_set == platform_comp_set
 
-    assert show_fw_comp_set == platform_comp_set
+    assert comp
 
 def test_fwutil_install_file(duthost, localhost, pdu_controller, fw_pkg, random_component):
     """Tests manually installing firmware to a component from a file."""
@@ -107,12 +108,12 @@ def test_fwutil_update_bad_config(duthost, fw_pkg, random_component):
     assert found_bad_component
 
 
-@pytest.mark.parametrize("reboot_type", ["none", "warm", "fast", "cold", "power off"])
+@pytest.mark.parametrize("reboot_type", ["none", "cold"])
 def test_fwutil_auto(duthost, localhost, pdu_controller, fw_pkg, reboot_type):
     """Tests fwutil update all command ability to properly select firmware for install based on boot type."""
     assert call_fwutil(duthost,
             localhost,
             pdu_controller,
             fw_pkg,
-            reboot=reboot_type)
+            boot=reboot_type)