enh: only restart nginx/subervisor in 'dcor inspect' when necessary

CHANGELOG

@@ -1,3 +1,5 @@
+0.10.11
+ - enh: only restart nginx/subervisor in 'dcor inspect' when necessary
 0.10.10
  - fix: don't ask user to remove empty list in inspect
  - update compatible versions

dcor_control/cli/inspect.py

@@ -8,11 +8,13 @@
 @click.option('--assume-yes', is_flag=True)
 def inspect(assume_yes=False):
     """Inspect this DCOR installation"""
+    cn = 0
     click.secho("Checking CKAN options...", bold=True)
-    inspect_mod.check_ckan_ini(autocorrect=assume_yes)
+    cn += inspect_mod.check_ckan_ini(autocorrect=assume_yes)
 
     click.secho("Checking beaker session secret...", bold=True)
-    inspect_mod.check_ckan_beaker_session_cookie_secret(autocorrect=assume_yes)
+    cn += inspect_mod.check_ckan_beaker_session_cookie_secret(
+        autocorrect=assume_yes)
 
     click.secho("Checking www-data permissions...", bold=True)
     for path in [
@@ -24,15 +26,15 @@ def inspect(assume_yes=False):
         get_ckan_config_option("ckanext.dc_serve.tmp_dir"),
             ]:
         if path is not None:
-            inspect_mod.check_permission(
+            cn += inspect_mod.check_permission(
                 path=path,
                 user="www-data",
                 mode_dir=0o755,
                 mode_file=0o644,
                 recursive=False,
                 autocorrect=assume_yes)
 
-    inspect_mod.check_permission(
+    cn += inspect_mod.check_permission(
         path="/var/log/ckan",
         user="www-data",
         group="adm",
@@ -42,7 +44,7 @@ def inspect(assume_yes=False):
         autocorrect=assume_yes)
 
     # Recursively make sure that www-data can upload things into storage
-    inspect_mod.check_permission(
+    cn += inspect_mod.check_permission(
         path=paths.get_ckan_storage_path() / "storage",
         user="www-data",
         mode_dir=0o755,
@@ -51,27 +53,29 @@ def inspect(assume_yes=False):
         recursive=True)
 
     click.secho("Checking i18n hack...", bold=True)
-    inspect_mod.check_dcor_theme_i18n_hack(autocorrect=assume_yes)
+    cn += inspect_mod.check_dcor_theme_i18n_hack(autocorrect=assume_yes)
 
     click.secho("Checking DCOR theme css branding...", bold=True)
-    inspect_mod.check_dcor_theme_main_css(autocorrect=assume_yes)
+    cn += inspect_mod.check_dcor_theme_main_css(autocorrect=assume_yes)
 
     click.secho("Checking ckan workers...", bold=True)
-    inspect_mod.check_supervisord(autocorrect=assume_yes)
+    cn += inspect_mod.check_supervisord(autocorrect=assume_yes)
 
     click.secho("Checking nginx configuration...", bold=True)
-    inspect_mod.check_nginx(cmbs="100G", autocorrect=assume_yes)
+    cn += inspect_mod.check_nginx(cmbs="100G", autocorrect=assume_yes)
 
     click.secho("Checking uploader symlink patch...", bold=True)
-    inspect_mod.check_ckan_uploader_patch_to_support_symlinks(
+    cn += inspect_mod.check_ckan_uploader_patch_to_support_symlinks(
         autocorrect=assume_yes)
 
     click.secho("Checking uwsgi configuration...", bold=True)
-    inspect_mod.check_uwsgi(harakiri=7200, autocorrect=assume_yes)
-
-    inspect_mod.reload_supervisord()
+    cn += inspect_mod.check_uwsgi(harakiri=7200, autocorrect=assume_yes)
 
-    inspect_mod.reload_nginx()
+    if cn:
+        inspect_mod.reload_supervisord()
+        inspect_mod.reload_nginx()
+    else:
+        click.secho("No changes made.")
 
     # ask the user whether to search for orphaned files
     if assume_yes or click.confirm('Perform search for orphaned files?'):

dcor_control/inspect/common.py

@@ -39,6 +39,7 @@ def check_permission(path: str | pathlib.Path,
     autocorrect: bool
         whether to autocorrect permissions
     """
+    did_something = 0
     group = group or user
     uid = pwd.getpwnam(user).pw_uid if user is not None else None
     gid = grp.getgrnam(group).gr_gid if group is not None else None
@@ -51,13 +52,15 @@ def check_permission(path: str | pathlib.Path,
         mode = mode_dir
         if recursive:
             for pp in path.glob("*"):
-                check_permission(path=pp,
-                                 user=user,
-                                 group=group,
-                                 mode_dir=mode_dir,
-                                 mode_file=mode_file,
-                                 recursive=recursive,
-                                 autocorrect=autocorrect)
+                did_something += check_permission(
+                    path=pp,
+                    user=user,
+                    group=group,
+                    mode_dir=mode_dir,
+                    mode_file=mode_file,
+                    recursive=recursive,
+                    autocorrect=autocorrect
+                )
     else:
         # create a directory
         mode = mode_dir
@@ -67,6 +70,7 @@ def check_permission(path: str | pathlib.Path,
         else:
             create = ask(f"Directory '{path}' does not exist")
         if create:
+            did_something += 1
             path.mkdir(parents=True)
             if mode is not None:
                 os.chmod(path, mode)
@@ -83,6 +87,7 @@ def check_permission(path: str | pathlib.Path,
             change = ask(f"Mode of '{path}' is '{oct(pmode)}', "
                          f"but should be '{oct(mode)}'")
         if change:
+            did_something += 1
             os.chmod(path, mode)
 
     # Check owner
@@ -112,8 +117,11 @@ def check_permission(path: str | pathlib.Path,
                 chowner = ask(f"Owner of '{path}' is '{pusr}:{pgrp}', "
                               f"but should be '{user}:{group}'")
             if chowner:
+                did_something += 1
                 os.chown(path, uid, gid)
 
+    return did_something
+
 
 def recursive_update_dict(d, u):
     """Updates dict `d` with `u` recursively"""

dcor_control/inspect/config_ckan.py

@@ -22,13 +22,15 @@ def check_ckan_beaker_session_cookie_secret(autocorrect=False):
     value for this each time it generates a config file. When used in a
     cluster environment, the value must be the same on every machine.
     """
+    did_something = 0
     for key in ["beaker.session.encrypt_key",
                 "beaker.session.validate_key"]:
         opt = get_actual_ckan_option(key)
         if opt == "NOT SET!":
-            check_ckan_ini_option(key,
-                                  str(uuid.uuid4()),
-                                  autocorrect=autocorrect)
+            did_something += check_ckan_ini_option(key,
+                                                   str(uuid.uuid4()),
+                                                   autocorrect=autocorrect)
+    return did_something
 
 
 def check_ckan_ini(autocorrect=False):
@@ -40,18 +42,23 @@ def check_ckan_ini(autocorrect=False):
 
     Custom options override general options.
     """
+    did_something = 0
     custom_opts = get_expected_ckan_options()["ckan.ini"]
     general_opts = parse_ini_config(
         resource_filename("dcor_control.resources", "dcor_options.ini"))
 
     general_opts.update(custom_opts)
 
     for key in general_opts:
-        check_ckan_ini_option(key, general_opts[key], autocorrect=autocorrect)
+        did_something += check_ckan_ini_option(
+            key, general_opts[key], autocorrect=autocorrect)
+
+    return did_something
 
 
 def check_ckan_ini_option(key, value, autocorrect=False):
     """Check one server option"""
+    did_something = 0
     ckan_ini = get_ckan_config_path()
     opt = get_actual_ckan_option(key)
     if opt != value:
@@ -63,6 +70,8 @@ def check_ckan_ini_option(key, value, autocorrect=False):
         if change:
             ckan_cmd = f"ckan config-tool {ckan_ini} '{key}={value}'"
             sp.check_output(ckan_cmd, shell=True)
+            did_something += 1
+    return did_something
 
 
 def check_ckan_uploader_patch_to_support_symlinks(autocorrect):
@@ -75,6 +84,7 @@ def check_ckan_uploader_patch_to_support_symlinks(autocorrect):
 
     TODO: Check should be reversed once fully migrated to S3 upload scheme
     """
+    did_something = 0
     from ckan.lib import uploader
     ulpath = pathlib.Path(uploader.__file__)
     ulstr = ulpath.read_text()
@@ -97,6 +107,8 @@ def check_ckan_uploader_patch_to_support_symlinks(autocorrect):
         if hack:
             print("Disabling symlinks in Uploader")
             ulpath.write_text(ulstr)
+            did_something += 1
+    return did_something
 
 
 def check_dcor_theme_i18n_hack(autocorrect):
@@ -106,6 +118,7 @@ def check_dcor_theme_i18n_hack(autocorrect):
 
        ckan -c /etc/ckan/default/ckan.ini dcor-theme-i18n-branding
     """
+    did_something = 0
     ckan_ini = get_ckan_config_path()
     opt = get_actual_ckan_option("ckan.locale_default")
     if opt != "en_US":
@@ -119,8 +132,9 @@ def check_dcor_theme_i18n_hack(autocorrect):
             ckan_cmd = f"ckan -c {ckan_ini} dcor-theme-i18n-branding"
             sp.check_output(ckan_cmd, shell=True)
             # set config option
-            check_ckan_ini_option("ckan.locale_default", "en_US",
-                                  autocorrect=True)
+            did_something += check_ckan_ini_option(
+                "ckan.locale_default", "en_US", autocorrect=True)
+    return did_something
 
 
 def check_dcor_theme_main_css(autocorrect):
@@ -130,6 +144,7 @@ def check_dcor_theme_main_css(autocorrect):
 
         ckan -c /etc/ckan/default/ckan.ini dcor-theme-main-css-branding
      """
+    did_something = 0
     ckan_ini = get_ckan_config_path()
     opt = get_actual_ckan_option("ckan.theme")
     # TODO: Check whether the paths created by this script are setup correctly
@@ -144,10 +159,11 @@ def check_dcor_theme_main_css(autocorrect):
             ckan_cmd = f"ckan -c {ckan_ini} dcor-theme-main-css-branding"
             sp.check_output(ckan_cmd, shell=True)
             # set config option
-            check_ckan_ini_option(
+            did_something += check_ckan_ini_option(
                 key="ckan.theme",
                 value="dcor_theme_main/dcor_theme_main",
                 autocorrect=True)
+    return did_something
 
 
 def get_actual_ckan_option(key):

dcor_control/inspect/config_nginx.py

@@ -4,6 +4,7 @@
 
 
 def check_nginx(cmbs, autocorrect=False):
+    did_something = 0
     path_nginx = get_nginx_config_path()
     with open(path_nginx) as fd:
         lines = fd.readlines()
@@ -20,8 +21,11 @@ def check_nginx(cmbs, autocorrect=False):
                     correct = ask("'client_max_body_size' should be "
                                   + "'{}', but is '{}'".format(cmbs, cur))
                 if correct:
+                    did_something += 1
                     lines[ii] = line.replace(cur, cmbs)
                     path_nginx.write_text("\n".join(lines))
             break
     else:
         raise ValueError("'client_max_body_size' not set!")
+
+    return did_something

dcor_control/inspect/config_supervisord.py

@@ -10,6 +10,7 @@
 
 def check_supervisord(autocorrect):
     """Check whether the separate dcor worker files exist"""
+    did_something = 0
     path_worker = resource_filename(
         "dcor_control.resources.config",
         "etc_supervisor_conf.d_ckan-worker-dcor.conf")
@@ -25,8 +26,10 @@ def check_supervisord(autocorrect):
             else:
                 wcr = ask(f"Supervisord entry 'dcor-{worker}' missing")
             if wcr:
+                did_something += 1
                 data = template.replace("{{QUEUE}}", f"dcor-{worker}")
                 wpath.write_text(data)
+    return did_something
 
 
 def is_nginx_running():

dcor_control/inspect/config_uwsgi.py

@@ -11,6 +11,7 @@ def check_uwsgi(harakiri, autocorrect=False):
     harakiri: int
         uwsgi timeout in minutes
     """
+    did_something = 0
     path_uwsgi = get_uwsgi_config_path()
     with open(path_uwsgi) as fd:
         lines = fd.readlines()
@@ -32,6 +33,8 @@ def check_uwsgi(harakiri, autocorrect=False):
                         "UWSGI timeout should be '{}' min".format(harakiri)
                         + ", but is '{}' min".format(value))
                 if change:
+                    did_something += 1
                     lines[ii] = line.replace(str(value), str(harakiri))
                     with open(path_uwsgi, "w") as fd:
                         fd.writelines(lines)
+    return did_something