Merge lp:~ack/landscape-client/fix-mountinfo-tests-in-container into lp:~landscape/landscape-client/trunk

[Alberto Donato]

Fix the following MountInfo unittests that fail in containers (LXC and LXD) because of lack of isolation from the system:

landscape.monitor.tests.test_mountinfo.MountInfoTest.test_read_proc_mounts
landscape.monitor.tests.test_mountinfo.MountInfoTest.test_call_on_accepted

Fix the following MountInfo unittests that fail in containers (LXC and LXD) because of lack of isolation from the system:

landscape.monitor.tests.test_mountinfo.MountInfoTest.test_read_proc_mounts
landscape.monitor.tests.test_mountinfo.MountInfoTest.test_call_on_accepted

Testing instructions:

In a container, run trial landscape.monitor.tests.test_mountinfo

[lp:~ack/landscape-client/fix-mountinfo-tests-in-container]

935. By Alberto Donato on 2017-02-22

Reapply change.

936. By Alberto Donato on 2017-02-22

Refactor some other tests.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 936
Branch: lp:~ack/landscape-client/fix-mountinfo-tests-in-container
Jenkins: https://ci.lscape.net/job/latch-test-precise/866/

review: Approve (test results)

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 936
Branch: lp:~ack/landscape-client/fix-mountinfo-tests-in-container
Jenkins: https://ci.lscape.net/job/latch-test-precise/865/

review: Approve (test results)

[Adam Collard (adam-collard)]

+1

review: Approve

[Данило Шеган (danilo)]

Nice clean-up: it's funny how all these tests had slightly different set-up, yet didn't really depend on it. Approved as such, but...

...I am not getting any test failures even without this change in a xenial container. My system only has /dev/sd* disks, so maybe that's why it works (i.e. /dev/hd* does not conflict)? Is there a bug that has more info?

review: Approve

[Alberto Donato (ack)]

> Nice clean-up: it's funny how all these tests had slightly different set-up,
> yet didn't really depend on it. Approved as such, but...
>
> ...I am not getting any test failures even without this change in a xenial
> container. My system only has /dev/sd* disks, so maybe that's why it works
> (i.e. /dev/hd* does not conflict)? Is there a bug that has more info?

No, I have been seeing those two tests failing for a long time in my LXC/LXDs, maybe it's peculiar to my setup.
Anyway, isolating tests from the system sounds like a good idea in general :)

[Alberto Donato (ack)]

> Nice clean-up: it's funny how all these tests had slightly different set-up,
> yet didn't really depend on it. Approved as such, but...
>
> ...I am not getting any test failures even without this change in a xenial
> container. My system only has /dev/sd* disks, so maybe that's why it works
> (i.e. /dev/hd* does not conflict)? Is there a bug that has more info?

No, I have been seeing those two tests failing for a long time in my LXC/LXDs, maybe it's peculiar to my setup.
Anyway, isolating tests from the system sounds like a good idea in general :)

[Alberto Donato (ack)]

> Nice clean-up: it's funny how all these tests had slightly different set-up,
> yet didn't really depend on it. Approved as such, but...
>
> ...I am not getting any test failures even without this change in a xenial
> container. My system only has /dev/sd* disks, so maybe that's why it works
> (i.e. /dev/hd* does not conflict)? Is there a bug that has more info?

No, I have been seeing those two tests failing for a long time in my LXC/LXDs, maybe it's peculiar to my setup.
Anyway, isolating tests from the system sounds like a good idea in general :)

=== modified file 'landscape/monitor/tests/test_mountinfo.py'
--- landscape/monitor/tests/test_mountinfo.py	2016-06-15 21:29:53 +0000
+++ landscape/monitor/tests/test_mountinfo.py	2017-02-22 10:56:00 +0000
@@ -19,6 +19,10 @@
         self.log_helper.ignore_errors("Typelib file for namespace")
 
     def get_mount_info(self, *args, **kwargs):
+        if "mounts_file" not in kwargs:
+            kwargs["mounts_file"] = self.makeFile("/dev/hda1 / ext3 rw 0 0\n")
+        if "mtab_file" not in kwargs:
+            kwargs["mtab_file"] = self.makeFile("/dev/hda1 / ext3 rw 0 0\n")
         if "statvfs" not in kwargs:
             kwargs["statvfs"] = lambda path: (0,) * 1000
         plugin = MountInfo(*args, **kwargs)
