Merge lp:~therve/landscape-client/sysinfo-network-disks into lp:~landscape/landscape-client/trunk

[Thomas Herve]

[Kevin McDermott (bigkevmcd)]

Fairly trivial change, I'd still have preferred to see a config option that allowed users to add further FS types, but if needed that can go in a separate branch.

+1

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Looks good to me, +1!

[1]

+ def test_whitelist(self):

This test has no docstring.

review: Approve

=== modified file 'landscape/lib/disk.py'
--- landscape/lib/disk.py	2008-08-20 22:23:23 +0000
+++ landscape/lib/disk.py	2010-03-24 15:30:35 +0000
@@ -4,11 +4,14 @@
 import statvfs
 
 
-def get_mount_info(mounts_file, statvfs_):
+def get_mount_info(mounts_file, statvfs_, filesystems_whitelist=None):
     """
     Given a mounts file (e.g., /proc/mounts), generate dicts with the following
     keys:
 
+    @param filesystems_whitelist: if provided, the list of file systems which
+        we're allowed to stat.
+
      - device: The device file which is mounted.
      - mount-point: The path at which the filesystem is mounted.
      - filesystem: The filesystem type.
@@ -21,6 +24,9 @@
             mount_point = mount_point.decode("string-escape")
         except ValueError:
             continue
+        if (filesystems_whitelist is not None and
+            filesystem not in filesystems_whitelist):
+            continue
         megabytes = 1024 * 1024
         stats = statvfs_(mount_point)
         block_size = stats[statvfs.F_BSIZE]
@@ -31,11 +37,12 @@
                "free-space": free_space}
 
 
-def get_filesystem_for_path(path, mounts_file, statvfs_):
+def get_filesystem_for_path(path, mounts_file, statvfs_,
+                            filesystems_whitelist=None):
     candidate = None
     path = os.path.realpath(path)
     path_segments = path.split("/")
-    for info in get_mount_info(mounts_file, statvfs_):
+    for info in get_mount_info(mounts_file, statvfs_, filesystems_whitelist):
         mount_segments = info["mount-point"].split("/")
         if path.startswith(info["mount-point"]):
             if ((not candidate)

=== modified file 'landscape/lib/tests/test_disk.py'
--- landscape/lib/tests/test_disk.py	2009-11-06 13:41:30 +0000
+++ landscape/lib/tests/test_disk.py	2010-03-24 15:30:35 +0000
@@ -50,3 +50,12 @@
         info = get_filesystem_for_path(symlink_path,
                                        self.mount_file, self.statvfs)
         self.assertEquals(info["mount-point"], "/foo")
+
+    def test_whitelist(self):
+        self.set_mount_points(["/"])
+        info = get_filesystem_for_path(
+            "/", self.mount_file, self.statvfs, ["ext3"])
+        self.assertIdentical(info, None)
+        info = get_filesystem_for_path(
+            "/", self.mount_file, self.statvfs, ["ext3", "fsfs"])
+        self.assertNotIdentical(info, None)

=== modified file 'landscape/sysinfo/deployment.py'
--- landscape/sysinfo/deployment.py	2010-02-05 14:55:32 +0000
+++ landscape/sysinfo/deployment.py	2010-03-24 15:30:35 +0000
@@ -60,6 +60,7 @@
                            % (plugin_name.lower(), plugin_name))()
                 for plugin_name in plugins]
 
+
 def get_landscape_log_directory(landscape_dir=None):
     """
     Work out the correct path to store logs in depending on the effective
@@ -79,7 +80,7 @@
     logger.propagate = False
     if not os.path.isdir(landscape_dir):
         os.mkdir(landscape_dir)
-    log_filename = os.path.join(landscape_dir,  "sysinfo.log")
+    log_filename = os.path.join(landscape_dir, "sysinfo.log")
     handler = RotatingFileHandler(log_filename,
                                   maxBytes=500 * 1024, backupCount=1)
     logger.addHandler(handler)
@@ -113,11 +114,13 @@
         done = Deferred()
         reactor.callWhenRunning(
             lambda: maybeDeferred(run_sysinfo).chainDeferred(done))
+
         def stop_reactor(result):
             # We won't need to use callLater here once we use Twisted >8.
             # tm:3011
             reactor.callLater(0, reactor.stop)
             return result
+
         done.addBoth(stop_reactor)
         reactor.run()
     else:

=== modified file 'landscape/sysinfo/disk.py'
--- landscape/sysinfo/disk.py	2009-07-17 00:15:44 +0000
+++ landscape/sysinfo/disk.py	2010-03-24 15:30:35 +0000
@@ -7,9 +7,10 @@
 from landscape.lib.disk import get_mount_info, get_filesystem_for_path
 
 
-# List of filesystem types to exclude when generating disk use statistics.
-BORING_FILESYSTEMS = set(["udf", "iso9660", "fuse.gvfs-fuse-daemon",
-                          "squashfs", "ecryptfs"])
+# List of filesystem types authorized when generating disk use statistics.
+STABLE_FILESYSTEMS = set(
+    ["ext", "ext2", "ext3", "ext4", "reiserfs", "ntfs", "msdos", "dos", "vfat",
+     "xfs", "hpfs", "jfs", "ufs", "hfs", "hfsplus"])
 
 
 def format_megabytes(megabytes):
@@ -38,22 +39,28 @@
 
     def run(self):
         main_info = get_filesystem_for_path("/home", self._mounts_file,
-                                            self._statvfs)
-        total = main_info["total-space"]
-        if total <= 0:
-            main_info = get_filesystem_for_path("/", self._mounts_file,
-                                                self._statvfs)
+                                            self._statvfs, STABLE_FILESYSTEMS)
+        if main_info is not None:
             total = main_info["total-space"]
-        if total <= 0:
-            main_usage = "unknown"
+            if total <= 0:
+                root_main_info = get_filesystem_for_path(
+                    "/", self._mounts_file, self._statvfs, STABLE_FILESYSTEMS)
+                if root_main_info is not None:
+                    total = root_main_info["total-space"]
+                    main_info = root_main_info
+            if total <= 0:
+                main_usage = "unknown"
+            else:
+                main_usage = usage(main_info)
+            self._sysinfo.add_header("Usage of " + main_info["mount-point"],
+                                     main_usage)
         else:
-            main_usage = usage(main_info)
-        self._sysinfo.add_header("Usage of " + main_info["mount-point"],
-                                 main_usage)
+            self._sysinfo.add_header("Usage of /home", "unknown")
 
         seen_mounts = set()
         seen_devices = set()
-        infos = list(get_mount_info(self._mounts_file, self._statvfs))
+        infos = list(get_mount_info(self._mounts_file, self._statvfs,
+                                    STABLE_FILESYSTEMS))
         infos.sort(key=lambda i: len(i["mount-point"]))
         for info in infos:
             total = info["total-space"]
@@ -64,8 +71,6 @@
             if mount_seen or device_seen:
                 continue
 
-            if info["filesystem"] in BORING_FILESYSTEMS:
-                continue
             if total <= 0:
                 # Some "virtual" filesystems have 0 total space. ignore them.
                 continue

=== modified file 'landscape/sysinfo/tests/test_disk.py'
--- landscape/sysinfo/tests/test_disk.py	2009-08-07 22:36:33 +0000
+++ landscape/sysinfo/tests/test_disk.py	2010-03-24 15:30:35 +0000
@@ -4,6 +4,7 @@
 from landscape.sysinfo.disk import Disk, format_megabytes
 from landscape.tests.helpers import LandscapeTest
 
+
 class DiskTest(LandscapeTest):
 
     def setUp(self):
@@ -31,8 +32,10 @@
         result = self.disk.run()
         self.assertTrue(isinstance(result, Deferred))
         called = []
+
         def callback(result):
             called.append(True)
+
         result.addCallback(callback)
         self.assertTrue(called)
 
@@ -93,11 +96,14 @@
 
     def test_multiple_notes(self):
         """
-        A note will be displayed for each filesystem using 85% or more capacity.
+        A note will be displayed for each filesystem using 85% or more
+        capacity.
         """
         self.add_mount("/", block_size=1024, capacity=1000000, unused=150000)
-        self.add_mount("/use", block_size=2048, capacity=2000000, unused=200000)
-        self.add_mount("/emp", block_size=4096, capacity=3000000, unused=460000)
+        self.add_mount(
+            "/use", block_size=2048, capacity=2000000, unused=200000)
+        self.add_mount(
+            "/emp", block_size=4096, capacity=3000000, unused=460000)
         self.disk.run()
         self.assertEquals(self.sysinfo.get_notes(),
                           ["/ is using 85.0% of 976MB",
@@ -151,7 +157,7 @@
         self.assertEquals(self.sysinfo.get_notes(), [])
 
     def test_no_duplicate_roots(self):
-        self.add_mount("/", capacity=0, unused=0, fs="rootfs")
+        self.add_mount("/", capacity=0, unused=0, fs="ext4")
         self.add_mount("/", capacity=1000, unused=1, fs="ext3")
         self.disk.run()
         self.assertEquals(self.sysinfo.get_notes(),
@@ -206,3 +212,35 @@
         self.disk.run()
         self.assertEquals(self.sysinfo.get_notes(),
                           ["/ is using 100.0% of 3MB"])
+
+    def test_ignore_filesystems(self):
+        """
+        Network filesystems like nfs are ignored, because they can stall
+        randomly in stat.
+        """
+        self.add_mount("/", capacity=1000, unused=1000, fs="ext3")
+        self.add_mount("/mnt/disk1", capacity=1000, unused=0, fs="nfs")
+        self.disk.run()
+        self.assertEquals(self.sysinfo.get_notes(), [])
+
+    def test_nfs_as_root(self):
+        """
+        If / is not a whitelist filesystem, we don't report the usage of /home.
+        """
+        self.add_mount("/", capacity=1000, unused=1000, fs="nfs")
+        self.disk.run()
+        self.assertEquals(self.sysinfo.get_notes(), [])
+        self.assertEquals(self.sysinfo.get_headers(),
+                          [("Usage of /home", "unknown")])
+
+    def test_nfs_as_root_but_not_home(self):
+        """
+        If / is not a whitelist filesystem, but that /home is with a weird stat
+        value, we don't report the usage of /home.
+        """
+        self.add_mount("/", capacity=1000, unused=1000, fs="nfs")
+        self.add_mount("/home", capacity=0, unused=0, fs="ext3")
+        self.disk.run()
+        self.assertEquals(self.sysinfo.get_notes(), [])
+        self.assertEquals(self.sysinfo.get_headers(),
+                          [("Usage of /home", "unknown")])