Merge ~pjdc/ubuntu-mirror-charm/+git/ubuntu-mirror-charm:testability-2 into ubuntu-mirror-charm:master

[Paul Collins]

refactor, eliminating globals

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Haw Loeung (hloeung)]

LGTM

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 9e0ddc52aa26875a2a24a95944320a9df62aa1bc

diff --git a/hooks/hooks.py b/hooks/hooks.py
index fb28d13..c4d3d47 100755
--- a/hooks/hooks.py
+++ b/hooks/hooks.py
@@ -46,10 +46,6 @@ from Cheetah.Template import Template
 from Config import Config
 
 hooks = Hooks()
-hostname = socket.gethostname()
-fqdn = socket.getfqdn()
-execd_dir = os.path.join(charm_dir(), 'exec.d')
-conf = Config()
 
 apache_tls_settings = {
     'ssl_cipher_suite': 'EECDH+AESGCM+AES128:EDH+AESGCM+AES128:EECDH+AES128:EDH+AES128:ECDH+AESGCM+AES128:aRSA+AESGCM+AES128:ECDH+AES128:DH+AES128:aRSA+AES128:EECDH+AESGCM:EDH+AESGCM:EECDH:EDH:ECDH+AESGCM:aRSA+AESGCM:ECDH:DH:aRSA:HIGH:!MEDIUM:!aNULL:!NULL:!LOW:!3DES:!DSS:!EXP:!PSK:!SRP',  # noqa: E501
@@ -148,7 +144,7 @@ def check_output(cmd):
         return None
 
 
-def configure_scripts():
+def configure_scripts(conf, hostname):
     mkdir(conf.script_dir())
     for script in scripts_to_copy:
         src = os.path.join(charm_dir(), "files", script)
@@ -195,7 +191,7 @@ def get_ssh_keyfile(username):
 # Creation of any necessary destination directories is the responsibility
 # of the sync script specified in ${mirror}_command
 #
-def configure_rsync_client():  # noqa: C901
+def configure_rsync_client(conf, hostname):  # noqa: C901
     log("CHARM: Configuring rsync client")
     mirror_userinfo = conf.mirror_userinfo()
     mirror_confdir = os.path.join(mirror_userinfo.pw_dir, "mirror-config")
@@ -264,7 +260,7 @@ def configure_rsync_client():  # noqa: C901
 # config with our own to give connection limits and motd.
 # Change to update the file rather than overwriting.
 #
-def configure_rsync_server():
+def configure_rsync_server(conf, hostname):
     log("CHARM: Configuring rsync server")
 
     roles = conf.roles()
@@ -325,7 +321,7 @@ def get_role_config(roles, hostname, role):
 #
 # Set up apache config files - adding or removing vhosts as needed
 #
-def configure_apache():  # noqa: C901
+def configure_apache(conf, hostname):  # noqa: C901
     log("CHARM: Configuring apache")
     ensure_package_status(service_affecting_packages, conf.package_status())
     apache_dir = "/etc/apache2"
@@ -418,7 +414,7 @@ def configure_apache():  # noqa: C901
                 log("CHARM: Unable to delete vhost {}".format(vhost))
 
     # Set up server limits
-    restart_for_mpm = configure_apache_mpm()
+    restart_for_mpm = configure_apache_mpm(conf, hostname)
 
     # Configure status module
     tmpl_data = {}
@@ -455,14 +451,14 @@ def configure_apache():  # noqa: C901
         service_reload("apache2")
 
     # Notify any interested parties
-    configure_extra_mirrors()
+    configure_extra_mirrors(conf, hostname)
     log("CHARM: Finished configuring apache")
 
 
 # Set up the mpm config
 # 2.4 defaults to event
 # Returns True is something changed, False otherwise
-def configure_apache_mpm():  # noqa: C901
+def configure_apache_mpm(conf, hostname):  # noqa: C901
     log("CHARM: Configuring mpm")
     restart_required = False
     apache_version = get_pkg_version("apache2")[0:3]
@@ -548,7 +544,7 @@ def configure_apache_mpm():  # noqa: C901
 #
 # Set up vsftp
 #
-def configure_vsftp():
+def configure_vsftp(conf, hostname):
     log("CHARM: Configuring vsftp")
 
     # Set the ftp user's homedir where we want it
@@ -572,7 +568,7 @@ def configure_vsftp():
 #
 # Set up any external ssh sync triggers
 #
-def configure_triggers():
+def configure_triggers(conf, hostname):
     roles = conf.roles()
     if hostname not in roles:
         return
@@ -602,7 +598,7 @@ def configure_triggers():
     write_file(keyfile, contents)
 
 
-def configure_nrpe():  # noqa: C901
+def configure_nrpe(conf, hostname):  # noqa: C901
     if not conf.nagios_relation_ok():
         log("CHARM: Nagios relation not established - not configuring nrpe")
         return
@@ -665,6 +661,7 @@ def configure_nrpe():  # noqa: C901
             tmpl_data["rsync_crit"] = int(conf.rsync_max_connections() + 4)
             tmpl_data["rsync_warn"] = int(conf.rsync_max_connections() + 1)  # all client slots used
 
+            fqdn = socket.getfqdn()
             if role == "ports":
                 tmpl_data["tracepath"] = os.path.join(mirror["path"], "project", "trace", fqdn)
             else:
@@ -746,7 +743,7 @@ def configure_nrpe():  # noqa: C901
     service_reload('nagios-nrpe-server')
 
 
-def configure_log_archiving():
+def configure_log_archiving(conf, hostname):
     settings = {}
     logdirs = []
 
@@ -768,7 +765,7 @@ def configure_log_archiving():
             relation_set(r, settings)
 
 
-def configure_extra_mirrors():
+def configure_extra_mirrors(conf, hostname):
     settings = {}
     if is_relation_made("ubuntu-mirror"):
         settings["apache_logdir"] = conf.logdir("apache")
@@ -807,7 +804,7 @@ def is_lxc():
     return False
 
 
-def configure_sysctl():
+def configure_sysctl(conf, hostname):
     # In a container, these keys exist but can't be changed.  sysctl
     # seems to realize that if setting these fail as root, something
     # is beyond our control and exits 0.  Only add such keys here.
@@ -833,14 +830,14 @@ def configure_sysctl():
         sysctl.create(yaml.dump(non_container_settings), '/etc/sysctl.d/90-ubuntu-mirror-non-container.conf')
 
 
-def configure_user():
+def configure_user(conf, hostname):
     if not user_exists(conf.mirror_user()):
         mirror_userinfo = adduser(conf.mirror_user(), system_user=True)
         if not os.path.isdir(mirror_userinfo.pw_dir):
             mkdir(mirror_userinfo.pw_dir, owner=conf.mirror_user())
 
 
-def configure_directories():
+def configure_directories(conf, hostname):
     if not os.path.isdir("/srv/ftp.root"):
         mkdir("/srv/ftp.root")
     chownr("/srv/ftp.root", conf.mirror_user(), conf.mirror_user(), follow_links=False)
@@ -848,66 +845,80 @@ def configure_directories():
 
 @hooks.hook("install.real")
 def install():
+    conf = Config()
+    hostname = socket.gethostname()
     log("CHARM: Installing {}".format(conf.app_name()))
     apt_install(required_pkgs, options=['--force-yes'])
-    configure_user()
-    configure_scripts()
-    configure_directories()
+    configure_user(conf, hostname)
+    configure_scripts(conf, hostname)
+    configure_directories(conf, hostname)
     for module in apache_modules:
         check_call(["/usr/sbin/a2enmod", module])
 
 
 @hooks.hook("config-changed")
 def config_changed():
+    conf = Config()
+    hostname = socket.gethostname()
     log("CHARM: Configuring {}".format(conf.app_name()))
-    configure_user()
-    configure_scripts()
-    configure_directories()
-    configure_rsync_client()
-    configure_rsync_server()
-    configure_apache()
-    configure_vsftp()
-    configure_triggers()
-    configure_nrpe()
-    configure_log_archiving()
-    configure_sysctl()
+    configure_user(conf, hostname)
+    configure_scripts(conf, hostname)
+    configure_directories(conf, hostname)
+    configure_rsync_client(conf, hostname)
+    configure_rsync_server(conf, hostname)
+    configure_apache(conf, hostname)
+    configure_vsftp(conf, hostname)
+    configure_triggers(conf, hostname)
+    configure_nrpe(conf, hostname)
+    configure_log_archiving(conf, hostname)
+    configure_sysctl(conf, hostname)
 
 
 @hooks.hook("upgrade-charm")
 def upgrade_charm():
+    conf = Config()
     log("CHARM: Upgrading {}".format(conf.app_name()))
     install()
 
 
 @hooks.hook("nrpe-external-master-relation-changed")
 def nrpe_external_master_relation_changed():
+    conf = Config()
+    hostname = socket.gethostname()
     log("CHARM: Changing nrpe relation for {}".format(conf.app_name()))
-    configure_nrpe()
-    configure_extra_mirrors()
+    configure_nrpe(conf, hostname)
+    configure_extra_mirrors(conf, hostname)
 
 
 @hooks.hook("local-monitors-relation-changed")
 def local_monitors_relation_changed():
+    conf = Config()
+    hostname = socket.gethostname()
     log("CHARM: Changing monitors relation for {}".format(conf.app_name()))
-    configure_nrpe()
+    configure_nrpe(conf, hostname)
 
 
 @hooks.hook("log-archive-relation-changed")
 def log_archive_relation_changed():
+    conf = Config()
+    hostname = socket.gethostname()
     log("CHARM: Changing log-archive relation for {}".format(conf.app_name()))
-    configure_log_archiving()
+    configure_log_archiving(conf, hostname)
 
 
 @hooks.hook("ubuntu-mirror-relation-changed")
 def ubuntu_mirror_relation_changed():
+    conf = Config()
+    hostname = socket.gethostname()
     log("CHARM: Changing ubuntu-mirror relation for {}".format(conf.app_name()))
-    configure_extra_mirrors()
+    configure_extra_mirrors(conf, hostname)
 
 
 if __name__ == "__main__":
     hook_name = os.path.basename(sys.argv[0])
     if hook_name.endswith('.real'):
         hook_name = hook_name[:-5]
+    execd_dir = os.path.join(charm_dir(), 'exec.d')
     execd_run(('charm-pre-%s' % hook_name), execd_dir=execd_dir)
     hooks.execute(sys.argv)
     execd_run(('charm-post-%s' % hook_name), execd_dir=execd_dir)