From d5f57168a61c28d1d3add1701e53cafada41f2e6 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Sat, 21 Jun 2025 02:01:05 +0000 Subject: [PATCH] Revert "[featured] fix non existing feature start (#234)" This reverts commit 29b8be81fd4f468ff8482df37b372dafe3c07b3a. Because it blocks submodule update in sonic-buildimage repo. The build and test are verified in a draft PR https://github.com/sonic-net/sonic-buildimage/pull/23027. --- scripts/featured | 16 ++++------- tests/featured/featured_test.py | 50 --------------------------------- 2 files changed, 6 insertions(+), 60 deletions(-) diff --git a/scripts/featured b/scripts/featured index 6d48700d..a3c91c28 100644 --- a/scripts/featured +++ b/scripts/featured @@ -266,13 +266,11 @@ class FeatureHandler(object): return True if enable: - if not self.enable_feature(feature): - return False + self.enable_feature(feature) syslog.syslog(syslog.LOG_INFO, "Feature {} is enabled and started".format(feature.name)) if disable: - if not self.disable_feature(feature): - return False + self.disable_feature(feature) syslog.syslog(syslog.LOG_INFO, "Feature {} is stopped and disabled".format(feature.name)) return True @@ -407,7 +405,7 @@ class FeatureHandler(object): for feature_name in feature_names: # Check if it is already enabled, if yes skip the system call unit_file_state = self.get_systemd_unit_state("{}.{}".format(feature_name, feature_suffixes[-1])) - if unit_file_state == "enabled": + if unit_file_state == "enabled" or not unit_file_state: continue cmds = [] for suffix in feature_suffixes: @@ -427,17 +425,16 @@ class FeatureHandler(object): syslog.syslog(syslog.LOG_ERR, "Feature '{}.{}' failed to be enabled and started" .format(feature.name, feature_suffixes[-1])) self.set_feature_state(feature, self.FEATURE_STATE_FAILED) - return False + return self.set_feature_state(feature, self.FEATURE_STATE_ENABLED) - return True def disable_feature(self, feature): feature_names, feature_suffixes = self.get_multiasic_feature_instances(feature) for feature_name in feature_names: # Check if it is already disabled, if yes skip the system call unit_file_state = self.get_systemd_unit_state("{}.{}".format(feature_name, feature_suffixes[-1])) - if unit_file_state in ("disabled", "masked"): + if unit_file_state in ("disabled", "masked") or not unit_file_state: continue cmds = [] for suffix in reversed(feature_suffixes): @@ -452,10 +449,9 @@ class FeatureHandler(object): syslog.syslog(syslog.LOG_ERR, "Feature '{}.{}' failed to be stopped and disabled" .format(feature.name, feature_suffixes[-1])) self.set_feature_state(feature, self.FEATURE_STATE_FAILED) - return False + return self.set_feature_state(feature, self.FEATURE_STATE_DISABLED) - return True def resync_feature_state(self, feature): current_entry = self._config_db.get_entry('FEATURE', feature.name) diff --git a/tests/featured/featured_test.py b/tests/featured/featured_test.py index d893f68a..a42d2bb8 100644 --- a/tests/featured/featured_test.py +++ b/tests/featured/featured_test.py @@ -493,53 +493,3 @@ def test_portinit_timeout(self, mock_syslog, get_runtime): 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)] mocked_subprocess.run.assert_has_calls(expected, any_order=True) - - def test_systemctl_command_failure(self, mock_syslog, get_runtime): - """Test that when systemctl commands fail: - 1. The feature state is not cached - 2. The feature state is set to FAILED - 3. The update_feature_state returns False - """ - mock_db = mock.MagicMock() - mock_feature_state_table = mock.MagicMock() - - feature_handler = featured.FeatureHandler(mock_db, mock_feature_state_table, {}, False) - feature_handler.is_delayed_enabled = True - - # Create a feature that should be enabled - feature_name = 'test_feature' - feature_cfg = { - 'state': 'enabled', - 'auto_restart': 'enabled', - 'delayed': 'False', - 'has_global_scope': 'True', - 'has_per_asic_scope': 'False' - } - - # Initialize the feature in cached_config using the same pattern as in featured - feature = featured.Feature(feature_name, feature_cfg) - feature_handler._cached_config.setdefault(feature_name, featured.Feature(feature_name, {})) - - # Mock subprocess.run and Popen to simulate command failure - with mock.patch('featured.subprocess') as mocked_subprocess: - # Mock Popen for get_systemd_unit_state - popen_mock = mock.Mock() - popen_mock.communicate.return_value = ('enabled', '') - popen_mock.returncode = 1 - mocked_subprocess.Popen.return_value = popen_mock - - # Mock run_cmd to raise an exception - with mock.patch('featured.run_cmd') as mocked_run_cmd: - mocked_run_cmd.side_effect = Exception("Command failed") - - # Try to update feature state - result = feature_handler.update_feature_state(feature) - - # Verify the result is False - assert result is False - - # Verify the feature state was set to FAILED - mock_feature_state_table.set.assert_called_with('test_feature', [('state', 'failed')]) - - # Verify the feature state was not enabled in the cache - assert feature_handler._cached_config[feature.name].state != 'enabled'