Skip to content

[Smartswitch] Fix the values being set for reboot_status call during smartswitch DPU reboot #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gpunathilell
Copy link

During smartswitch DPU reboot we query the rebootstatus call to check the current status. This is currently failing on DPU because the RebootStatus enum is not serializable:

root@sonic:/home/admin# dbus-send --system --print-reply --dest=org.SONiC.HostService.reboot /org/SONiC/HostService/reboot org.SONiC.HostService.reboot.get_reboot_status
Error org.freedesktop.DBus.Python.TypeError: Traceback (most recent call last):
 File "/usr/lib/python3/dist-packages/dbus/service.py", line 712, in _message_cb
   retval = candidate_method(self, *args, **keywords)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/local/lib/python3.11/dist-packages/host_modules/reboot.py", line 218, in get_reboot_status=
   response_data = json.dumps(self.reboot_status_flag)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/json/__init__.py", line 231, in dumps
   return _default_encoder.encode(obj
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/json/encoder.py", line 200, in encode
   chunks = self.iterencode(o, _one_shot=True)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/json/encoder.py", line 258, in iterencode
   return _iterencode(o, 0)
          ^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/json/encoder.py", line 180, in default
   raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type RebootStatus is not JSON serializable

So setting it to value everywhere to fix the issue

Signed-off-by: gpunathilell <[email protected]>
@mssonicbld
Copy link

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also change the test case
test_get_reboot_status_active
and
test_get_reboot_status_inactive

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated test

@vvolam vvolam requested a review from rameshraghupathy July 30, 2025 17:23
@@ -53,7 +53,7 @@ def __init__(self, mod_name):
self.populate_reboot_status_flag()
super(Reboot, self).__init__(mod_name)

def populate_reboot_status_flag(self, active = False, when = 0, reason = "", method = "", status = RebootStatus.STATUS_UNKNOWN):
def populate_reboot_status_flag(self, active = False, when = 0, reason = "", method = "", status = RebootStatus.STATUS_UNKNOWN.value):
Copy link
Contributor

@rameshraghupathy rameshraghupathy Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gpunathilell self.reboot_status_flag can sometimes be None, Can you please handle that case also? Do the same everywhere.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rameshraghupathy self.reboot_status_flag should ideally never be empty. since we are populating it with default values on initialization of reboot class

@mssonicbld
Copy link

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@gpunathilell
Copy link
Author

/azpw run

@mssonicbld
Copy link

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@gpunathilell
Copy link
Author

@vvolam I have changed the output of the status to display the name of the status instead of the value because it is more readable, let me know if it affects anything else and I can revert it

root@sonic:/usr# dbus-send --system --print-reply --dest=org.SONiC.HostService.reboot /org/SONiC/HostService/reboot org.SONiC.HostService.reboot.get_reboot_status
method return time=1754051013.509891 sender=:1.9 -> destination=:1.13 serial=14 reply_serial=2
   int32 0
   string "{"active": false, "when": 0, "reason": "", "count": 0, "method": "", "status": "STATUS_UNKNOWN"}"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants