Clarify some code (SUSE#1070)

* Remove unnecessary methods/variables for extracting version

This is not a problem to use functions, variables for defining
the version. However they burden the reviewer, which need
to jumping through things, where it's not really necessary.
This simplifies the review and the code.

* Clarify "restart"

Without this patch, one might be confused by the meaning of
restart, and why we are doing actions twice.

This patch makes it clear that the method used to evaluate
if zypper should run twice is in fact dependent of zypper itself
and it's return code (not on some kind of obscure code output).
It should be easier to follow/review with this kind of wording.

Co-authored-by: Jean-Philippe Evrard <jevrard@suse.com>

skuba-update/skuba_update/skuba_update.py

@@ -43,7 +43,6 @@
 REBOOT_REQUIRED_PATH = '/var/run/reboot-required'
 
 # Exit codes as defined by zypper.
-
 ZYPPER_EXIT_INF_UPDATE_NEEDED = 100
 ZYPPER_EXIT_INF_SEC_UPDATE_NEEDED = 101
 ZYPPER_EXIT_INF_REBOOT_NEEDED = 102
@@ -91,36 +90,30 @@ def parse_args():
 
     annotate_only_msg = \
         'Do not install any update, just annotate there are available updates'
-    version_msg = f"%(prog)s {version()}"
 
     parser = argparse.ArgumentParser(description='Updates a CaaSP node')
     parser.add_argument(
         '--annotate-only', action='store_true', help=annotate_only_msg
     )
     parser.add_argument(
-        '--version', action='version', version=version_msg
+        "--version",
+        action="version",
+        version="%(prog)s {0}".format(
+            pkg_resources.require("skuba-update")[0].version),
     )
 
     return parser.parse_args()
 
 
-def version():
-    """
-    Returns the version of the current skuba-update
-    """
-
-    return pkg_resources.require('skuba-update')[0].version
-
-
 def update():
     """
     Performs an update operation.
     """
 
-    code = run_zypper_patch()
-    if is_restart_needed(code):
-        code = run_zypper_patch()
-    return code
+    returncode = run_zypper_patch()
+    if zypper_needs_transaction_restart(returncode):
+        returncode = run_zypper_patch()
+    return returncode
 
 
 def annotate_node():
@@ -256,10 +249,10 @@ def is_zypper_error(code):
     return code != 0 and code < ZYPPER_EXIT_INF_UPDATE_NEEDED
 
 
-def is_restart_needed(code):
+def zypper_needs_transaction_restart(code):
     """
-    Returns true of the given code is defined by zypper to mean that restart is
-    needed (zypper itself has been updated).
+    Returns true if the given return code by zypper means a restart
+    of zypper is needed (zypper itself has been updated).
     """
 
     return code == ZYPPER_EXIT_INF_RESTART_NEEDED
@@ -322,8 +315,8 @@ def run_zypper_command(command, needsOutput=False):
 
 def run_zypper_patch():
     """
-    Run patch updates without --with-optional. --with-optional can cause
-    conflicts with K8s upgrade scenario.
+    Install patch updates (zypper patch) without '--with-optional' flag.
+    --with-optional can cause conflicts with K8s upgrade scenario.
     """
     return run_zypper_command([
         '--non-interactive', '--non-interactive-include-reboot-patches',