[sonic-package-manager] remove leftovers from featured on uninstall

sonic_package_manager/service_creator/creator.py

@@ -2,6 +2,7 @@
 
 import contextlib
 import os
+import glob
 import sys
 import shutil
 import stat
@@ -33,6 +34,7 @@
 TIMER_UNIT_TEMPLATE = 'timer.unit.j2'
 
 SYSTEMD_LOCATION = '/usr/lib/systemd/system'
+ETC_SYSTEMD_LOCATION = '/etc/systemd/system'
 
 GENERATED_SERVICES_CONF_FILE = '/etc/sonic/generated_services.conf'
 
@@ -92,18 +94,30 @@ def set_executable_bit(filepath):
     os.chmod(filepath, st.st_mode | stat.S_IEXEC)
 
 
-def remove_if_exists(path):
+def remove_file(path):
     """ Remove filepath if it exists """
 
-    if not os.path.exists(path):
-        return
+    try:
+        os.remove(path)
+        log.info(f'removed {path}')
+    except FileNotFoundError:
+        pass
+
+
+def remove_dir(path):
+    """ Remove filepath if it exists """
+
+    try:
+        shutil.rmtree(path)
+        log.info(f'removed {path}')
+    except FileNotFoundError:
+        pass
 
-    os.remove(path)
-    log.info(f'removed {path}')
 
 def is_list_of_strings(command):
     return isinstance(command, list) and all(isinstance(item, str) for item in command)
 
+
 def run_command(command: List[str]):
     """ Run arbitrary bash command.
     Args:
@@ -197,12 +211,22 @@ def remove(self,
         """
 
         name = package.manifest['service']['name']
-        remove_if_exists(os.path.join(SYSTEMD_LOCATION, f'{name}.service'))
-        remove_if_exists(os.path.join(SYSTEMD_LOCATION, f'{name}@.service'))
-        remove_if_exists(os.path.join(SERVICE_MGMT_SCRIPT_LOCATION, f'{name}.sh'))
-        remove_if_exists(os.path.join(DOCKER_CTL_SCRIPT_LOCATION, f'{name}.sh'))
-        remove_if_exists(os.path.join(DEBUG_DUMP_SCRIPT_LOCATION, f'{name}'))
-        remove_if_exists(os.path.join(ETC_SONIC_PATH, f'{name}_reconcile'))
+        remove_file(os.path.join(SYSTEMD_LOCATION, f'{name}.service'))
+        remove_file(os.path.join(SYSTEMD_LOCATION, f'{name}@.service'))
+        remove_file(os.path.join(SERVICE_MGMT_SCRIPT_LOCATION, f'{name}.sh'))
+        remove_file(os.path.join(DOCKER_CTL_SCRIPT_LOCATION, f'{name}.sh'))
+        remove_file(os.path.join(DEBUG_DUMP_SCRIPT_LOCATION, f'{name}'))
+        remove_file(os.path.join(ETC_SONIC_PATH, f'{name}_reconcile'))
+
+        # remove symlinks and configuration directories created by featured
+        remove_file(os.path.join(ETC_SYSTEMD_LOCATION, f'{name}.service'))
+        for unit_file in glob.glob(os.path.join(ETC_SYSTEMD_LOCATION, f'{name}@*.service')):
+            remove_file(unit_file)
+
+        remove_dir(os.path.join(ETC_SYSTEMD_LOCATION, f'{name}.service.d'))
+        for unit_dir in glob.glob(os.path.join(ETC_SYSTEMD_LOCATION, f'{name}@*.service.d')):
+            remove_dir(unit_dir)
+
         self.update_dependent_list_file(package, remove=True)
         self.update_generated_services_conf_file(package, remove=True)
 

tests/sonic_package_manager/conftest.py

@@ -16,6 +16,7 @@
 from sonic_package_manager.registry import RegistryResolver
 from sonic_package_manager.version import Version
 from sonic_package_manager.service_creator.creator import *
+from sonic_package_manager.service_creator.creator import ETC_SYSTEMD_LOCATION
 
 
 @pytest.fixture
@@ -405,6 +406,7 @@ def fake_db_for_migration(fake_metadata_resolver):
 def sonic_fs(fs):
     fs.create_file('/proc/1/root')
     fs.create_dir(ETC_SONIC_PATH)
+    fs.create_dir(ETC_SYSTEMD_LOCATION)
     fs.create_dir(SYSTEMD_LOCATION)
     fs.create_dir(DOCKER_CTL_SCRIPT_LOCATION)
     fs.create_dir(SERVICE_MGMT_SCRIPT_LOCATION)

tests/sonic_package_manager/test_service_creator.py

@@ -12,6 +12,7 @@
 from sonic_package_manager.metadata import Metadata
 from sonic_package_manager.package import Package
 from sonic_package_manager.service_creator.creator import *
+from sonic_package_manager.service_creator.creator import ETC_SYSTEMD_LOCATION
 from sonic_package_manager.service_creator.feature import FeatureRegistry
 
 
@@ -106,6 +107,14 @@ def test_service_creator(sonic_fs, manifest, service_creator, package_manager):
     assert sonic_fs.exists(os.path.join(SERVICE_MGMT_SCRIPT_LOCATION, 'test.sh'))
     assert sonic_fs.exists(os.path.join(SYSTEMD_LOCATION, 'test.service'))
 
+    # Create symlinks and directory featured creates
+    os.symlink('/dev/null', os.path.join(ETC_SYSTEMD_LOCATION, 'test.service'))
+    os.symlink('/dev/null', os.path.join(ETC_SYSTEMD_LOCATION, '[email protected]'))
+    os.symlink('/dev/null', os.path.join(ETC_SYSTEMD_LOCATION, '[email protected]'))
+    os.mkdir(os.path.join(ETC_SYSTEMD_LOCATION, 'test.service.d'))
+    os.mkdir(os.path.join(ETC_SYSTEMD_LOCATION, '[email protected]'))
+    os.mkdir(os.path.join(ETC_SYSTEMD_LOCATION, '[email protected]'))
+
     def read_file(name):
         with open(os.path.join(ETC_SONIC_PATH, name)) as file:
             return file.read()
@@ -118,6 +127,15 @@ def read_file(name):
     assert generated_services_conf_content.endswith('\n')
     assert set(generated_services_conf_content.split()) == set(['test.service', '[email protected]'])
 
+    service_creator.remove(package)
+
+    assert not sonic_fs.exists(os.path.join(ETC_SYSTEMD_LOCATION, 'test.service'))
+    assert not sonic_fs.exists(os.path.join(ETC_SYSTEMD_LOCATION, '[email protected]'))
+    assert not sonic_fs.exists(os.path.join(ETC_SYSTEMD_LOCATION, '[email protected]'))
+    assert not sonic_fs.exists(os.path.join(ETC_SYSTEMD_LOCATION, 'test.service.d'))
+    assert not sonic_fs.exists(os.path.join(ETC_SYSTEMD_LOCATION, '[email protected]'))
+    assert not sonic_fs.exists(os.path.join(ETC_SYSTEMD_LOCATION, '[email protected]'))
+
 
 def test_service_creator_with_timer_unit(sonic_fs, manifest, service_creator):
     entry = PackageEntry('test', 'azure/sonic-test')