Skip to content

Restart chrony/ntp service on interfaces-config restart - #69

Open
anamehra wants to merge 3 commits into
sonic-net:masterfrom
anamehra:anamehra/chrony_ztp_restart
Open

Restart chrony/ntp service on interfaces-config restart#69
anamehra wants to merge 3 commits into
sonic-net:masterfrom
anamehra:anamehra/chrony_ztp_restart

Conversation

@anamehra

@anamehra anamehra commented Nov 13, 2025

Copy link
Copy Markdown

Restart NTP service when interfaces-config restarts

Problem

When ZTP restarts the interfaces-config service (during link scan or periodic restarts), it causes issues with the NTP service if management VRF is configured.

Why this happens:

  • When management VRF is set up, NTP needs to bind to the correct VRF context
  • Restarting interfaces-config changes the network namespace and VRF configuration
  • If NTP is not restarted after this, it stays bound to the old network context and stops working
  • This breaks time synchronization during the ZTP provisioning process

Solution

Added logic to detect which NTP service is running (chrony or ntp) and restart it around the interfaces-config restart:

  1. Stop the NTP service (releases the old VRF binding)
  2. Restart interfaces-config (applies new network/VRF config)
  3. Start the NTP service (rebinds to the correct VRF context)

This ensures NTP continues working correctly in the management VRF after network reconfigurations.

Changes

  • ztp-engine.py: Added get_ntp_service_name() function to detect which service is available, and __restart_network_services() method to handle the restart sequence
  • ztp-profile.sh: Added get_ntp_service() function and updated install/remove sections to stop/start NTP around interfaces-config restarts

Why detect the service?

Different SONiC releases use different NTP implementations (ntp vs chrony), so we check which one is actually running instead of hardcoding it. The code checks in priority order: chrony first, then ntp.

Signed-off-by: Anand Mehra (anamehra) <anamehra@cisco.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@anamehra
anamehra marked this pull request as ready for review December 22, 2025 06:27
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

'''
ntp_service = get_ntp_service_name()

if ntp_service:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should this behavior be handled by interfaces-config or systemd service dependency instead of ZTP? ZTP should ideally not manage unrelated service lifecycle.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 840e0de. I kept this scoped to ZTP because this is the code path explicitly restarting interfaces-config during discovery/resume/remove. chrony.service already has After=interfaces-config.service for startup ordering, but that does not restart/rebind chrony when ZTP later runs systemctl restart interfaces-config. Adding reverse ordering/dependencies from interfaces-config back to NTP/chrony risks an ordering cycle, so I did not alter the systemd unit dependencies in this PR.

Comment thread src/usr/lib/ztp/ztp-engine.py Outdated
runCommand('systemctl stop ' + ntp_service, capture_stdout=False)

logger.info('Restarting interfaces-config service...')
runCommand('systemctl restart interfaces-config', capture_stdout=False)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If interfaces-config restart fails, will the NTP service be restarted? Consider using finally/cleanup handling to avoid leaving NTP stopped.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 840e0de. The Python path now wraps the interfaces-config restart in try/finally so the active NTP service is started again even if an unexpected exception occurs, and it logs a nonzero interfaces-config return code. The shell path now uses a shared restart_interfaces_config_with_ntp helper that captures the restart rc, starts NTP afterward, and returns the original rc.

Comment thread src/usr/lib/ztp/ztp-engine.py Outdated
ntp_services = ['chrony', 'ntp']
for service in ntp_services:
try:
rc = runCommand('systemctl is-enabled ' + service, capture_stdout=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we check the active service instead of the enabled service here? A service can be enabled but not running, so checking active status may better reflect the NTP service currently in use

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 840e0de. Detection now uses systemctl is-active --quiet for chrony/ntp in both ztp-engine.py and ztp-profile.sh, so only the service that was actually running before the interfaces-config restart is stopped and started again.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Signed-off-by: Anand Mehra (anamehra) <anamehra@cisco.com>
@anamehra
anamehra force-pushed the anamehra/chrony_ztp_restart branch from 840e0de to 024b2ba Compare July 20, 2026 16:35
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@anamehra
anamehra requested a review from lucas-li-micas July 22, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants