diff --git a/scripts/featured b/scripts/featured index a3c91c28..169267d4 100644 --- a/scripts/featured +++ b/scripts/featured @@ -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") @@ -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) @@ -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. @@ -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) diff --git a/tests/featured/featured_test.py b/tests/featured/featured_test.py index a42d2bb8..ef07b1c1 100644 --- a/tests/featured/featured_test.py +++ b/tests/featured/featured_test.py @@ -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)] @@ -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)] @@ -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): @@ -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)]