Merge lp:~ajkavanagh/charm-helpers/clean-up-render into lp:charm-helpers

[Alex Kavanagh]

Update the core.host.write_file(...) function to do less work

The primary motivation for this change is to do less work, touch the
filesystem less, and try to clean the logs up a bit. This change is
to the rendering of configuration files, which generates noise in the
logs (at INFO) level, and often results in writing the exact same
content, thus updating the atime of the file even when the contents
haven't changed.

This change detects if the content will change, and if not, doesn't
write the file. It does detect if the uid,gid is changing and takes
the appropriate action. The logs are demoted to the lowest level of
DEBUG/TRACE, and are only generated IFF the file is actually written
or the uid/gid is changed.

[David Ames (thedac)]

This looks good.

Before final approval I'd like to see it in one of our charms running an amulet full.

I realize this is a bit chicken and egg. But you can hijack the charm-helpers-hooks.yaml for this purpose.

[David Ames (thedac)]

Tests from the glance example pass.

Merging.

review: Approve

=== modified file 'charmhelpers/contrib/openstack/templating.py'
--- charmhelpers/contrib/openstack/templating.py	2017-02-16 03:04:17 +0000
+++ charmhelpers/contrib/openstack/templating.py	2017-08-10 16:11:24 +0000
@@ -20,7 +20,8 @@
 from charmhelpers.core.hookenv import (
     log,
     ERROR,
-    INFO
+    INFO,
+    TRACE
 )
 from charmhelpers.contrib.openstack.utils import OPENSTACK_CODENAMES
 
@@ -80,8 +81,10 @@
             loaders.insert(0, FileSystemLoader(tmpl_dir))
         if rel == os_release:
             break
+    # demote this log to the lowest level; we don't really need to see these
+    # lots in production even when debugging.
     log('Creating choice loader with dirs: %s' %
-        [l.searchpath for l in loaders], level=INFO)
+        [l.searchpath for l in loaders], level=TRACE)
     return ChoiceLoader(loaders)
 
 

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2017-08-01 10:53:23 +0000
+++ charmhelpers/core/hookenv.py	2017-08-10 16:11:24 +0000
@@ -43,6 +43,7 @@
 WARNING = "WARNING"
 INFO = "INFO"
 DEBUG = "DEBUG"
+TRACE = "TRACE"
 MARKER = object()
 
 cache = {}

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2017-05-20 06:05:44 +0000
+++ charmhelpers/core/host.py	2017-08-10 16:11:24 +0000
@@ -34,7 +34,7 @@
 
 from contextlib import contextmanager
 from collections import OrderedDict
-from .hookenv import log
+from .hookenv import log, DEBUG
 from .fstab import Fstab
 from charmhelpers.osplatform import get_platform
 
@@ -487,13 +487,37 @@
 
 def write_file(path, content, owner='root', group='root', perms=0o444):
     """Create or overwrite a file with the contents of a byte string."""
-    log("Writing file {} {}:{} {:o}".format(path, owner, group, perms))
     uid = pwd.getpwnam(owner).pw_uid
     gid = grp.getgrnam(group).gr_gid
-    with open(path, 'wb') as target:
-        os.fchown(target.fileno(), uid, gid)
-        os.fchmod(target.fileno(), perms)
-        target.write(content)
+    # lets see if we can grab the file and compare the context, to avoid doing
+    # a write.
+    existing_content = None
+    existing_uid, existing_gid = None, None
+    try:
+        with open(path, 'rb') as target:
+            existing_content = target.read()
+        stat = os.stat(path)
+        existing_uid, existing_gid = stat.st_uid, stat.st_gid
+    except:
+        pass
+    if content != existing_content:
+        log("Writing file {} {}:{} {:o}".format(path, owner, group, perms),
+            level=DEBUG)
+        with open(path, 'wb') as target:
+            os.fchown(target.fileno(), uid, gid)
+            os.fchmod(target.fileno(), perms)
+            target.write(content)
+        return
+    # the contents were the same, but we might still need to change the
+    # ownership.
+    if existing_uid != uid:
+        log("Changing uid on already existing content: {} -> {}"
+            .format(existing_uid, uid), level=DEBUG)
+        os.chown(path, uid, -1)
+    if existing_gid != gid:
+        log("Changing gid on already existing content: {} -> {}"
+            .format(existing_gid, gid), level=DEBUG)
+        os.chown(path, -1, gid)
 
 
 def fstab_remove(mp):