Format messages for consistency

- no full stop at the end of a report message title
- no word please (technical writers' rule)
- boot loader => bootloader
- fix typos
- add missing context where messages are too short and vague
- make TASK tense consistent
- ensure each Action has a TASK level log message
- ensure every Action prints what it's done
- add unit tests to lines reported by caplog as uncovered

Author: bocekm

convert2rhel/actions/__init__.py

@@ -559,7 +559,7 @@ def run(self, successes=None, failures=None, skips=None):
                     title="Skipped action",
                     description="This action was skipped due to another action failing.",
                     diagnosis=diagnosis,
-                    remediations="Please ensure that the {} check passes so that this Action can evaluate your system".format(
+                    remediations="Ensure that the {} check passes so that this Action can evaluate your system".format(
                         utils.format_sequence_as_message(failed_deps)
                     ),
                 )
@@ -586,7 +586,7 @@ def run(self, successes=None, failures=None, skips=None):
 
             # Categorize the results
             if action.result.level <= STATUS_CODE["WARNING"]:
-                logger.info("{} has succeeded".format(action.id))
+                logger.info("The {} action has succeeded.".format(action.id))
                 successes.append(action)
 
             if action.result.level > STATUS_CODE["WARNING"]:
@@ -742,7 +742,7 @@ def run_pre_actions():
     # When we call check_dependencies() or run() on the first Stage
     # (system_checks), it will operate on the first Stage and then recursively
     # call check_dependencies() or run() on the next_stage.
-    pre_ponr_changes = Stage("pre_ponr_changes", "Making recoverable changes")
+    pre_ponr_changes = Stage("pre_ponr_changes", "Make recoverable changes")
     system_checks = Stage("system_checks", "Check whether system is ready for conversion", next_stage=pre_ponr_changes)
 
     try:

convert2rhel/actions/conversion/list_non_red_hat_pkgs_left.py

@@ -34,7 +34,7 @@ def run(self):
         super(ListNonRedHatPkgsLeft, self).run()
         loggerinst.task("List remaining non-Red Hat packages")
 
-        loggerinst.info("Listing packages not signed by Red Hat")
+        loggerinst.info("Listing packages not signed by Red Hat.")
         non_red_hat_pkgs = get_installed_pkgs_w_different_key_id(system_info.key_ids_rhel)
         if not non_red_hat_pkgs:
             loggerinst.info("All packages are now signed by Red Hat.")

convert2rhel/actions/conversion/preserve_only_rhel_kernel.py

@@ -36,7 +36,7 @@ def run(self):
         non-RHEL kernel(s) conflicted with the available RHEL kernels.
         """
         super(InstallRhelKernel, self).run()
-        loggerinst.info("Verifying that RHEL kernel has been installed")
+        loggerinst.task("Verify RHEL kernel installation")
 
         rhel_kernels = pkghandler.get_installed_pkgs_by_key_id(system_info.key_ids_rhel, name="kernel")
 
@@ -68,8 +68,8 @@ class FixInvalidGrub2Entries(actions.Action):
 
     def run(self):
         """
-        On systems derived from RHEL 8 and later, /etc/machine-id is being used to identify grub2 boot loader entries per
-        the Boot Loader Specification.
+        On systems derived from RHEL 8 and later, /etc/machine-id is being used to identify grub2 bootloader entries per
+        the bootloader Specification.
         However, at the time of executing convert2rhel, the current machine-id can be different from the machine-id from the
         time when the kernels were installed. If that happens:
         - convert2rhel installs the RHEL kernel, but it's not set as default
@@ -79,18 +79,18 @@ def run(self):
         """
         super(FixInvalidGrub2Entries, self).run()
 
+        loggerinst.task("Fix GRUB2 bootloader entries")
         if system_info.version.major < 8:
             # Applicable only on systems derived from RHEL 8 and later, and systems using GRUB2 (s390x uses zipl)
+            loggerinst.info("Skipped. Only relevant to RHEL 8 and newer.")
             return
 
-        loggerinst.info("Fixing GRUB boot loader entries.")
-
         machine_id = utils.get_file_content("/etc/machine-id").strip()
         boot_entries = glob.glob("/boot/loader/entries/*.conf")
         for entry in boot_entries:
-            # The boot loader entries in /boot/loader/entries/<machine-id>-<kernel-version>.conf
+            # The bootloader entries in /boot/loader/entries/<machine-id>-<kernel-version>.conf
             if machine_id not in os.path.basename(entry):
-                loggerinst.debug("Removing boot entry {}".format(entry))
+                loggerinst.debug("Removing boot entry {}.".format(entry))
                 os.remove(entry)
 
         # Removing a boot entry that used to be the default makes grubby to choose a different entry as default,
@@ -99,24 +99,24 @@ def run(self):
         if ret_code:
             # Not setting the default entry shouldn't be a deal breaker and the reason to stop the conversions,
             # grub should pick one entry in any case.
-            description = "Couldn't get the default GRUB2 boot loader entry:\n{}".format(output)
+            description = "Couldn't get the default GRUB2 bootloader entry:\n{}".format(output)
             loggerinst.warning(description)
             self.add_message(
                 level="WARNING",
                 id="UNABLE_TO_GET_GRUB2_BOOT_LOADER_ENTRY",
-                title="Unable to get the GRUB2 boot loader entry",
+                title="Unable to get the GRUB2 bootloader entry",
                 description=description,
             )
             return
-        loggerinst.debug("Setting RHEL kernel {} as the default boot loader entry.".format(output.strip()))
+        loggerinst.debug("Setting RHEL kernel {} as the default bootloader entry.".format(output.strip()))
         output, ret_code = utils.run_subprocess(["/usr/sbin/grubby", "--set-default", output.strip()])
         if ret_code:
-            description = "Couldn't set the default GRUB2 boot loader entry:\n{}".format(output)
+            description = "Couldn't set the default GRUB2 bootloader entry:\n{}".format(output)
             loggerinst.warning(description)
             self.add_message(
                 level="WARNING",
                 id="UNABLE_TO_SET_GRUB2_BOOT_LOADER_ENTRY",
-                title="Unable to set the GRUB2 boot loader entry",
+                title="Unable to set the GRUB2 bootloader entry",
                 description=description,
             )
 
@@ -134,7 +134,7 @@ def run(self):
         """
         super(FixDefaultKernel, self).run()
 
-        loggerinst.info("Checking for incorrect boot kernel")
+        loggerinst.task("Check for default boot kernel in /etc/sysconfig/kernel")
         kernel_sys_cfg = utils.get_file_content("/etc/sysconfig/kernel")
 
         possible_kernels = ["kernel-uek", "kernel-plus"]
@@ -143,22 +143,29 @@ def run(self):
             None,
         )
         if kernel_to_change:
-            description = "Detected leftover boot kernel, changing to RHEL kernel"
-            loggerinst.warning(description)
+            diagnosis = "Detected default boot kernel {} in /etc/sysconfig/kernel.".format(kernel_to_change)
+            # need to change to "kernel" in rhel7 and "kernel-core" in rhel8
+            new_kernel_str = "DEFAULTKERNEL=" + ("kernel" if system_info.version.major == 7 else "kernel-core")
+            loggerinst.warning(diagnosis)
             self.add_message(
                 level="WARNING",
                 id="LEFTOVER_BOOT_KERNEL_DETECTED",
-                title="Leftover boot kernel detected",
-                description=description,
+                title="Leftover default boot kernel detected",
+                diagnosis=diagnosis,
+                description="Some systems have the default boot kernel in /etc/sysconfig/kernel set to a distribution"
+                " specific kernel even after such a kernel is uninstalled. We have changed it to {}.".format(
+                    new_kernel_str
+                ),
             )
-            # need to change to "kernel" in rhel7 and "kernel-core" in rhel8
-            new_kernel_str = "DEFAULTKERNEL=" + ("kernel" if system_info.version.major == 7 else "kernel-core")
-
             kernel_sys_cfg = kernel_sys_cfg.replace("DEFAULTKERNEL=" + kernel_to_change, new_kernel_str)
             utils.store_content_to_file("/etc/sysconfig/kernel", kernel_sys_cfg)
-            loggerinst.info("Boot kernel {} was changed to {}".format(kernel_to_change, new_kernel_str))
+            loggerinst.info(
+                "Default boot kernel {} was changed to {} in /etc/sysconfig/kernel.".format(
+                    kernel_to_change, new_kernel_str
+                )
+            )
         else:
-            loggerinst.debug("Boot kernel validated.")
+            loggerinst.debug("The default boot kernel is correct.")
 
 
 class KernelPkgsInstall(actions.Action):
@@ -169,18 +176,20 @@ def run(self):
         """Remove non-RHEL kernels."""
         super(KernelPkgsInstall, self).run()
 
+        loggerinst.task("Remove non-RHEL kernels")
+
         kernel_pkgs_to_install = self.remove_non_rhel_kernels()
         if kernel_pkgs_to_install:
             self.install_additional_rhel_kernel_pkgs(kernel_pkgs_to_install)
 
     def remove_non_rhel_kernels(self):
-        loggerinst.info("Searching for non-RHEL kernels ...")
+        loggerinst.info("Searching for non-RHEL kernels.")
         non_rhel_kernels = pkghandler.get_installed_pkgs_w_different_key_id(system_info.key_ids_rhel, "kernel*")
         if not non_rhel_kernels:
             loggerinst.info("None found.")
             return None
 
-        loggerinst.info("Removing non-RHEL kernels\n")
+        loggerinst.info("Removing detected non-RHEL kernels.\n")
         pkghandler.print_pkg_info(non_rhel_kernels)
         pkgs_to_remove = [pkghandler.get_pkg_nvra(pkg) for pkg in non_rhel_kernels]
         utils.remove_pkgs(pkgs_to_remove)
@@ -198,7 +207,7 @@ def install_additional_rhel_kernel_pkgs(self, additional_pkgs):
         pkg_names = [p.nevra.name.replace(ol_kernel_ext, "", 1) for p in additional_pkgs]
         for name in set(pkg_names):
             if name != "kernel":
-                loggerinst.info("Installing RHEL {}".format(name))
+                loggerinst.info("Installing RHEL {}.".format(name))
                 pkgmanager.call_yum_cmd("install", args=[name])
 
 
@@ -217,4 +226,5 @@ def run(self):
         """
         super(UpdateKernel, self).run()
 
+        loggerinst.task("Update RHEL kernel")
         pkghandler.update_rhel_kernel()

convert2rhel/actions/conversion/set_efi_config.py

@@ -169,7 +169,7 @@ def run(self):
                 )
                 continue
 
-            logger.info("Moving '{}' to '{}'".format(src_file, dst_file))
+            logger.info("Moving '{}' to '{}'.".format(src_file, dst_file))
 
             try:
                 shutil.move(src_file, dst_file)

convert2rhel/actions/post_conversion/hostmetering.py

@@ -69,8 +69,8 @@ def run(self):
             self.add_message(
                 level="WARNING",
                 id="INSTALL_HOST_METERING_FAILURE",
-                title="Failed to install host metering package.",
-                description="When installing host metering package an error occurred meaning we can't"
+                title="Failed to install the host-metering package",
+                description="When installing the host-metering package an error occurred meaning we can't"
                 " enable host metering on the system.",
                 diagnosis="`yum install host-metering` command returned {ret_install} with message {output}".format(
                     ret_install=ret_install, output=output
@@ -89,9 +89,9 @@ def run(self):
             self.add_message(
                 level="WARNING",
                 id="CONFIGURE_HOST_METERING_FAILURE",
-                title="Failed to enable and start host metering service.",
+                title="Failed to enable and start the host metering service",
                 description="The host metering service failed to start"
-                " successfully and won't be able to keep track.",
+                " successfully and won't be able to report on the use of the system for the billing purposes.",
                 diagnosis="Command {command} failed with {error_message}".format(
                     command=command, error_message=error_message
                 ),
@@ -109,7 +109,7 @@ def run(self):
             self.set_result(
                 level="ERROR",
                 id="HOST_METERING_NOT_RUNNING",
-                title="Host metering service is not running.",
+                title="Host metering service is not running",
                 description="host-metering.service is not running.",
                 remediations="You can try to start the service manually"
                 " by running following command:\n"
@@ -126,7 +126,7 @@ def _check_host_metering_configuration(self):
         :rtype: bool
         """
         if tool_opts.configure_host_metering is None:
-            logger.debug("You have not enabled configuration of host metering. Skipping it.")
+            logger.info("You have not enabled configuration of host metering. Skipping it.")
             return False
 
         if tool_opts.configure_host_metering not in ("force", "auto"):
@@ -152,19 +152,22 @@ def _check_host_metering_configuration(self):
         if tool_opts.configure_host_metering == "force":
             logger.warning(
                 "You've set the host metering setting to 'force'."
-                " Please note that this option is mainly used for testing and will configure host-metering unconditionally. "
-                " For generic usage please use the 'auto' option."
+                " Note that this option is mainly used for testing and will configure host-metering unconditionally. "
+                " For generic usage use the 'auto' option."
             )
             self.add_message(
                 level="WARNING",
                 id="FORCED_CONFIGURE_HOST_METERING",
                 title="Configuration of host metering set to 'force'",
-                description="Please note that this option is mainly used for testing and"
+                description="Note that this option is mainly used for testing and"
                 " will configure host-metering unconditionally."
-                " For generic usage please use the 'auto' option.",
+                " For generic usage use the 'auto' option.",
             )
         elif tool_opts.configure_host_metering == "auto":
-            logger.debug("Automatic detection of host hyperscaler and configuration.")
+            logger.debug(
+                "Configuration of host metering set to 'auto' - host-metering will be enabled based on"
+                " a detected hyperscaler."
+            )
 
         return True
 

convert2rhel/actions/post_conversion/modified_rpm_files_diff.py

