Refactoring

cloudlinux7to8/actions/common_checks.py

@@ -19,7 +19,7 @@ def _do_check(self) -> bool:
 
 
 class AssertNoMoreThenOneKernelNamedNIC(action.CheckAction):
-    def __init__(self):
+    def __init__(self) -> None:
         self.name = "checking if there is more than one NIC interface using ketnel-name"
         self.description = """The system has one or more network interface cards (NICs) using kernel-names (ethX).
 \tLeapp cannot guarantee the interface names' stability during the conversion.
@@ -28,7 +28,7 @@ def __init__(self):
 """
 
     def _do_check(self) -> bool:
-        # We can't use this method th get interfaces names, so just skip the check
+        # We can't use this method to get interfaces names, so just skip the check
         if not os.path.exists("/sys/class/net"):
             return True
 
@@ -43,7 +43,7 @@ def _do_check(self) -> bool:
 
 # ToDo. Implement for deb-based and move to common part. Might be useful for distupgrade/other converters
 class AssertLastInstalledKernelInUse(action.CheckAction):
-    def __init__(self):
+    def __init__(self) -> None:
         self.name = "checking if the last installed kernel is in use"
         self.description = """The last installed kernel is not in use.
 \tThe kernel version in use is '{}'. The last installed kernel version is '{}'.
@@ -77,7 +77,7 @@ def _do_check(self) -> bool:
 
 
 class AssertRedHatKernelInstalled(action.CheckAction):
-    def __init__(self):
+    def __init__(self) -> None:
         self.name = "checking if the Red Hat kernel is installed"
         self.description = """No Red Hat signed kernel is installed.
 \tTo proceed with the conversion, install a kernel by running:
@@ -104,12 +104,12 @@ def _find_repo_files() -> typing.List[str]:
 class AssertLocalRepositoryNotPresent(action.CheckAction):
     def __init__(self):
         self.name = "checking if the local repository is present"
-        self.description = """There are rpm repository with local storage present. Leapp is not support such kind of repositories.
+        self.description = """There are rpm repositories with local storage present. Leapp is not support such kind of repositories.
 \tPlease remove the local repositories to proceed the conversion. Files where locally stored repositories are defined:
 \t- {}
 """
 
-    def _is_repo_contains_local_storage(self, repo_file) -> bool:
+    def _is_repo_with_local_storage(self, repo_file) -> bool:
         with open(repo_file) as f:
             repository_content = f.read()
             return ("baseurl=file:" in repository_content or "baseurl = file:" in repository_content or
@@ -121,7 +121,7 @@ def _do_check(self) -> bool:
         # but leapp allows it anyway. So we could skip it.
         local_repositories_files = [
             file for file in _find_repo_files()
-            if os.path.basename(file) != "CentOS-Media.repo" and self._is_repo_contains_local_storage(file)
+            if os.path.basename(file) != "CentOS-Media.repo" and self._is_repo_with_local_storage(file)
         ]
 
         if len(local_repositories_files) == 0:
@@ -131,13 +131,13 @@ def _do_check(self) -> bool:
         return False
 
 
-class AssertThereIsNoRepositoryDuplicates(action.CheckAction):
-    def __init__(self):
+class AssertNoRepositoryDuplicates(action.CheckAction):
+    def __init__(self) -> None:
         self.name = "checking if there are duplicate repositories"
         self.description = """There are duplicate repositories present:
 \t- {}
 
-\tPlease remove the duplicate to proceed the conversion.
+\tPlease remove duplicates to proceed the conversion.
 """
 
     def _do_check(self) -> bool:
@@ -170,15 +170,15 @@ def _do_check(self) -> bool:
 
 
 class AssertAvailableSpace(action.CheckAction):
-    def __init__(self):
+    def __init__(self) -> None:
         self.name = "checking available space"
         self.required_space = 5 * 1024 * 1024 * 1024  # 5GB
         self.description = """There is insufficient disk space available. Leapp requires a minimum of {} of free space
 \ton the disk where the '/var/lib' directory is located. Available space: {}.
 \tFree up enough disk space and try again.
 """
 
-    def _huminize_size(self, size) -> str:
+    def _get_human_readable_size(self, size) -> str:
         original = size
         for unit in ("B", "KB", "MB", "GB", "TB"):
             if size < 1024:
@@ -193,5 +193,5 @@ def _do_check(self) -> bool:
         if available_space >= self.required_space:
             return True
 
-        self.description = self.description.format(self._huminize_size(self.required_space), self._huminize_size(available_space))
+        self.description = self.description.format(self._get_human_readable_size(self.required_space), self._get_human_readable_size(available_space))
         return False

cloudlinux7to8/actions/convert.py

@@ -2,7 +2,7 @@
 from pleskdistup.common import action, util
 
 
-class DoCentos2AlmaConvert(action.ActiveAction):
+class DoCloudLinux7to8Convert(action.ActiveAction):
     def __init__(self):
         self.name = "doing the conversion"
 

cloudlinux7to8/actions/mariadb.py

@@ -12,14 +12,23 @@
     "mariadb.repo",
     "mariadb10.repo",
 ]
+MARIADB_PACKAGES = [
+    "MariaDB-client",
+    "MariaDB-client-compat",
+    "MariaDB-compat",
+    "MariaDB-common",
+    "MariaDB-server",
+    "MariaDB-server-compat",
+    "MariaDB-shared"
+]
 
 
 def _find_mariadb_repo_files() -> typing.List[str]:
     return files.find_files_case_insensitive("/etc/yum.repos.d", KNOWN_MARIADB_REPO_FILES)
 
 
 class AssertMariadbRepoAvailable(action.CheckAction):
-    def __init__(self):
+    def __init__(self) -> None:
         self.name = "check mariadb repo available"
         self.description = """
 The MariaDB repository with id '{}' from the file '{}' is not accessible.
@@ -52,8 +61,12 @@ def _do_check(self) -> bool:
         return True
 
 
+def _remove_mariadb_packages() -> None:
+    rpm.remove_packages(rpm.filter_installed_packages(MARIADB_PACKAGES))
+
+
 class UpdateModernMariadb(action.ActiveAction):
-    def __init__(self):
+    def __init__(self) -> None:
         self.name = "update modern mariadb"
 
     def _is_required(self) -> bool:
@@ -81,13 +94,7 @@ def _post_action(self) -> action.ActionResult:
 
         mariadb_repo_id, _1, _2, _3, _4, _5 = [repo for repo in rpm.extract_repodata(repofiles[0])][0]
 
-        rpm.remove_packages(rpm.filter_installed_packages(["MariaDB-client",
-                                                           "MariaDB-client-compat",
-                                                           "MariaDB-compat",
-                                                           "MariaDB-common",
-                                                           "MariaDB-server",
-                                                           "MariaDB-server-compat",
-                                                           "MariaDB-shared"]))
+        _remove_mariadb_packages()
         rpm.install_packages(["MariaDB-client", "MariaDB-server"], repository=mariadb_repo_id)
         return action.ActionResult()
 
@@ -102,40 +109,28 @@ def estimate_post_time(self) -> int:
 
 
 class UpdateMariadbDatabase(action.ActiveAction):
-    def __init__(self):
+    def __init__(self) -> None:
         self.name = "updating mariadb databases"
 
     def _is_required(self) -> bool:
         return mariadb.is_mariadb_installed() and not mariadb.get_installed_mariadb_version() > MARIADB_VERSION_ON_ALMA
 
     def _prepare_action(self) -> action.ActionResult:
-        rpm.remove_packages(rpm.filter_installed_packages(["MariaDB-client",
-                                                           "MariaDB-client-compat",
-                                                           "MariaDB-compat",
-                                                           "MariaDB-common",
-                                                           "MariaDB-server",
-                                                           "MariaDB-server-compat",
-                                                           "MariaDB-shared"]))
+        _remove_mariadb_packages()
         return action.ActionResult()
 
     def _post_action(self) -> action.ActionResult:
-        # Leapp is not remove non-standard MariaDB-client package. But since we have updated
+        # Leapp does not remove non-standard MariaDB-client package. But since we have updated
         # mariadb to 10.3.35 old client is not relevant anymore. So we have to switch to new client.
-        # On the other hand we want to be sure AlmaLinux mariadb-server installed as well
+        # On the other hand, we want to be sure AlmaLinux mariadb-server installed as well
         for repofile in _find_mariadb_repo_files():
             files.backup_file(repofile)
             os.unlink(repofile)
 
-        rpm.remove_packages(rpm.filter_installed_packages(["MariaDB-client",
-                                                           "MariaDB-client-compat",
-                                                           "MariaDB-compat",
-                                                           "MariaDB-common",
-                                                           "MariaDB-server",
-                                                           "MariaDB-server-compat",
-                                                           "MariaDB-shared"]))
+        _remove_mariadb_packages()
         rpm.install_packages(["mariadb", "mariadb-server"])
 
-        # We should be sure mariadb is started, otherwise restore woulden't work
+        # We should be sure mariadb is started, otherwise restore wouldn't work
         util.logged_check_call(["/usr/bin/systemctl", "start", "mariadb"])
 
         with open('/etc/psa/.psa.shadow', 'r') as shadowfile:
@@ -149,12 +144,12 @@ def _post_action(self) -> action.ActionResult:
     def _revert_action(self) -> action.ActionResult:
         return action.ActionResult()
 
-    def estimate_post_time(self):
+    def estimate_post_time(self) -> int:
         return 2 * 60
 
 
 class AddMysqlConnector(action.ActiveAction):
-    def __init__(self):
+    def __init__(self) -> None:
         self.name = "install mysql connector"
 
     def _is_required(self) -> bool:

cloudlinux7to8/actions/packages.py

@@ -157,19 +157,18 @@ def _revert_action(self) -> action.ActionResult:
     def estimate_prepare_time(self) -> int:
         return 10
 
-    def estimate_post_time(self) -> int:
-        pkgs_number = 0
+    @property
+    def _removed_packages_num(self) -> int:
         if os.path.exists(self.removed_packages_file):
             with open(self.removed_packages_file, "r") as f:
-                pkgs_number = len(f.read().splitlines())
-        return 60 + 10 * pkgs_number
+                return len(f.read().splitlines())
+        return 0
+
+    def estimate_post_time(self) -> int:
+        return 60 + 10 * self._removed_packages_num
 
     def estimate_revert_time(self) -> int:
-        pkgs_number = 0
-        if os.path.exists(self.removed_packages_file):
-            with open(self.removed_packages_file, "r") as f:
-                pkgs_number = len(f.read().splitlines())
-        return 60 + 10 * pkgs_number
+        return 60 + 10 * self._removed_packages_num
 
 
 CHANGED_REPOS_MSG_FMT = """During the conversion, some of customized .repo files were updated. You can find the old
@@ -186,9 +185,9 @@ def _prepare_action(self) -> action.ActionResult:
         return action.ActionResult()
 
     def _use_rpmnew_repositories(self) -> None:
-        # The problem is about changed repofiles, that leapp tring to install form packages.
+        # The problem is about changed repofiles, that leapp is trying to install from packages.
         # For example, when epel.repo file was changed, dnf will save the new one as epel.repo.rpmnew.
-        # I beleive there could be other files with the same problem, so lets iterate every .rpmnew file in /etc/yum.repos.d
+        # I beleive there could be other files with the same problem, so let's iterate every .rpmnew file in /etc/yum.repos.d
         fixed_list = []
         for file in files.find_files_case_insensitive("/etc/yum.repos.d", ["*.rpmnew"]):
             original_file = file[:-len(".rpmnew")]
@@ -225,8 +224,8 @@ def estimate_post_time(self) -> int:
 
 class AssertPleskRepositoriesNotNoneLink(action.CheckAction):
     def __init__(self):
-        self.name = "checking if plesk repositories are adoptable"
-        self.description = """There are plesk repositories has none link. To proceed the conversion, remove following repositories:
+        self.name = "checking if plesk repositories does not have a 'none' link"
+        self.description = """There are plesk repositories with link set to 'none'. To proceed with the conversion, remove following repositories:
 \t- {}
 """
 
@@ -303,7 +302,7 @@ def estimate_post_time(self) -> int:
         return 3 * 60
 
 
-class CheckOutdatedLetsencryptExtensionRepository(action.CheckAction):
+class AssertNoOutdatedLetsEncryptExtRepository(action.CheckAction):
     OUTDATED_LETSENCRYPT_REPO_PATHS = ["/etc/yum.repos.d/plesk-letsencrypt.repo", "/etc/yum.repos.d/plesk-ext-letsencrypt.repo"]
 
     def __init__(self) -> None:

cloudlinux7to8/actions/perl.py

@@ -66,13 +66,19 @@ def _do_check(self):
 
 
 class ReinstallPerlCpanModules(action.ActiveAction):
-    def __init__(self, store_dir: str):
+    removed_modules_file: str
+
+    def __init__(self, store_dir: str) -> None:
         self.name = "reinstalling perl cpan modules"
         self.removed_modules_file = os.path.join(store_dir, "cloudlinux7to8_removed_perl_modules.txt")
 
-    def _is_required(self):
+    def _is_required(self) -> bool:
         return not files.is_directory_empty(CPAN_MODULES_DIRECTORY)
 
