[action] [PR:4611] [BMC] Improvements in show chassis/platform leak_profile commands

config/chassis_modules.py

@@ -252,6 +252,8 @@ def set_power_on_delay(db, chassis_module_name, seconds):
         config_db = db.cfgdb
         fvs = config_db.get_entry('CHASSIS_MODULE', chassis_module_name) or {}
         fvs['power_on_delay'] = str(seconds)
+        # Seed admin_status of SWITCH-HOST with default if not already set
+        fvs.setdefault('admin_status', 'down')
         config_db.set_entry('CHASSIS_MODULE', chassis_module_name, fvs)
         click.echo(f"Power-on-delay for {chassis_module_name} set to {seconds} seconds")
 
@@ -272,5 +274,7 @@ def set_graceful_shutdown_timeout(db, chassis_module_name, seconds):
         config_db = db.cfgdb
         fvs = config_db.get_entry('CHASSIS_MODULE', chassis_module_name) or {}
         fvs['graceful_shutdown_timeout'] = str(seconds)
+        # Seed admin_status of SWITCH-HOST with default if not already set
+        fvs.setdefault('admin_status', 'down')
         config_db.set_entry('CHASSIS_MODULE', chassis_module_name, fvs)
         click.echo(f"Shutdown-timeout for {chassis_module_name} set to {seconds} seconds")

show/chassis_modules.py

@@ -48,9 +48,16 @@ def status(db, chassis_module_name):
     """Show chassis-modules status"""
 
     smartswitch = is_smartswitch()
+    bmc = is_bmc()
     header = ['Name', 'Description', 'Physical-Slot', 'Oper-Status', 'Admin-Status', 'Serial']
     if smartswitch:
         header.append('Ready-Status')
+    if bmc:
+        # Physical-Slot is not meaningful on BMC; drop it and add the
+        # BMC-only timing fields configured via 'config chassis modules
+        # power-on-delay' / 'shutdown-timeout' for SWITCH-HOST modules.
+        header.remove('Physical-Slot')
+        header.extend(['Power-On-Delay (sec)', 'Shutdown-Timeout (sec)'])
 
     chassis_cfg_table = db.cfgdb.get_table('CHASSIS_MODULE')
 
@@ -124,19 +131,36 @@ def status(db, chassis_module_name):
         # Determine admin_status
         if smartswitch:
             admin_status = 'down'
+        elif is_bmc() and key_list[1].startswith("SWITCH-HOST"):
+            # On BMC, SWITCH-HOST default is 'down' (kept powered off on boot)
+            admin_status = 'down'
         else:
             admin_status = 'up'
         config_data = chassis_cfg_table.get(key_list[1])
         if config_data is not None:
             admin_status = config_data.get(CHASSIS_MODULE_INFO_ADMINSTATUS_FIELD, admin_status)
 
         row = [key_list[1], desc, slot, oper_status, admin_status, serial]
+        if bmc:
+            # Physical-Slot column omitted from header on BMC; drop matching value
+            row.pop(2)
 
         if smartswitch:
             dpu_info = dpu_state_data.get(key_list[1], {})
             ready_status = dpu_info.get(DPU_STATE_READY_STATUS_FIELD, 'N/A')
             row.append(ready_status)
 
+        if bmc:
+            # Only meaningful for SWITCH-HOST modules; other module types show N/A.
+            if key_list[1].startswith("SWITCH-HOST"):
+                cfg = config_data or {}
+                power_on_delay = cfg.get('power_on_delay', '0')
+                shutdown_timeout = cfg.get('graceful_shutdown_timeout', '120')
+            else:
+                power_on_delay = 'N/A'
+                shutdown_timeout = 'N/A'
+            row.extend([power_on_delay, shutdown_timeout])
+
         table.append(tuple(row))
 
     if chassis_state_db:

show/platform.py

@@ -381,14 +381,14 @@ def leak_rack_manager_alerts():
 def leak_profiles():
     """Show leak sensor profiles"""
     try:
-        from utilities_common.db import Db
-        db = Db()
-        keys = db.cfgdb.get_keys(LEAK_PROFILE_TABLE) or []
+        state_db = _get_state_db()
+        keys = state_db.keys(state_db.STATE_DB, f"{LEAK_PROFILE_TABLE}|*") or []
         header = ['Sensor-Type', 'Max-Minor-Duration-Sec']
         rows = []
-        for sensor_type in sorted(keys):
-            entry = db.cfgdb.get_entry(LEAK_PROFILE_TABLE, sensor_type)
-            max_dur = entry.get('max_minor_duration_sec', 'N/A')
+        for key in sorted(keys):
+            sensor_type = key.split('|', 1)[1]
+            data = state_db.get_all(state_db.STATE_DB, key) or {}
+            max_dur = data.get('max_minor_duration_sec', 'N/A')
             rows.append((sensor_type, max_dur))
         if rows:
             click.echo(tabulate(rows, header, tablefmt='simple'))

