From e0aebbd652e7a564541612ce43b1dd44c03dc672 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Thu, 16 Jan 2025 08:41:57 +0000 Subject: [PATCH 01/13] add dbus api for listing sonic images. --- host_modules/image_service.py | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index e30e1293..63ccc8de 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -133,3 +133,45 @@ def checksum(self, file_path, algorithm): except Exception as e: logger.error("Failed to calculate checksum: {}".format(e)) return errno.EIO, str(e) + + @host_service.method( + host_service.bus_name(MOD_NAME), in_signature="", out_signature="a{ss}" + ) + def list(self): + """ + List the current, next, and available SONiC images. + + Returns: + A dictionary with keys "current", "next", and "available". + """ + logger.info("Listing SONiC images") + + try: + output = subprocess.check_output( + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, + ).decode().strip().split("\n") + + current_image = "" + next_image = "" + available_images = [] + + for line in output: + if line.startswith("Current:"): + current_image = line.split("Current:")[1].strip() + elif line.startswith("Next:"): + next_image = line.split("Next:")[1].strip() + elif line.startswith("Available:"): + continue + else: + available_images.append(line.strip()) + + except subprocess.CalledProcessError as e: + logger.error("Failed to list images: {}".format(e.output.decode())) + return errno.EIO, "Failed to list images" + + return 0, { + "current": current_image, + "next": next_image, + "available": available_images, + } \ No newline at end of file From 11308406da466dcdc9b00c96c1a9e6f66cdcdb0e Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Thu, 16 Jan 2025 16:11:40 +0000 Subject: [PATCH 02/13] add unit tests. --- tests/host_modules/image_service_test.py | 56 ++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/host_modules/image_service_test.py b/tests/host_modules/image_service_test.py index 22e70e82..39046cfe 100644 --- a/tests/host_modules/image_service_test.py +++ b/tests/host_modules/image_service_test.py @@ -370,3 +370,59 @@ def test_checksum_general_exception( ), "message should contain 'general error'" mock_isfile.assert_called_once_with(file_path) mock_open.assert_called_once_with(file_path, "rb") + + @mock.patch("dbus.SystemBus") + @mock.patch("dbus.service.BusName") + @mock.patch("dbus.service.Object.__init__") + @mock.patch("subprocess.check_output") + def test_list_success(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + """ + Test that the `list` method successfully lists the current, next, and available SONiC images. + """ + # Arrange + image_service = ImageService(mod_name="image_service") + mock_output = ( + "Current: current_image\n" + "Next: next_image\n" + "Available:\n" + "image1\n" + "image2\n" + ) + mock_check_output.return_value = mock_output.encode() + + # Act + rc, images = image_service.list() + + # Assert + assert rc == 0, "wrong return value" + assert images["current"] == "current_image", "current image does not match" + assert images["next"] == "next_image", "next image does not match" + assert images["available"] == ["image1", "image2"], "available images do not match" + mock_check_output.assert_called_once_with( + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, + ) + + @mock.patch("dbus.SystemBus") + @mock.patch("dbus.service.BusName") + @mock.patch("dbus.service.Object.__init__") + @mock.patch("subprocess.check_output") + def test_list_failed(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + """ + Test that the `list` method fails when the subprocess command returns a non-zero exit code. + """ + # Arrange + image_service = ImageService(mod_name="image_service") + mock_check_output.side_effect = subprocess.CalledProcessError( + returncode=1, cmd="sonic-installer list", output=b"Error: command failed" + ) + + # Act + rc, msg = image_service.list() + + # Assert + assert rc != 0, "wrong return value" + mock_check_output.assert_called_once_with( + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, + ) \ No newline at end of file From 56663a514ac52c94aab2b9e8f0b832ab2e795150 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Thu, 16 Jan 2025 16:53:18 +0000 Subject: [PATCH 03/13] address copilot comment. --- host_modules/image_service.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index 63ccc8de..23d2cbd4 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -171,7 +171,7 @@ def list(self): return errno.EIO, "Failed to list images" return 0, { - "current": current_image, - "next": next_image, - "available": available_images, + "current": current_image or "", + "next": next_image or "", + "available": available_images or [], } \ No newline at end of file From 35f60a7fbab89bf2705101df7c91db0175b0870d Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Tue, 21 Jan 2025 16:28:55 +0000 Subject: [PATCH 04/13] use json string or error message as output. --- host_modules/image_service.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index 23d2cbd4..88ccb618 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -12,6 +12,7 @@ import requests import stat import subprocess +import json from host_modules import host_service import tempfile @@ -135,21 +136,21 @@ def checksum(self, file_path, algorithm): return errno.EIO, str(e) @host_service.method( - host_service.bus_name(MOD_NAME), in_signature="", out_signature="a{ss}" + host_service.bus_name(MOD_NAME), in_signature="", out_signature="is" ) def list(self): """ List the current, next, and available SONiC images. Returns: - A dictionary with keys "current", "next", and "available". + A tuple with an error code and a JSON string with keys "current", "next", and "available" or an error message. """ logger.info("Listing SONiC images") try: output = subprocess.check_output( - ["/usr/local/bin/sonic-installer", "list"], - stderr=subprocess.STDOUT, + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, ).decode().strip().split("\n") current_image = "" @@ -166,12 +167,12 @@ def list(self): else: available_images.append(line.strip()) + result = { + "current": current_image or "", + "next": next_image or "", + "available": available_images or [], + } + return 0, json.dumps(result) except subprocess.CalledProcessError as e: logger.error("Failed to list images: {}".format(e.output.decode())) - return errno.EIO, "Failed to list images" - - return 0, { - "current": current_image or "", - "next": next_image or "", - "available": available_images or [], - } \ No newline at end of file + return errno.EIO, json.dumps({"error": "Failed to list images: {}".format(e.output.decode())}) \ No newline at end of file From a0b223bf6bb2d7ace4198d4b06b4fb9ec552c5b2 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Tue, 21 Jan 2025 16:56:22 +0000 Subject: [PATCH 05/13] address comment. --- host_modules/image_service.py | 62 +++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index 88ccb618..a71152e3 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -149,30 +149,42 @@ def list(self): try: output = subprocess.check_output( - ["/usr/local/bin/sonic-installer", "list"], - stderr=subprocess.STDOUT, - ).decode().strip().split("\n") - - current_image = "" - next_image = "" - available_images = [] - - for line in output: - if line.startswith("Current:"): - current_image = line.split("Current:")[1].strip() - elif line.startswith("Next:"): - next_image = line.split("Next:")[1].strip() - elif line.startswith("Available:"): - continue - else: - available_images.append(line.strip()) - - result = { - "current": current_image or "", - "next": next_image or "", - "available": available_images or [], - } + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, + ).decode().strip() + result = self._parse_sonic_installer_list(output) + logger.info("List result: {}".format(result)) return 0, json.dumps(result) except subprocess.CalledProcessError as e: - logger.error("Failed to list images: {}".format(e.output.decode())) - return errno.EIO, json.dumps({"error": "Failed to list images: {}".format(e.output.decode())}) \ No newline at end of file + logger.error("Failed to list images: {} with return code {}".format(e.output.decode(), e.returncode)) + return e.returncode, json.dumps({"error": "Failed to list images: {}".format(e.output.decode())}) + + def _parse_sonic_installer_list(self, output): + """ + Parse the output of the sonic-installer list command. + + Args: + output: The output of the sonic-installer list command. + + Returns: + A dictionary with keys "current", "next", and "available" containing the respective images. + """ + current_image = "" + next_image = "" + available_images = [] + + for line in output.split("\n"): + if "current:" in line.lower(): + current_image = line.split(":")[1].strip() + elif "next:" in line.lower(): + next_image = line.split(":")[1].strip() + elif "available:" in line.lower(): + continue + else: + available_images.append(line.strip()) + + return { + "current": current_image or "", + "next": next_image or "", + "available": available_images or [], + } \ No newline at end of file From d8b5708c49c1ef7d64904687f87c44609402010c Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Tue, 21 Jan 2025 17:04:34 +0000 Subject: [PATCH 06/13] update the method name to list_image. --- host_modules/image_service.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index a71152e3..b8328eba 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -138,7 +138,7 @@ def checksum(self, file_path, algorithm): @host_service.method( host_service.bus_name(MOD_NAME), in_signature="", out_signature="is" ) - def list(self): + def list_images(self): """ List the current, next, and available SONiC images. @@ -149,8 +149,8 @@ def list(self): try: output = subprocess.check_output( - ["/usr/local/bin/sonic-installer", "list"], - stderr=subprocess.STDOUT, + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, ).decode().strip() result = self._parse_sonic_installer_list(output) logger.info("List result: {}".format(result)) From c8104f5ec893cd5786a74602c9a34120ea5bb044 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Tue, 21 Jan 2025 17:16:03 +0000 Subject: [PATCH 07/13] update unit test. --- tests/host_modules/image_service_test.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/host_modules/image_service_test.py b/tests/host_modules/image_service_test.py index 39046cfe..94257794 100644 --- a/tests/host_modules/image_service_test.py +++ b/tests/host_modules/image_service_test.py @@ -4,6 +4,7 @@ import os import stat import pytest +import json from unittest import mock from host_modules.image_service import ImageService @@ -375,9 +376,9 @@ def test_checksum_general_exception( @mock.patch("dbus.service.BusName") @mock.patch("dbus.service.Object.__init__") @mock.patch("subprocess.check_output") - def test_list_success(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + def test_list_image_success(self, mock_check_output, MockInit, MockBusName, MockSystemBus): """ - Test that the `list` method successfully lists the current, next, and available SONiC images. + Test that the `list_images` method successfully lists the current, next, and available SONiC images. """ # Arrange image_service = ImageService(mod_name="image_service") @@ -391,7 +392,8 @@ def test_list_success(self, mock_check_output, MockInit, MockBusName, MockSystem mock_check_output.return_value = mock_output.encode() # Act - rc, images = image_service.list() + rc, images_json = image_service.list_images() + images = json.loads(images_json) # Assert assert rc == 0, "wrong return value" @@ -407,9 +409,9 @@ def test_list_success(self, mock_check_output, MockInit, MockBusName, MockSystem @mock.patch("dbus.service.BusName") @mock.patch("dbus.service.Object.__init__") @mock.patch("subprocess.check_output") - def test_list_failed(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + def test_list_image_failed(self, mock_check_output, MockInit, MockBusName, MockSystemBus): """ - Test that the `list` method fails when the subprocess command returns a non-zero exit code. + Test that the `list_image` method fails when the subprocess command returns a non-zero exit code. """ # Arrange image_service = ImageService(mod_name="image_service") @@ -418,7 +420,7 @@ def test_list_failed(self, mock_check_output, MockInit, MockBusName, MockSystemB ) # Act - rc, msg = image_service.list() + rc, msg = image_service.list_images() # Assert assert rc != 0, "wrong return value" From 01d7ecabe457fee6df597bd3e181c382fd8da5bb Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Tue, 21 Jan 2025 21:55:08 +0000 Subject: [PATCH 08/13] change name from list_images to list_image. --- host_modules/image_service.py | 7 ++++--- tests/host_modules/image_service_test.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index b8328eba..6a765884 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -138,7 +138,7 @@ def checksum(self, file_path, algorithm): @host_service.method( host_service.bus_name(MOD_NAME), in_signature="", out_signature="is" ) - def list_images(self): + def list_image(self): """ List the current, next, and available SONiC images. @@ -156,8 +156,9 @@ def list_images(self): logger.info("List result: {}".format(result)) return 0, json.dumps(result) except subprocess.CalledProcessError as e: - logger.error("Failed to list images: {} with return code {}".format(e.output.decode(), e.returncode)) - return e.returncode, json.dumps({"error": "Failed to list images: {}".format(e.output.decode())}) + msg = "Failed to list images: command {} failed with return code {} and message {}".format(e.cmd, e.returncode, e.output.decode()) + logger.error(msg) + return e.returncode, msg def _parse_sonic_installer_list(self, output): """ diff --git a/tests/host_modules/image_service_test.py b/tests/host_modules/image_service_test.py index 94257794..75d86f68 100644 --- a/tests/host_modules/image_service_test.py +++ b/tests/host_modules/image_service_test.py @@ -392,7 +392,7 @@ def test_list_image_success(self, mock_check_output, MockInit, MockBusName, Mock mock_check_output.return_value = mock_output.encode() # Act - rc, images_json = image_service.list_images() + rc, images_json = image_service.list_image() images = json.loads(images_json) # Assert From ab8fa1ac77db4df60041bdce64f935264c61e44c Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Tue, 21 Jan 2025 21:56:43 +0000 Subject: [PATCH 09/13] chnage to list_images. --- host_modules/image_service.py | 2 +- tests/host_modules/image_service_test.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index 6a765884..6f9e1bde 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -138,7 +138,7 @@ def checksum(self, file_path, algorithm): @host_service.method( host_service.bus_name(MOD_NAME), in_signature="", out_signature="is" ) - def list_image(self): + def list_images(self): """ List the current, next, and available SONiC images. diff --git a/tests/host_modules/image_service_test.py b/tests/host_modules/image_service_test.py index 75d86f68..048e356a 100644 --- a/tests/host_modules/image_service_test.py +++ b/tests/host_modules/image_service_test.py @@ -376,7 +376,7 @@ def test_checksum_general_exception( @mock.patch("dbus.service.BusName") @mock.patch("dbus.service.Object.__init__") @mock.patch("subprocess.check_output") - def test_list_image_success(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + def test_list_images_success(self, mock_check_output, MockInit, MockBusName, MockSystemBus): """ Test that the `list_images` method successfully lists the current, next, and available SONiC images. """ @@ -392,7 +392,7 @@ def test_list_image_success(self, mock_check_output, MockInit, MockBusName, Mock mock_check_output.return_value = mock_output.encode() # Act - rc, images_json = image_service.list_image() + rc, images_json = image_service.list_images() images = json.loads(images_json) # Assert @@ -409,7 +409,7 @@ def test_list_image_success(self, mock_check_output, MockInit, MockBusName, Mock @mock.patch("dbus.service.BusName") @mock.patch("dbus.service.Object.__init__") @mock.patch("subprocess.check_output") - def test_list_image_failed(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + def test_list_images_failed(self, mock_check_output, MockInit, MockBusName, MockSystemBus): """ Test that the `list_image` method fails when the subprocess command returns a non-zero exit code. """ From 8dd89cde17c66b13d268b27b53a1f8580d5f1ec1 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Thu, 23 Jan 2025 16:21:40 +0000 Subject: [PATCH 10/13] handles the case where current and available images are empty. --- host_modules/image_service.py | 8 ++- tests/host_modules/image_service_test.py | 66 +++++++++++++++++++++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index 6f9e1bde..924ee21e 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -176,9 +176,13 @@ def _parse_sonic_installer_list(self, output): for line in output.split("\n"): if "current:" in line.lower(): - current_image = line.split(":")[1].strip() + parts = line.split(":") + if len(parts) > 1: + current_image = parts[1].strip() elif "next:" in line.lower(): - next_image = line.split(":")[1].strip() + parts = line.split(":") + if len(parts) > 1: + next_image = parts[1].strip() elif "available:" in line.lower(): continue else: diff --git a/tests/host_modules/image_service_test.py b/tests/host_modules/image_service_test.py index 048e356a..2f116c96 100644 --- a/tests/host_modules/image_service_test.py +++ b/tests/host_modules/image_service_test.py @@ -405,6 +405,69 @@ def test_list_images_success(self, mock_check_output, MockInit, MockBusName, Moc stderr=subprocess.STDOUT, ) + @mock.patch("dbus.SystemBus") + @mock.patch("dbus.service.BusName") + @mock.patch("dbus.service.Object.__init__") + @mock.patch("subprocess.check_output") + def test_list_images_success_empty_current_image(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + """ + Test that the `list_images` method successfully lists the current, next, and available SONiC images. + """ + # Arrange + image_service = ImageService(mod_name="image_service") + mock_output = ( + "Current:\n" + "Next: next_image\n" + "Available:\n" + "image1\n" + "image2\n" + ) + mock_check_output.return_value = mock_output.encode() + + # Act + rc, images_json = image_service.list_images() + images = json.loads(images_json) + + # Assert + assert rc == 0, "wrong return value" + assert images["current"] == "", "current image should be empty" + assert images["next"] == "next_image", "next image does not match" + assert images["available"] == ["image1", "image2"], "available images do not match" + mock_check_output.assert_called_once_with( + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, + ) + + @mock.patch("dbus.SystemBus") + @mock.patch("dbus.service.BusName") + @mock.patch("dbus.service.Object.__init__") + @mock.patch("subprocess.check_output") + def test_list_images_success_empty_available_images(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + """ + Test that the `list_images` method successfully lists the current, next, and available SONiC images. + """ + # Arrange + image_service = ImageService(mod_name="image_service") + mock_output = ( + "Current: current_image\n" + "Next: next_image\n" + "Available:\n" + ) + mock_check_output.return_value = mock_output.encode() + + # Act + rc, images_json = image_service.list_images() + images = json.loads(images_json) + + # Assert + assert rc == 0, "wrong return value" + assert images["available"] == [], "available images should be empty" + mock_check_output.assert_called_once_with( + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, + ) + + @mock.patch("dbus.SystemBus") @mock.patch("dbus.service.BusName") @mock.patch("dbus.service.Object.__init__") @@ -427,4 +490,5 @@ def test_list_images_failed(self, mock_check_output, MockInit, MockBusName, Mock mock_check_output.assert_called_once_with( ["/usr/local/bin/sonic-installer", "list"], stderr=subprocess.STDOUT, - ) \ No newline at end of file + ) + From 56e6115cc33eb3704ac0f8e615b2f3f1554b8c22 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Thu, 23 Jan 2025 16:41:26 +0000 Subject: [PATCH 11/13] add test coverage for missing lines and empty image string. --- tests/host_modules/image_service_test.py | 64 ++++++++++++++++++++---- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/tests/host_modules/image_service_test.py b/tests/host_modules/image_service_test.py index 2f116c96..3d2a88b0 100644 --- a/tests/host_modules/image_service_test.py +++ b/tests/host_modules/image_service_test.py @@ -405,23 +405,65 @@ def test_list_images_success(self, mock_check_output, MockInit, MockBusName, Moc stderr=subprocess.STDOUT, ) + @pytest.mark.parametrize( + "mock_output, expected_current, expected_next, expected_available", + [ + ("Current: \nNext: next_image\nAvailable:\nimage1\nimage2\n", "", "next_image", ["image1", "image2"]), + ("Current: current_image\nNext: \nAvailable:\nimage1\nimage2\n", "current_image", "", ["image1", "image2"]), + ("Current: current_image\nNext: next_image\nAvailable:\n", "current_image", "next_image", []), + ], + ) @mock.patch("dbus.SystemBus") @mock.patch("dbus.service.BusName") @mock.patch("dbus.service.Object.__init__") @mock.patch("subprocess.check_output") - def test_list_images_success_empty_current_image(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + def test_list_images_success_empty_image_output( + self, mock_check_output, MockInit, MockBusName, MockSystemBus, mock_output, expected_current, expected_next, expected_available + ): """ - Test that the `list_images` method successfully lists the current, next, and available SONiC images. + Test that the `list_images` method successfully lists the current, next, and available SONiC images even if + sonic-installer output empty string for the image name. """ # Arrange image_service = ImageService(mod_name="image_service") - mock_output = ( - "Current:\n" - "Next: next_image\n" - "Available:\n" - "image1\n" - "image2\n" + mock_check_output.return_value = mock_output.encode() + + # Act + rc, images_json = image_service.list_images() + images = json.loads(images_json) + + # Assert + assert rc == 0, "wrong return value" + assert images["current"] == expected_current, "current image does not match" + assert images["next"] == expected_next, "next image does not match" + assert images["available"] == expected_available, "available images do not match" + mock_check_output.assert_called_once_with( + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, ) + + @pytest.mark.parametrize( + "mock_output, expected_current, expected_next, expected_available", + [ + ("Next: next_image\nAvailable:\nimage1\nimage2\n", "", "next_image", ["image1", "image2"]), + ("Current: current_image\nAvailable:\nimage1\nimage2\n", "current_image", "", ["image1", "image2"]), + ("Current: current_image\nNext: next_image\n", "current_image", "next_image", []), + ("Available:\nimage1\nimage2\n", "", "", ["image1", "image2"]), + ("Current: current_image\nNext: next_image\nAvailable:\n", "current_image", "next_image", []), + ], + ) + @mock.patch("dbus.SystemBus") + @mock.patch("dbus.service.BusName") + @mock.patch("dbus.service.Object.__init__") + @mock.patch("subprocess.check_output") + def test_list_images_various_missing_lines( + self, mock_check_output, MockInit, MockBusName, MockSystemBus, mock_output, expected_current, expected_next, expected_available + ): + """ + Test that the `list_images` method handles various scenarios where the sonic-installer output is missing lines for current, next, or available images. + """ + # Arrange + image_service = ImageService(mod_name="image_service") mock_check_output.return_value = mock_output.encode() # Act @@ -430,9 +472,9 @@ def test_list_images_success_empty_current_image(self, mock_check_output, MockIn # Assert assert rc == 0, "wrong return value" - assert images["current"] == "", "current image should be empty" - assert images["next"] == "next_image", "next image does not match" - assert images["available"] == ["image1", "image2"], "available images do not match" + assert images["current"] == expected_current, "current image does not match" + assert images["next"] == expected_next, "next image does not match" + assert images["available"] == expected_available, "available images do not match" mock_check_output.assert_called_once_with( ["/usr/local/bin/sonic-installer", "list"], stderr=subprocess.STDOUT, From 02b402c1052db3f115d43fe9fd29bef6725165ab Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Thu, 23 Jan 2025 16:50:04 +0000 Subject: [PATCH 12/13] add a testcase for lowercase output. --- tests/host_modules/image_service_test.py | 34 ++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/host_modules/image_service_test.py b/tests/host_modules/image_service_test.py index 3d2a88b0..bc941778 100644 --- a/tests/host_modules/image_service_test.py +++ b/tests/host_modules/image_service_test.py @@ -405,6 +405,40 @@ def test_list_images_success(self, mock_check_output, MockInit, MockBusName, Moc stderr=subprocess.STDOUT, ) + @mock.patch("dbus.SystemBus") + @mock.patch("dbus.service.BusName") + @mock.patch("dbus.service.Object.__init__") + @mock.patch("subprocess.check_output") + def test_list_images_success_lowercase_output(self, mock_check_output, MockInit, MockBusName, MockSystemBus): + """ + Test that the `list_images` method successfully lists the current, next, and available SONiC images + even if the output from sonic-installer is in lowercase. + """ + # Arrange + image_service = ImageService(mod_name="image_service") + mock_output = ( + "current: current_image\n" + "next: next_image\n" + "available:\n" + "image1\n" + "image2\n" + ) + mock_check_output.return_value = mock_output.encode() + + # Act + rc, images_json = image_service.list_images() + images = json.loads(images_json) + + # Assert + assert rc == 0, "wrong return value" + assert images["current"] == "current_image", "current image does not match" + assert images["next"] == "next_image", "next image does not match" + assert images["available"] == ["image1", "image2"], "available images do not match" + mock_check_output.assert_called_once_with( + ["/usr/local/bin/sonic-installer", "list"], + stderr=subprocess.STDOUT, + ) + @pytest.mark.parametrize( "mock_output, expected_current, expected_next, expected_available", [ From 1ceda79f9e54ab1ec07b717225dbc5c612cbe2b1 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Thu, 23 Jan 2025 16:55:34 +0000 Subject: [PATCH 13/13] more logs for parsing output from sonic-installer. --- host_modules/image_service.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/host_modules/image_service.py b/host_modules/image_service.py index 924ee21e..c52c6de9 100644 --- a/host_modules/image_service.py +++ b/host_modules/image_service.py @@ -188,6 +188,9 @@ def _parse_sonic_installer_list(self, output): else: available_images.append(line.strip()) + logger.info("Current image: {}".format(current_image)) + logger.info("Next image: {}".format(next_image)) + logger.info("Available images: {}".format(available_images)) return { "current": current_image or "", "next": next_image or "",