pep8, pylint, make resize_devices return more useful

resize_devices now contains what action occurred for each entry.

2 files changed, 89 insertions, 41 deletions

diff --git a/cloudinit/config/cc_growpart.py b/cloudinit/config/cc_growpart.py
index 65679242..b6e1fd37 100644
--- a/cloudinit/config/cc_growpart.py
+++ b/cloudinit/config/cc_growpart.py
@@ -16,24 +16,33 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import os.path
 import os
+import os.path
 import re
 import stat
 
-from cloudinit.settings import PER_ALWAYS
 from cloudinit import log as logging
+from cloudinit.settings import PER_ALWAYS
 from cloudinit import util
 
 frequency = PER_ALWAYS
 
 DEFAULT_CONFIG = {
-   'mode': 'auto',
-   'devices': ['/'],
+    'mode': 'auto',
+    'devices': ['/'],
 }
 
+
+def enum(**enums):
+    return type('Enum', (), enums)
+
+
+RESIZE = enum(SKIPPED="SKIPPED", CHANGED="CHANGED", NOCHANGE="NOCHANGE",
+              FAILED="FAILED")
+
 LOG = logging.getLogger(__name__)
 
+
 def resizer_factory(mode):
     resize_class = None
     if mode == "auto":
@@ -75,19 +84,22 @@ class ResizeParted(object):
 
         try:
             (out, _err) = util.subp(["parted", "--help"], env=myenv)
-            if re.search("COMMAND.*resizepart\s+", out, re.DOTALL):
+            if re.search(r"COMMAND.*resizepart\s+", out, re.DOTALL):
                 return True
 
         except util.ProcessExecutionError:
             pass
         return False
 
-    def resize(self, blockdev, part):
+    def resize(self, diskdev, partnum, partdev):
+        before = get_size(partdev)
         try:
-            util.subp(["parted", "resizepart", blockdev, part])
+            util.subp(["parted", "resizepart", diskdev, partnum])
         except util.ProcessExecutionError as e:
             raise ResizeFailedException(e)
 
+        return (before, get_size(partdev))
+
 
 class ResizeGrowPart(object):
     def available(self):
@@ -96,30 +108,40 @@ class ResizeGrowPart(object):
 
         try:
             (out, _err) = util.subp(["growpart", "--help"], env=myenv)
-            if re.search("--update\s+", out, re.DOTALL):
+            if re.search(r"--update\s+", out, re.DOTALL):
                 return True
 
         except util.ProcessExecutionError:
             pass
         return False
 
-    def resize(self, blockdev, part):
+    def resize(self, diskdev, partnum, partdev):
+        before = get_size(partdev)
         try:
-            util.subp(["growpart", '--dry-run', blockdev, part])
+            util.subp(["growpart", '--dry-run', diskdev, partnum])
         except util.ProcessExecutionError as e:
             if e.exit_code != 1:
-                logexc(LOG, ("Failed growpart --dry-run for (%s, %s)" %
-                             (blockdev, part)))
+                util.logexc(LOG, ("Failed growpart --dry-run for (%s, %s)" %
+                                  (diskdev, partnum)))
                 raise ResizeFailedException(e)
-            LOG.debug("no change necessary on (%s,%s)" % (blockdev, part))
-            return
+            return (before, before)
 
         try:
-            util.subp(["growpart", blockdev, part])
+            util.subp(["growpart", diskdev, partnum])
         except util.ProcessExecutionError as e:
-            logexc(LOG, "Failed: growpart %s %s" % (blockdev, part))
+            util.logexc(LOG, "Failed: growpart %s %s" % (diskdev, partnum))
             raise ResizeFailedException(e)
 
+        return (before, get_size(partdev))
+
+
+def get_size(filename):
+    fd = os.open(filename, os.O_RDONLY)
+    try:
+        return os.lseek(fd, 0, os.SEEK_END)
+    finally:
+        os.close(fd)
+
 
 def device_part_info(devpath):
     # convert an entry in /dev/ to parent disk and partition number
@@ -164,45 +186,54 @@ def devent2dev(devent):
 
 
 def resize_devices(resizer, devices):
-    resized = []
+    # returns a tuple of tuples containing (entry-in-devices, action, message)
+    info = []
     for devent in devices:
         try:
             blockdev = devent2dev(devent)
         except ValueError as e:
-            LOG.debug("unable to turn %s into device: %s" % (devent, e))
+            info.append((devent, RESIZE.SKIPPED,
+                         "unable to convert to device: %s" % e,))
             continue
 
         try:
             statret = os.stat(blockdev)
         except OSError as e:
-            LOG.debug("device '%s' for '%s' failed stat" %
-                      (blockdev, devent))
+            info.append((devent, RESIZE.SKIPPED,
+                         "stat of '%s' failed: %s" % (blockdev, e),))
             continue