tests/chassis_modules_test.py

@@ -708,6 +708,69 @@ def test_both_fields_preserved_in_db(self):
         assert entry.get("power_on_delay") == "60"
         assert entry.get("graceful_shutdown_timeout") == "30"
 
+    def test_power_on_delay_seeds_admin_status_when_missing(self):
+        """First-time 'power-on-delay' on a fresh DB must seed admin_status='down'
+        so the entry stays valid for downstream consumers (startup/shutdown)."""
+        runner = CliRunner()
+        db = Db()
+        # Sanity: no entry exists
+        assert db.cfgdb.get_entry("CHASSIS_MODULE", "SWITCH-HOST") == {}
+        result = runner.invoke(
+            self.modules.commands["power-on-delay"],
+            ["SWITCH-HOST", "300"],
+            obj=db
+        )
+        assert result.exit_code == 0
+        entry = db.cfgdb.get_entry("CHASSIS_MODULE", "SWITCH-HOST")
+        assert entry.get("power_on_delay") == "300"
+        assert entry.get("admin_status") == "down"
+
+    def test_power_on_delay_does_not_overwrite_existing_admin_status(self):
+        """If admin_status is already configured (e.g. 'up'), 'power-on-delay'
+        must NOT downgrade it back to 'down'."""
+        runner = CliRunner()
+        db = Db()
+        db.cfgdb.mod_entry('CHASSIS_MODULE', 'SWITCH-HOST', {'admin_status': 'up'})
+        result = runner.invoke(
+            self.modules.commands["power-on-delay"],
+            ["SWITCH-HOST", "300"],
+            obj=db
+        )
+        assert result.exit_code == 0
+        entry = db.cfgdb.get_entry("CHASSIS_MODULE", "SWITCH-HOST")
+        assert entry.get("power_on_delay") == "300"
+        assert entry.get("admin_status") == "up", "existing admin_status must be preserved"
+
+    def test_shutdown_timeout_seeds_admin_status_when_missing(self):
+        """First-time 'shutdown-timeout' on a fresh DB must seed admin_status='down'."""
+        runner = CliRunner()
+        db = Db()
+        assert db.cfgdb.get_entry("CHASSIS_MODULE", "SWITCH-HOST") == {}
+        result = runner.invoke(
+            self.modules.commands["shutdown-timeout"],
+            ["SWITCH-HOST", "120"],
+            obj=db
+        )
+        assert result.exit_code == 0
+        entry = db.cfgdb.get_entry("CHASSIS_MODULE", "SWITCH-HOST")
+        assert entry.get("graceful_shutdown_timeout") == "120"
+        assert entry.get("admin_status") == "down"
+
+    def test_shutdown_timeout_does_not_overwrite_existing_admin_status(self):
+        """If admin_status='up' already, 'shutdown-timeout' must not downgrade it."""
+        runner = CliRunner()
+        db = Db()
+        db.cfgdb.mod_entry('CHASSIS_MODULE', 'SWITCH-HOST', {'admin_status': 'up'})
+        result = runner.invoke(
+            self.modules.commands["shutdown-timeout"],
+            ["SWITCH-HOST", "120"],
+            obj=db
+        )
+        assert result.exit_code == 0
+        entry = db.cfgdb.get_entry("CHASSIS_MODULE", "SWITCH-HOST")
+        assert entry.get("graceful_shutdown_timeout") == "120"
+        assert entry.get("admin_status") == "up", "existing admin_status must be preserved"
+
     @classmethod
     def teardown_class(cls):
         print("TEARDOWN")
@@ -864,6 +927,100 @@ def test_show_status_bmc_module_helper_init_failure_falls_back_to_db(self):
         assert result.exit_code == 0
         assert "Empty" in result.output
 
