Merge lp:~psusi/landscape-client/correct-free-mem into lp:~landscape/landscape-client/trunk

[Phillip Susi]

landscape-sysinfo was reporting memory usage values that
were entirely incorrect. This was because it was calculating free
memory to be all memory not on the active list. Changed to use
free + cached + buffers, just like the free command does.

Corrected test suite.

[Andreas Hasenack (ahasenack)]

There are a bunch of test failures, can you take a look at those?

[FAIL]
Traceback (most recent call last):
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/tests/mocker.py", line 146, in test_method_wrapper
    result = test_method()
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/lib/tests/test_sysstats.py", line 42, in test_get_memory_info
    self.assertEqual(memstats.free_memory, 503)
  File "/usr/lib/python2.7/dist-packages/twisted/trial/unittest.py", line 271, in assertEqual
    % (msg, pformat(first), pformat(second)))
twisted.trial.unittest.FailTest: not equal:
a = 852
b = 503

landscape.lib.tests.test_sysstats.MemoryStatsTest.test_get_memory_info
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/tests/mocker.py", line 146, in test_method_wrapper
    result = test_method()
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/monitor/tests/test_memoryinfo.py", line 154, in test_exchange_messages
    (step_size * 2, 503, 1567)]}])
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/tests/helpers.py", line 82, in assertMessages
    self.assertMessage(obtained_message, expected_message)
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/tests/helpers.py", line 76, in assertMessage
    pprint.pformat(obtained)))
twisted.trial.unittest.FailTest: Messages don't match.
Expected:
{'memory-info': [(300, 503, 1567), (600, 503, 1567)], 'type': 'memory-info'}
Obtained:
{'memory-info': [(300, 852, 1567), (600, 852, 1567)], 'type': 'memory-info'}

landscape.monitor.tests.test_memoryinfo.MemoryInfoTest.test_exchange_messages
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/tests/mocker.py", line 146, in test_method_wrapper
    result = test_method()
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/monitor/tests/test_memoryinfo.py", line 78, in test_read_sample_data
    self.assertEqual(message["memory-info"][0], (step_size, 503, 1567))
  File "/usr/lib/python2.7/dist-packages/twisted/trial/unittest.py", line 271, in assertEqual
    % (msg, pformat(first), pformat(second)))
twisted.trial.unittest.FailTest: not equal:
a = (300, 852, 1567)
b = (300, 503, 1567)

landscape.monitor.tests.test_memoryinfo.MemoryInfoTest.test_read_sample_data
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/tests/mocker.py", line 146, in test_method_wrapper
    result = test_method()
  File "/home/andreas/canonical/source/landscape-client/psusi/correct-free-mem/landscape/sysinfo/tests/test_memory.py", line 53, in test_run_adds_header
    ("Swap usage", "39%")])
  File...

[Jerry Seutter (jseutter)]

+1, this makes landscape-sysinfo report more accurate memory usage.

Without the change:
jseutter@perth:~/src/landscape-client/trunk$ ./scripts/landscape-sysinfo
  System load: 0.82 Users logged in: 1
  Usage of /: 58.1% of 228.21GB IP address for eth0: 192.168.2.138
  Memory usage: 42% IP address for lxcbr0: 10.0.3.1
  Swap usage: 0% IP address for wlan0: 192.168.2.100
  Processes: 238

With the change:
jseutter@perth:~/src/landscape-client/correct-free-mem$ ./scripts/landscape-sysinfo
  System load: 0.92 Users logged in: 1
  Usage of /: 58.1% of 228.21GB IP address for eth0: 192.168.2.138
  Memory usage: 22% IP address for lxcbr0: 10.0.3.1
  Swap usage: 0% IP address for wlan0: 192.168.2.100
  Processes: 238

This matches the output of top, free and The Internet. It differs from the free command by 1%, but I assume this is because we truncate the value somewhere.

I'm getting two PEP8 warnings, can you clean these up?
=== pep8 ===

landscape/lib/sysstats.py:16:80: E501 line too long (96 characters)
landscape/lib/sysstats.py:20:80: E501 line too long (87 characters)

[lp:~psusi/landscape-client/correct-free-mem]

594. By Phillip Susi on 2012-12-27

landscape-sysinfo was reporting memory usage values that
were entirely incorrect. This was because it was calculating free
memory to be all memory not on the active list. Changed to use
free + cached + buffers, just like the free command does.

[Phillip Susi (psusi)]

How's this?

[Jerry Seutter (jseutter)]

> How's this?

Looks good. Thanks!

[Geoff Teale (tealeg)]

+1 Nice change!

review: Approve

[Jerry Seutter (jseutter)]

+1

review: Approve

[Chris Glass (tribaal)]

Marking this as Approved to let tarmac pick this up and merge it in.

=== modified file 'landscape/lib/sysstats.py'
--- landscape/lib/sysstats.py	2010-04-28 13:07:53 +0000
+++ landscape/lib/sysstats.py	2012-12-27 20:12:21 +0000
@@ -13,11 +13,13 @@
         for line in open(filename):
             if ":" in line:
                 key, value = line.split(":", 1)
-                if key in ["Active", "MemTotal", "SwapFree", "SwapTotal"]:
+                if key in ["MemTotal", "SwapFree", "SwapTotal", "MemFree",
+                           "Buffers", "Cached"]:
                     data[key] = int(value.split()[0])
 
         self.total_memory = data["MemTotal"] // 1024
-        self.free_memory = (data["MemTotal"] - data["Active"]) // 1024
+        self.free_memory = (data["MemFree"] + data["Buffers"] +
+                            data["Cached"]) // 1024
         self.total_swap = data["SwapTotal"] // 1024
         self.free_swap = data["SwapFree"] // 1024
 

=== modified file 'landscape/lib/tests/test_sysstats.py'
--- landscape/lib/tests/test_sysstats.py	2011-07-05 05:09:11 +0000
+++ landscape/lib/tests/test_sysstats.py	2012-12-27 20:12:21 +0000
@@ -39,14 +39,14 @@
         filename = self.makeFile(SAMPLE_MEMORY_INFO)
         memstats = MemoryStats(filename)
         self.assertEqual(memstats.total_memory, 1510)
-        self.assertEqual(memstats.free_memory, 503)
-        self.assertEqual(memstats.used_memory, 1007)
+        self.assertEqual(memstats.free_memory, 852)
+        self.assertEqual(memstats.used_memory, 658)
         self.assertEqual(memstats.total_swap, 1584)
         self.assertEqual(memstats.free_swap, 1567)
         self.assertEqual(memstats.used_swap, 17)
-        self.assertEqual("%.2f" % memstats.free_memory_percentage, "33.31")
+        self.assertEqual("%.2f" % memstats.free_memory_percentage, "56.42")
         self.assertEqual("%.2f" % memstats.free_swap_percentage, "98.93")
-        self.assertEqual("%.2f" % memstats.used_memory_percentage, "66.69")
+        self.assertEqual("%.2f" % memstats.used_memory_percentage, "43.58")
         self.assertEqual("%.2f" % memstats.used_swap_percentage, "1.07")
 
     def test_get_memory_info_without_swap(self):

=== modified file 'landscape/monitor/tests/test_memoryinfo.py'
--- landscape/monitor/tests/test_memoryinfo.py	2011-07-05 05:09:11 +0000
+++ landscape/monitor/tests/test_memoryinfo.py	2012-12-27 20:12:21 +0000
@@ -75,7 +75,7 @@
         self.reactor.advance(step_size)
 
         message = plugin.create_message()
-        self.assertEqual(message["memory-info"][0], (step_size, 503, 1567))
+        self.assertEqual(message["memory-info"][0], (step_size, 852, 1567))
 
     def test_messaging_flushes(self):
         """
@@ -150,8 +150,8 @@
 
         self.assertMessages(self.mstore.get_pending_messages(),
                             [{"type": "memory-info",
-                              "memory-info": [(step_size, 503, 1567),
-                                              (step_size * 2, 503, 1567)]}])
+                              "memory-info": [(step_size, 852, 1567),
+                                              (step_size * 2, 852, 1567)]}])
 
     def test_call_on_accepted(self):
         plugin = MemoryInfo(source_filename=self.makeFile(self.SAMPLE_DATA),

=== modified file 'landscape/sysinfo/tests/test_memory.py'
--- landscape/sysinfo/tests/test_memory.py	2011-07-05 05:09:11 +0000
+++ landscape/sysinfo/tests/test_memory.py	2012-12-27 20:12:21 +0000
@@ -49,5 +49,5 @@
     def test_run_adds_header(self):
         self.memory.run()
         self.assertEqual(self.sysinfo.get_headers(),
-                         [("Memory usage", "34%"),
+                         [("Memory usage", "27%"),
                           ("Swap usage", "39%")])