Merge ~pjdc/ubuntu-mirror-charm/+git/ubuntu-mirror-charm:symlink-following into ubuntu-mirror-charm:master

[Paul Collins]

stop creating extra symlinks in mirror directories

[🤖 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 3ee1f300437dcb10eb7620358e717e207bfaae47

diff --git a/hooks/hooks.py b/hooks/hooks.py
index 2757b35..4c272f4 100755
--- a/hooks/hooks.py
+++ b/hooks/hooks.py
@@ -5,6 +5,7 @@
 
 import base64
 import datetime
+import errno
 import json
 import os
 import os.path
@@ -24,7 +25,6 @@ from charmhelpers.core.host import (
     mkdir,
     service_reload,
     service_restart,
-    symlink,
     user_exists,
     write_file,
 )
@@ -103,6 +103,29 @@ GET_DEFAULT_ADDRESS_DESTINATIONS = {
 }
 
 
+def ensure_symlink(target, link_path):
+    """Ensure that link_path is a symlink to target.  Do not follow link_path.
+    Raise an exception if link_path exists but is not a symlink."""
+    need_unlink = False
+    try:
+        current = os.readlink(link_path)
+        if current == target:
+            log("Skipping {} - symlink exists and already points to {}".format(link_path, current))
+            return
+        need_unlink = True
+    except OSError as ose:
+        if ose.errno != errno.ENOENT:
+            raise
+    except Exception:
+        raise
+
+    # There's no non-racy way to replace a symlink.  Even `ls -sfn target link_path` does this.
+    if need_unlink:
+        os.unlink(link_path)
+    os.symlink(target, link_path)
+    log("Symlink {} at {}, pointing to {}".format("replaced" if need_unlink else "created", link_path, target))
+
+
 def get_default_address(af):
     """Determine our default address for the given address family."""
     try:
@@ -486,7 +509,7 @@ def configure_apache(conf, hostname):  # noqa: C901
             tmpl_data["ports"] = [80, 443] if role_config.get('https') else [80]
             all_ports.update(tmpl_data["ports"])
             file_from_template(mirror['tmpl_file'], sites_available, tmpl_data)
-            symlink(sites_available, sites_enabled)
+            ensure_symlink(sites_available, sites_enabled)
 
             # And make sure the docroot exists
             if role == "ubuntu":
@@ -502,7 +525,7 @@ def configure_apache(conf, hostname):  # noqa: C901
             mkdir(mirror["path"])
             os.chmod(mirror["path"], 0o755)
             os.chown(mirror["path"], mirror_userinfo.pw_uid, mirror_userinfo.pw_gid)
-            symlink(mirror["path"], linkdest)
+            ensure_symlink(mirror["path"], linkdest)
 
     # Update ports file
     all_addresses.discard('*')  # archive.ubuntu.com must not open port 443.