+    def test_show_status_bmc_header_drops_physical_slot_adds_timing_columns(self):
+        """On BMC, Physical-Slot column is removed and Power-On-Delay /
+        Shutdown-Timeout columns are appended."""
+        runner = CliRunner()
+        with mock.patch('show.chassis_modules.is_bmc', return_value=True), \
+             mock.patch('show.chassis_modules.ModuleHelper', side_effect=Exception("no chassis")):
+            result = runner.invoke(
+                show.cli.commands["chassis"].commands["modules"].commands["status"],
+                ["LINE-CARD0"]
+            )
+        print(result.output)
+        assert result.exit_code == 0
+        assert "Physical-Slot" not in result.output
+        assert "Power-On-Delay" in result.output
+        assert "Shutdown-Timeout" in result.output
+
+    def test_show_status_bmc_non_switch_host_shows_na_for_timing(self):
+        """On BMC, non-SWITCH-HOST rows display N/A for the timing columns."""
+        runner = CliRunner()
+        with mock.patch('show.chassis_modules.is_bmc', return_value=True), \
+             mock.patch('show.chassis_modules.ModuleHelper', side_effect=Exception("no chassis")):
+            result = runner.invoke(
+                show.cli.commands["chassis"].commands["modules"].commands["status"],
+                ["LINE-CARD0"]
+            )
+        print(result.output)
+        assert result.exit_code == 0
+        assert "N/A" in result.output
+
+    def test_show_status_non_bmc_output_unchanged(self):
+        """Regression: non-BMC output must still contain Physical-Slot and
+        must NOT contain the new BMC-only timing columns."""
+        runner = CliRunner()
+        with mock.patch('show.chassis_modules.is_bmc', return_value=False):
+            result = runner.invoke(
+                show.cli.commands["chassis"].commands["modules"].commands["status"],
+                []
+            )
+        print(result.output)
+        assert result.exit_code == 0
+        assert "Physical-Slot" in result.output
+        assert "Power-On-Delay" not in result.output
+        assert "Shutdown-Timeout" not in result.output
+
+    def test_show_status_bmc_switch_host_shows_configured_timing_values(self):
+        """On BMC, SWITCH-HOST row shows the configured power-on-delay and
+        graceful-shutdown-timeout values from CONFIG_DB."""
+        runner = CliRunner()
+        db = Db()
+        db.cfgdb.mod_entry('CHASSIS_MODULE', 'SWITCH-HOST', {
+            'admin_status': 'up',
+            'power_on_delay': '300',
+            'graceful_shutdown_timeout': '90',
+        })
+
+        # Mock state_db to return a SWITCH-HOST entry (mock_tables doesn't have one)
+        switch_host_state = {
+            'desc': 'Switch Host',
+            'slot': '1',
+            'oper_status': 'Online',
+            'serial': 'SH1000101',
+        }
+
+        def fake_keys(_db_id, pattern):
+            if 'SWITCH-HOST' in pattern:
+                return ['CHASSIS_MODULE_TABLE|SWITCH-HOST']
+            return []
+
+        def fake_get_all(_db_id, key):
+            if key.endswith('|SWITCH-HOST'):
+                return switch_host_state
+            return {}
+
+        with mock.patch('show.chassis_modules.is_bmc', return_value=True), \
+             mock.patch('show.chassis_modules.ModuleHelper', side_effect=Exception("no chassis")), \
+             mock.patch('show.chassis_modules.SonicV2Connector') as mock_conn_cls:
+            mock_conn = mock.MagicMock()
+            mock_conn.STATE_DB = 'STATE_DB'
+            mock_conn.keys.side_effect = fake_keys
+            mock_conn.get_all.side_effect = fake_get_all
+            mock_conn_cls.return_value = mock_conn
+
+            result = runner.invoke(
+                show.cli.commands["chassis"].commands["modules"].commands["status"],
+                ["SWITCH-HOST"],
+                obj=db
+            )
+        print(result.output)
+        assert result.exit_code == 0
+        assert "SWITCH-HOST" in result.output
+        assert "300" in result.output
+        assert "90" in result.output
+        assert "Physical-Slot" not in result.output
+
     @classmethod
     def teardown_class(cls):
         print("TEARDOWN")

tests/show_platform_leak_test.py

@@ -130,17 +130,13 @@ def setup_class(cls):
 
     def test_leak_profiles_with_data(self):
         runner = CliRunner()
-        mock_cfgdb = MagicMock()
-        mock_cfgdb.get_keys.return_value = ['rope', 'spot', 'flex_pcb']
-        mock_cfgdb.get_entry.side_effect = lambda t, k: {
-            'rope':     {'max_minor_duration_sec': '300'},
-            'spot':     {'max_minor_duration_sec': '600'},
-            'flex_pcb': {'max_minor_duration_sec': '180'},
-        }[k]
-        mock_db = MagicMock()
-        mock_db.cfgdb = mock_cfgdb
+        state_db = _make_state_db({
+            'LEAK_PROFILE|rope':     {'type': 'rope', 'max_minor_duration_sec': '300'},
+            'LEAK_PROFILE|spot':     {'type': 'spot', 'max_minor_duration_sec': '600'},
+            'LEAK_PROFILE|flex_pcb': {'type': 'flex_pcb', 'max_minor_duration_sec': '180'},
+        })
 
-        with patch('utilities_common.db.Db', return_value=mock_db):
+        with patch('show.platform._get_state_db', return_value=state_db):
             result = runner.invoke(
                 show_platform.platform.commands['leak'].commands['profiles']
             )
@@ -153,12 +149,9 @@ def test_leak_profiles_with_data(self):
 
     def test_leak_profiles_empty(self):
         runner = CliRunner()
-        mock_cfgdb = MagicMock()
-        mock_cfgdb.get_keys.return_value = []
-        mock_db = MagicMock()
-        mock_db.cfgdb = mock_cfgdb
+        state_db = _make_state_db({})
 
-        with patch('utilities_common.db.Db', return_value=mock_db):
+        with patch('show.platform._get_state_db', return_value=state_db):
             result = runner.invoke(
                 show_platform.platform.commands['leak'].commands['profiles']
             )