@@ -136,13 +140,9 @@
         def statvfs(path, multiplier=mock_counter(1).next):
             return (4096, 0, mb(multiplier() * 1000), mb(100), 0, 0, 0, 0, 0)
 
-        filename = self.makeFile("""\
-/dev/hda1 / ext3 rw 0 0
-""")
-        plugin = self.get_mount_info(mounts_file=filename, statvfs=statvfs,
-                                     create_time=self.reactor.time,
-                                     interval=self.monitor.step_size,
-                                     mtab_file=filename)
+        plugin = self.get_mount_info(
+            statvfs=statvfs, create_time=self.reactor.time,
+            interval=self.monitor.step_size)
         self.monitor.add(plugin)
 
         self.reactor.advance(self.monitor.step_size * 2)
@@ -216,12 +216,8 @@
         def statvfs(path):
             return (4096, 0, mb(1000), mb(100), 0, 0, 0, 0, 0)
 
-        filename = self.makeFile("""\
-/dev/hda1 / ext3 rw 0 0
-""")
-        plugin = self.get_mount_info(mounts_file=filename, statvfs=statvfs,
-                                     create_time=self.reactor.time,
-                                     mtab_file=filename)
+        plugin = self.get_mount_info(
+            statvfs=statvfs, create_time=self.reactor.time)
         step_size = self.monitor.step_size
         self.monitor.add(plugin)
 
@@ -251,12 +247,8 @@
         def statvfs(path):
             return (4096, 0, mb(1000), mb(100), 0, 0, 0, 0, 0)
 
-        filename = self.makeFile("""\
-/dev/hda1 / ext3 rw 0 0
-""")
-        plugin = self.get_mount_info(mounts_file=filename, statvfs=statvfs,
-                                     create_time=self.reactor.time,
-                                     mtab_file=filename)
+        plugin = self.get_mount_info(
+            statvfs=statvfs, create_time=self.reactor.time)
         self.monitor.add(plugin)
 
         self.reactor.advance(self.monitor.step_size)
@@ -323,9 +315,7 @@
         """
         "Removable" partitions are not reported to the server.
         """
-        filename = self.makeFile("""\
-/dev/hdc4 /mm xfs rw 0 0""")
-        plugin = self.get_mount_info(mounts_file=filename, mtab_file=filename)
+        plugin = self.get_mount_info()
 
         plugin.is_device_removable = lambda x: True  # They are all removable
 
@@ -339,12 +329,8 @@
         def statvfs(path, multiplier=mock_counter(1).next):
             return (4096, 0, mb(1000), mb(multiplier() * 100), 0, 0, 0, 0, 0)
 
-        filename = self.makeFile("""\
-/dev/hda2 / xfs rw 0 0
-""")
-        plugin = self.get_mount_info(mounts_file=filename, statvfs=statvfs,
-                                     create_time=self.reactor.time,
-                                     mtab_file=filename)
+        plugin = self.get_mount_info(
+            statvfs=statvfs, create_time=self.reactor.time)
         step_size = self.monitor.step_size
         self.monitor.add(plugin)
 
@@ -409,12 +395,8 @@
         """
         def statvfs(path):
             return (4096, 0, mb(1000), mb(100), 0, 0, 0, 0, 0)
-        filename = self.makeFile("""\
-/dev/hda1 / ext3 rw 0 0
-""")
-        plugin = self.get_mount_info(mounts_file=filename,
-                                     create_time=self.reactor.time,
-                                     statvfs=statvfs, mtab_file=filename)
+        plugin = self.get_mount_info(
+            create_time=self.reactor.time, statvfs=statvfs)
         self.monitor.add(plugin)
 
         plugin.run()
@@ -605,12 +587,8 @@
         def statvfs(path):
             return (4096, 0, mb(1000), mb(100), 0, 0, 0, 0, 0)
 
-        filename = self.makeFile("""\
-/dev/hda1 / ext3 rw 0 0
-""")
-        plugin = self.get_mount_info(mounts_file=filename, statvfs=statvfs,
-                                     create_time=self.reactor.time,
-                                     mtab_file=filename)
+        plugin = self.get_mount_info(
+            statvfs=statvfs, create_time=self.reactor.time)
         # Limit the test exchange to 5 items.
         plugin.max_free_space_items_to_exchange = 5
         step_size = self.monitor.step_size