+    @property
+    def cpan_modules_directory_backup(self) -> str:
+        return CPAN_MODULES_DIRECTORY + ".backup"
+
     def _prepare_action(self) -> action.ActionResult:
         with open(self.removed_modules_file, "w") as f:
             for module in files.find_files_case_insensitive(CPAN_MODULES_DIRECTORY, ["*.pm"], recursive=True):
@@ -83,12 +89,12 @@ def _prepare_action(self) -> action.ActionResult:
         # but cpan don't have an option to remove one module for some reason.
         # Since we can't be sure cpan-minimal is installed, we have to
         # remove all in barbaric way.
-        shutil.move(CPAN_MODULES_DIRECTORY, CPAN_MODULES_DIRECTORY + ".backup")
+        shutil.move(CPAN_MODULES_DIRECTORY, self.cpan_modules_directory_backup)
         return action.ActionResult()
 
     def _post_action(self) -> action.ActionResult:
         if not os.path.exists(self.removed_modules_file):
-            no_file_warning = "The file containing the list of removed Perl modules does not exist. However, the action itself was not skipped. You can find the previously installed modules at the following path: {}.\n".format(CPAN_MODULES_DIRECTORY + ".backup")
+            no_file_warning = f"The file containing the list of removed Perl modules does not exist. However, the action itself was not skipped. You can find the previously installed modules at the following path: {CPAN_MODULES_DIRECTORY}.backup.\n"
             log.warn(no_file_warning)
             motd.add_finish_ssh_login_message(no_file_warning)
             return action.ActionResult()
@@ -98,17 +104,17 @@ def _post_action(self) -> action.ActionResult:
             rpm.install_packages(packages_to_install)
 
         os.unlink(self.removed_modules_file)
-        shutil.rmtree(CPAN_MODULES_DIRECTORY + ".backup")
+        shutil.rmtree(self.cpan_modules_directory_backup)
         return action.ActionResult()
 
     def _revert_action(self) -> action.ActionResult:
-        if os.path.exists(CPAN_MODULES_DIRECTORY + ".backup"):
-            shutil.move(CPAN_MODULES_DIRECTORY + ".backup", CPAN_MODULES_DIRECTORY)
+        if os.path.exists(self.cpan_modules_directory_backup):
+            shutil.move(self.cpan_modules_directory_backup, CPAN_MODULES_DIRECTORY)
 
         if os.path.exists(self.removed_modules_file):
             os.unlink(self.removed_modules_file)
 
         return action.ActionResult()
 
-    def estimate_post_time(self):
+    def estimate_post_time(self) -> int:
         return 60

cloudlinux7to8/actions/php.py

@@ -13,9 +13,7 @@ def __init__(self) -> None:
         self.name = "fix OS vendor PHP configuration"
 
     def is_required(self) -> bool:
-        if os.path.exists(OS_VENDOR_PHP_FPM_CONFIG):
-            return True
-        return False
+        return os.path.exists(OS_VENDOR_PHP_FPM_CONFIG)
 
     def _prepare_action(self) -> action.ActionResult:
         return action.ActionResult()

cloudlinux7to8/actions/postgres.py

@@ -21,6 +21,8 @@ def _do_check(self) -> bool:
 
 
 class PostgresDatabasesUpdate(action.ActiveAction):
+    service_name: str
+
     def __init__(self) -> None:
         self.name = "updating PostgreSQL databases"
         self.service_name = 'postgresql'

cloudlinux7to8/upgrader.py

@@ -160,7 +160,7 @@ def construct_actions(
             ],
             "Do convert": [
                 custom_actions.AdoptRepositories(),
-                custom_actions.DoCentos2AlmaConvert(),
+                custom_actions.DoCloudLinux7to8Convert(),
             ],
             "Pause before reboot": [
             ],
@@ -214,11 +214,11 @@ def get_check_actions(
             custom_actions.AssertRedHatKernelInstalled(),
             custom_actions.AssertLastInstalledKernelInUse(),
             custom_actions.AssertLocalRepositoryNotPresent(),
-            custom_actions.AssertThereIsNoRepositoryDuplicates(),
+            custom_actions.AssertNoRepositoryDuplicates(),
             custom_actions.AssertMariadbRepoAvailable(),
             common_actions.AssertNotInContainer(),
             custom_actions.AssertPackagesUpToDate(),
-            custom_actions.CheckOutdatedLetsencryptExtensionRepository(),
+            custom_actions.AssertNoOutdatedLetsEncryptExtRepository(),
             custom_actions.AssertPleskRepositoriesNotNoneLink(),
         ]