Skip to content

[202405] Fix race condition between featured daemon-reload and docker container start on chassis #388

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
arista-setu wants to merge 1 commit into
base: 202405
Choose a base branch
from
+28 −11
Open

[202405] Fix race condition between featured daemon-reload and docker container start on chassis #388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 26 additions & 2 deletions scripts/featured
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,14 @@ class FeatureHandler(object):
Summary:
Updates the state field in the FEATURE|* tables as the state field
might have to be rendered based on DEVICE_METADATA table and generated Device Running Metadata

Uses a two-phase approach to avoid a race condition between systemctl
daemon-reload and Docker container startup:
Phase 1: Write all systemd config files, followed by daemon-reload.
Phase 2: Start/stop all feature services (no daemon-reloads required).
"""
# Phase 1: Write all systemd configs and cache features
syslog.syslog(syslog.LOG_INFO, "Writing systemd configs")
for feature_name in feature_table.keys():
if not feature_name:
syslog.syslog(syslog.LOG_WARNING, "Feature is None")
Expand All @@ -224,7 +231,18 @@ class FeatureHandler(object):
feature = Feature(feature_name, feature_table[feature_name], device_config)

self._cached_config.setdefault(feature_name, feature)
self.update_systemd_config(feature)
self.update_systemd_config(feature, reload=False)

# Perform daemon-reload after all configs are written
self.reload_systemd_config()

# Phase 2: Start/stop services without daemon-reloads
syslog.syslog(syslog.LOG_INFO, "Restarting services")
for feature_name in feature_table.keys():
if not feature_name:
syslog.syslog(syslog.LOG_WARNING, "Feature is None")
continue
feature = self._cached_config[feature_name]
self.update_feature_state(feature)
self.sync_feature_scope(feature)
self.resync_feature_state(feature)
Expand Down Expand Up @@ -318,13 +336,15 @@ class FeatureHandler(object):
db.mod_entry(FEATURE_TBL, feature_config.name, {'has_per_asic_scope': str(feature_config.has_per_asic_scope)})
db.mod_entry(FEATURE_TBL, feature_config.name, {'has_global_scope': str(feature_config.has_global_scope)})

def update_systemd_config(self, feature_config):
def update_systemd_config(self, feature_config, reload=True):
"""Updates `Restart=` field in feature's systemd configuration file
according to the value of `auto_restart` field in `FEATURE` table of `CONFIG_DB`.

Args:
feature: An object represents a feature's configuration in `FEATURE`
table of `CONFIG_DB`.
reload: Whether to run systemctl daemon-reload after writing config.
Set to False when batching multiple config updates.

Returns:
None.
Expand Down Expand Up @@ -362,6 +382,10 @@ class FeatureHandler(object):
syslog.syslog(syslog.LOG_INFO, "Feature '{}' systemd config file related to auto-restart is updated!"
.format(feature_name))

if reload:
self.reload_systemd_config()

def reload_systemd_config(self):
try:
syslog.syslog(syslog.LOG_INFO, "Reloading systemd configuration files ...")
run_cmd(["sudo", "systemctl", "daemon-reload"], raise_exception=True)
Expand Down
11 changes: 2 additions & 9 deletions tests/featured/featured_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,6 @@ def test_feature_events(self, mock_syslog, get_runtime):
call(['sudo', 'systemctl', 'unmask', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'daemon-reload'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'unmask', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'mux.service'], capture_output=True, check=True, text=True)]
Expand Down Expand Up @@ -421,11 +420,9 @@ def test_delayed_service(self, mock_syslog, get_runtime):
call(['sudo', 'systemctl', 'unmask', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'daemon-reload'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'unmask', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'daemon-reload'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'unmask', 'telemetry.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'telemetry.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'telemetry.service'], capture_output=True, check=True, text=True)]
Expand All @@ -451,15 +448,13 @@ def test_advanced_reboot(self, mock_syslog, get_runtime):
call(['sudo', 'systemctl', 'unmask', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'daemon-reload'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'unmask', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'daemon-reload'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'unmask', 'telemetry.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'telemetry.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'telemetry.service'], capture_output=True, check=True, text=True)]
call(['sudo', 'systemctl', 'start', 'telemetry.service'], capture_output=True, check=True, text=True)]

mocked_subprocess.run.assert_has_calls(expected, any_order=True)

def test_portinit_timeout(self, mock_syslog, get_runtime):
Expand All @@ -484,11 +479,9 @@ def test_portinit_timeout(self, mock_syslog, get_runtime):
call(['sudo', 'systemctl', 'unmask', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'dhcp_relay.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'daemon-reload'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'unmask', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'mux.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'daemon-reload'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'unmask', 'telemetry.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'enable', 'telemetry.service'], capture_output=True, check=True, text=True),
call(['sudo', 'systemctl', 'start', 'telemetry.service'], capture_output=True, check=True, text=True)]
Expand Down
Loading