@@ -46,7 +46,7 @@ def run(self):
             self.add_message(
                 level="INFO",
                 id="SKIPPED_MODIFIED_RPM_FILES_DIFF",
-                title="Skipped comparison of 'rpm -Va' output from before and after the conversion.",
+                title="Skipped comparison of 'rpm -Va' output from before and after the conversion",
                 description="Comparison of 'rpm -Va' output was not performed due to missing output "
                 "of the 'rpm -Va' run before the conversion.",
                 diagnosis="This is caused mainly by using '--no-rpm-va' argument for convert2rhel.",
@@ -76,8 +76,8 @@ def run(self):
             self.add_message(
                 level="INFO",
                 id="FOUND_MODIFIED_RPM_FILES",
-                title="Modified rpm files from before and after the conversion were found.",
-                description="Comparison of modified rpm files from before and after " "the conversion: \n{}".format(
+                title="Modified rpm files from before and after the conversion were found",
+                description="Comparison of modified rpm files from before and after the conversion: \n{}".format(
                     modified_rpm_files_diff
                 ),
             )

convert2rhel/actions/post_conversion/remove_tmp_dir.py

@@ -45,7 +45,7 @@ def run(self):
 
         try:
             shutil.rmtree(self.tmp_dir)
-            loggerinst.info("Temporary folder {} removed".format(self.tmp_dir))
+            loggerinst.info("Temporary folder {} removed.".format(self.tmp_dir))
         except OSError as exc:
             # We want run() to be idempotent, so do nothing silently if
             # the path doesn't exist.
@@ -61,6 +61,6 @@ def run(self):
             self.add_message(
                 level="WARNING",
                 id="UNSUCCESSFUL_REMOVE_TMP_DIR",
-                title="Temporary folder {tmp_dir} wasn't removed.".format(tmp_dir=self.tmp_dir),
+                title="Temporary folder {tmp_dir} wasn't removed".format(tmp_dir=self.tmp_dir),
                 description=warning_message,
             )

convert2rhel/actions/post_conversion/update_grub.py

@@ -77,7 +77,7 @@ def run(self):
                     level="ERROR",
                     id="FAILED_TO_IDENTIFY_GRUB2_BLOCK_DEVICE",
                     title="Failed to identify GRUB2 block device",
-                    description="The block device could not be identified, please look at the diagnosis "
+                    description="The block device could not be identified. Look at the diagnosis "
                     "for more information.",
                     diagnosis=str(e),
                 )

convert2rhel/actions/pre_ponr_changes/backup_system.py

@@ -51,8 +51,8 @@ class BackupRedhatRelease(actions.Action):
     id = "BACKUP_REDHAT_RELEASE"
 
     def run(self):
-        """Backup redhat release file before starting conversion process"""
-        logger.task("Backup Redhat Release Files")
+        """Back up redhat release file before starting conversion process"""
+        logger.task("Back up redhat-release files")
 
         super(BackupRedhatRelease, self).run()
 
@@ -79,7 +79,7 @@ class BackupRepository(actions.Action):
 
     def run(self):
         """Backup .repo files in /etc/yum.repos.d/ so the repositories can be restored on rollback."""
-        logger.task("Backup Repository Files")
+        logger.task("Back up repository files")
 
         super(BackupRepository, self).run()
 
@@ -105,7 +105,7 @@ class BackupYumVariables(actions.Action):
 
     def run(self):
         """Backup varsdir folder in /etc/{yum,dnf}/vars so the variables can be restored on rollback."""
-        logger.task("Backup variables")
+        logger.task("Back up variables")
 
         super(BackupYumVariables, self).run()
 
@@ -146,7 +146,7 @@ def run(self):
         """Backup changed package files"""
         super(BackupPackageFiles, self).run()
 
-        logger.task("Backup package files")
+        logger.task("Back up package files")
 
         package_files_changes = self._get_changed_package_files()
 
@@ -178,7 +178,6 @@ def _get_changed_package_files(self):
         """Get the output from rpm -Va command from during resolving system info
         to get changes made to package files.
 
-
         :return dict: Return them as a list of dict, for example:
         [{"status":"S5T", "file_type":"c", "path":"/etc/yum.repos.d/CentOS-Linux-AppStream.repo"}]
         """
@@ -199,10 +198,9 @@ def _get_changed_package_files(self):
                 # Return empty list results in no backup of the files
                 return data
             else:
-                # The file should be there
-                # If missing conversion is in unknown state
+                # The file should be there. If missing, the conversion is in an unknown state.
                 logger.warning("Error({}): {}".format(err.errno, err.strerror))
-                logger.critical("Missing file {rpm_va_output} in it's location".format(rpm_va_output=path))
+                logger.critical("The file {rpm_va_output} is missing.".format(rpm_va_output=path))
 
         lines = output.strip().split("\n")
         for line in lines:
@@ -220,7 +218,7 @@ def _parse_line(self, line):
         if not match:  # line not matching the regex
             if line.strip() != "":
                 # Line is not empty string
-                logger.debug("Skipping invalid output {}".format(line))
+                logger.debug("Skipping invalid output: {}".format(line))
             return {"status": None, "file_type": None, "path": None}
 
         line = line.split()