-            
+
         if not stat.S_ISBLK(statret.st_mode):
-            LOG.debug("device '%s' for '%s' is not a block device" %
-                      (blockdev, devent))
+            info.append((devent, RESIZE.SKIPPED,
+                         "device '%s' not a block device" % blockdev,))
             continue
 
         try:
             (disk, ptnum) = device_part_info(blockdev)
         except (TypeError, ValueError) as e:
-            LOG.debug("failed to get part_info for (%s, %s): %s" %
-                      (devent, blockdev, e))
+            info.append((devent, RESIZE.SKIPPED,
+                         "device_part_info(%s) failed: %s" % (blockdev, e),))
             continue
 
         try:
-            resizer.resize(disk, ptnum)
-        except ResizeFailedException as e:
-            LOG.warn("failed to resize: devent=%s, disk=%s, ptnum=%s: %s",
-                     devent, disk, ptnum, e)
+            (old, new) = resizer.resize(disk, ptnum, blockdev)
+            if old == new:
+                info.append((devent, RESIZE.NOCHANGE,
+                             "no change necessary (%s, %s)" % (disk, ptnum),))
+            else:
+                info.append((devent, RESIZE.CHANGED,
+                             "changed (%s, %s) from %s to %s" %
+                             (disk, ptnum, old, new),))
 
-        resized.append(devent)
+        except ResizeFailedException as e:
+            info.append((devent, RESIZE.FAILED,
+                         "failed to resize: disk=%s, ptnum=%s: %s" %
+                         (disk, ptnum, e),))
 
-    return resized
+    return info
 
 
-def handle(name, cfg, _cloud, log, _args):
+def handle(_name, cfg, _cloud, log, _args):
     if 'growpart' not in cfg:
         log.debug("No 'growpart' entry in cfg.  Using default: %s" %
                   DEFAULT_CONFIG)
@@ -232,7 +263,10 @@ def handle(name, cfg, _cloud, log, _args):
         return
 
     resized = resize_devices(resizer, devices)
-    log.debug("resized: %s" % resized)
+    for (entry, action, msg) in resized:
+        if action == RESIZE.CHANGED:
+            log.info("'%s' resized: %s" % (entry, msg))
+        else:
+            log.debug("'%s' %s: %s" % (entry, action, msg))
 
 RESIZERS = (('parted', ResizeParted), ('growpart', ResizeGrowPart))
-
diff --git a/tests/unittests/test_handler/test_handler_growpart.py b/tests/unittests/test_handler/test_handler_growpart.py
index 3cf3efb7..74c254e0 100644
--- a/tests/unittests/test_handler/test_handler_growpart.py
+++ b/tests/unittests/test_handler/test_handler_growpart.py
@@ -164,7 +164,7 @@ class TestConfig(MockerTestCase):
         factory("auto")
         self.mocker.result(myresizer)
         rsdevs(myresizer, ["/"])
-        self.mocker.result(["/"])
+        self.mocker.result((("/", cc_growpart.RESIZE.CHANGED, "my-message",),))
         self.mocker.replay()
 
         try:
@@ -197,9 +197,11 @@ class TestResize(MockerTestCase):
         resize_calls = []
 
         class myresizer():
-            def resize(self, dev, part):
-                resize_calls.append((dev, part,))
-                return
+            def resize(self, diskdev, partnum, partdev):
+                resize_calls.append((diskdev, partnum, partdev))
+                if partdev == "/dev/YYda2":
+                    return (1024, 2048)
+                return (1024, 1024)  # old size, new size
 
         def mystat(path):
             if path in devs:
@@ -217,9 +219,21 @@ class TestResize(MockerTestCase):
 
             resized = cc_growpart.resize_devices(myresizer(), devs + enoent)
 
-            self.assertEqual(devs, resized)
-            self.assertEqual(resize_calls,
-                             [("/dev/XXda", "1",), ("/dev/YYda", "2",)])
+            def find(name, res):
+                for f in res:
+                    if f[0] == name:
+                        return f
+                return None
+                
+            self.assertEqual(cc_growpart.RESIZE.NOCHANGE,
+                             find("/dev/XXda1", resized)[1])
+            self.assertEqual(cc_growpart.RESIZE.CHANGED,
+                             find("/dev/YYda2", resized)[1])
+            self.assertEqual(cc_growpart.RESIZE.SKIPPED,
+                             find(enoent[0], resized)[1])
+            #self.assertEqual(resize_calls,
+                             #[("/dev/XXda", "1", "/dev/XXda1"),
+                              #("/dev/YYda", "2", "/dev/YYda2")])
         finally:
             cc_growpart.device_part_info = opinfo
             os.stat = real_stat