Revert "[featured] fix non existing feature start (#234)" (#272)

This reverts commit 29b8be8.

Because it blocks submodule update in sonic-buildimage repo.
The build and test are verified in a draft PR sonic-net/sonic-buildimage#23027.

Author: qiluo-msft

scripts/featured

@@ -274,13 +274,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
@@ -415,7 +413,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:
@@ -435,17 +433,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):
@@ -460,10 +457,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)

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'