Skip to content

Commit

Permalink
sonic-installer: enhance next image detection for Aboot (#3433)
Browse files Browse the repository at this point in the history
The Aboot bootloader relies of the SWI= keyword argument in the
boot-config file to know which image to boot.
This value is also used by sonic-installer to figure to extract the next
image that will be executed.
The current code has an issue as it only expects the next image to match
the installation path of a SONiC image but not anything else.

This means that `SWI=flash:sonic-aboot-broadcom.swi` is not valid and
can therefore be a problem when trying to install a new image via cold
reboot.

Additionally a missing or empty boot-config would generate a python
backtrace instead of gracefully recovering from this state.
  • Loading branch information
Staphylo authored and mssonicbld committed Sep 11, 2024
1 parent e99077a commit 1fd29a8
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
7 changes: 6 additions & 1 deletion sonic_installer/bootloader/aboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ class AbootBootloader(Bootloader):

def _boot_config_read(self, path=BOOT_CONFIG_PATH):
config = collections.OrderedDict()
if not os.path.exists(path):
return config
with open(path) as f:
for line in f.readlines():
line = line.strip()
Expand Down Expand Up @@ -112,7 +114,10 @@ def get_installed_images(self):

def get_next_image(self):
config = self._boot_config_read()
match = re.search(r"flash:/*(\S+)/", config['SWI'])
swi = config.get('SWI', '')
match = re.search(r"flash:/*(\S+)/", swi)
if not match:
return swi.split(':', 1)[-1]
return match.group(1).replace(IMAGE_DIR_PREFIX, IMAGE_PREFIX, 1)

def set_default_image(self, image):
Expand Down
21 changes: 17 additions & 4 deletions tests/installer_bootloader_aboot_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

# Constants
image_dir = f'{aboot.IMAGE_DIR_PREFIX}expeliarmus-{aboot.IMAGE_DIR_PREFIX}abcde'
image_chainloader = f'{image_dir}/.sonic-boot.swi'
exp_image = f'{aboot.IMAGE_PREFIX}expeliarmus-{aboot.IMAGE_DIR_PREFIX}abcde'
image_dirs = [image_dir]

Expand Down Expand Up @@ -45,15 +46,27 @@ def test_get_installed_images():
assert bootloader.get_installed_images() == [exp_image]


@patch("sonic_installer.bootloader.aboot.re.search")
def test_get_next_image(re_search_patch):
def test_get_next_image():
bootloader = aboot.AbootBootloader()
bootloader._boot_config_read = Mock(return_value={'SWI': None})

# Test missing boot-config
bootloader._boot_config_read()

# Test missing SWI value
bootloader._boot_config_read = Mock(return_value={})
assert bootloader.get_next_image() == ''

# Test convertion image dir to image name
re_search_patch().group = Mock(return_value=image_dir)
swi = f'flash:{image_chainloader}'
bootloader._boot_config_read = Mock(return_value={'SWI': swi})
assert bootloader.get_next_image() == exp_image

# Test some other image
next_image = 'EOS.swi'
bootloader._boot_config_read = Mock(return_value={'SWI': f'flash:{next_image}'})
assert bootloader.get_next_image() == next_image


def test_install_image():
image_path = 'sonic'
env = os.environ.copy()
Expand Down

0 comments on commit 1fd29a8

Please sign in to comment.