Skip to content

[reload] stop timers on reload #4021

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.

Sign up for GitHub

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 2 commits into
base: master
Choose a base branch
from
Open

[reload] stop timers on reload #4021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a44ba65
[reload] stop timers on reload
stepanblyschak Aug 7, 2025
9a7fd1e
Update config_test.py
stepanblyschak Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,12 @@ def _stop_services():
except subprocess.CalledProcessError as err:
pass

# Get the list of dependencies for sonic.target to fetch timer units
for service in _get_sonic_services():
if service.endswith('.timer'):
Copy link
Contributor

Choose a reason for hiding this comment

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

service

Just curios, these services are already dependencies of sonic.target. Why original code does not stop the them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's one of the timers:

# /lib/systemd/system/hostcfgd.timer
[Unit]
Description=Delays hostcfgd daemon until SONiC has started
PartOf=hostcfgd.service

[Timer]
OnUnitActiveSec=0 sec
OnBootSec=1min 30 sec
Unit=hostcfgd.service

[Install]
WantedBy=timers.target sonic.target

The relation between sonic.target and the timer is only WantedBy, meaning start of sonic.target starts the timer but does not stop it

# Stop the timer unit
clicommon.run_command(['sudo', 'systemctl', 'stop', service], display_cmd=True)

click.echo("Stopping SONiC target ...")
clicommon.run_command(['sudo', 'systemctl', 'stop', 'sonic.target', '--job-mode', 'replace-irreversibly'])

Expand Down
17 changes: 13 additions & 4 deletions tests/config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
load_minigraph_command_output="""\
Acquired lock on {0}
Disabling container and routeCheck monitoring ...
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -H -m --write-to-db
Running command: config qos reload --no-dynamic-buffer --no-delay
Expand All @@ -70,6 +71,7 @@

load_minigraph_command_bypass_lock_output = """\
Bypass lock on {}
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -H -m --write-to-db
Running command: config qos reload --no-dynamic-buffer --no-delay
Expand All @@ -81,6 +83,7 @@

load_minigraph_platform_plugin_command_output="""\
Acquired lock on {0}
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -H -m --write-to-db
Running command: config qos reload --no-dynamic-buffer --no-delay
Expand Down Expand Up @@ -165,6 +168,7 @@

RELOAD_CONFIG_DB_OUTPUT = """\
Acquired lock on {0}
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json --write-to-db
Restarting SONiC target ...
Expand All @@ -178,6 +182,7 @@

RELOAD_CONFIG_DB_BYPASS_LOCK_OUTPUT = """\
Bypass lock on {0}
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json --write-to-db
Restarting SONiC target ...
Expand All @@ -186,6 +191,7 @@

RELOAD_YANG_CFG_OUTPUT = """\
Acquired lock on {0}
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -Y /tmp/config.json --write-to-db
Restarting SONiC target ...
Expand All @@ -195,6 +201,7 @@

RELOAD_MASIC_CONFIG_DB_OUTPUT = """\
Acquired lock on {0}
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json --write-to-db
Running command: /usr/local/bin/sonic-cfggen -j /tmp/config0.json -n asic0 --write-to-db
Expand All @@ -219,6 +226,7 @@

reload_config_masic_onefile_output = """\
Acquired lock on {0}
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Restarting SONiC target ...
Reloading Monit configuration ...
Expand All @@ -227,6 +235,7 @@

reload_config_masic_onefile_gen_sysinfo_output = """\
Acquired lock on {0}
Running command: sudo systemctl stop featured.timer
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -H -k Mellanox-SN3800-D112C8 --write-to-db
Running command: /usr/local/bin/sonic-cfggen -H -k multi_asic -n asic0 --write-to-db
Expand Down Expand Up @@ -307,7 +316,7 @@ def mock_run_command_side_effect(*args, **kwargs):
if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
return 'snmp.timer', 0
elif command == "systemctl list-dependencies --plain sonic.target":
return 'sonic.target\nswss', 0
return 'sonic.target\nswss\nfeatured.timer', 0
elif command == "systemctl is-enabled snmp.timer":
return 'enabled', 0
elif command == 'cat /var/run/dhclient.eth0.pid':
Expand Down Expand Up @@ -1058,7 +1067,7 @@ def test_load_minigraph(self, mock_check_call, get_cmd_module, setup_single_broa
(load_minigraph_command_output.format(config.SYSTEM_RELOAD_LOCK))
# Verify "systemctl reset-failed" is called for services under sonic.target
mock_run_command.assert_any_call(['systemctl', 'reset-failed', 'swss'])
assert mock_run_command.call_count == 16
assert mock_run_command.call_count == 19

@mock.patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs',
mock.MagicMock(return_value=("dummy_path", None)))
Expand Down Expand Up @@ -1102,7 +1111,7 @@ def test_load_minigraph_bypass_lock(self, get_cmd_module, setup_single_broadcom_
assert result.exit_code == 0
assert result.output == \
load_minigraph_command_bypass_lock_output.format(config.SYSTEM_RELOAD_LOCK)
assert mock_run_command.call_count == 12
assert mock_run_command.call_count == 15
finally:
flock.release_flock(fd)

Expand All @@ -1120,7 +1129,7 @@ def test_load_minigraph_platform_plugin(self, get_cmd_module, setup_single_broad
(load_minigraph_platform_plugin_command_output.format(config.SYSTEM_RELOAD_LOCK))
# Verify "systemctl reset-failed" is called for services under sonic.target
mock_run_command.assert_any_call(['systemctl', 'reset-failed', 'swss'])
assert mock_run_command.call_count == 12
assert mock_run_command.call_count == 15

@mock.patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', mock.MagicMock(return_value=(load_minigraph_platform_false_path, None)))
def test_load_minigraph_platform_plugin_fail(self, get_cmd_module, setup_single_broadcom_asic):
Expand Down
Loading