Merge lp:~canonical-platform-qa/ubuntu-ota-tests/check-for-running-service into lp:ubuntu-ota-tests

[Christopher Lee]

Add script for ensuring the system-image-dbus process is not running.

Add script for ensuring the system-image-dbus process is not running.

Perhaps there is a better way to have the support scripts instead of in the d/tests/ dir. SHould they instead be packaged up in ubuntu-ota-tests so that they can be consumed outside of the dep8 tests?

Also, note that the is a simple workable case in revno 4 where it just catches the dbus exception, I added the use of checking for the running of the process (check with psutil) but that adds a Depends: python3-psutil meaning the test setup will take a little longer.

Also, also :-) I don't think the test that I have there will stay, it's purely there as a stand-in for now.

[Barry Warsaw (barry)]

Comments inline.

I like the way you've created a package for utilities to live in. We should definitely do that. Also, I think it's perfectly fine to import things from the systemimage package. There's a lot of useful utilities there, such as systemimage.reactor.Reactor, and since this is the SUT, I think it's perfectly acceptable to use them when appropriate.

(Note, in the case where you locate the system-image-dbus process, si has a similar utility, but it's semantics are somewhat different, so it's not a perfect fit.)

review: Needs Fixing

[Barry Warsaw (barry)]

Oh also, re: packaging. Yes, they probably should live outside the debian/tests directory, i.e. get installed into the system Python 3's dist-packages like a normal Python 3 package. That's a little more work though, so I think we can refactor that in later.

[lp:~canonical-platform-qa/ubuntu-ota-tests/check-for-running-service]

6. By Christopher Lee on 2015-03-10

Tidy up use of list comprehension.

7. By Christopher Lee on 2015-03-10

Add timed check to ensure process has actually stopped, adds depends on autopilot though.

8. By Christopher Lee on 2015-03-10

Use global for process name

9. By Christopher Lee on 2015-03-10

Rename loop method

[Christopher Lee (veebers)]

Thanks Barry I've made the suggested changes.

I came across an issue today where the example test would fail due to the process not being exited right at that moment. As a fix I'm using some functionality from autopilot, but I'm concerned by this for som ereasons:
  1. This pulls in autopilot as a dep. Not sure if we want to do that for the core parts of the scripts.
  2. There may be a better way to resolve the issue I use ap for.

I agree re: the packaging, we can iterate over it this week. We need to get the base code down so we can continue.

I'll take a closer look at what systemimage provides. Are you suggesting that there is code there already I can use in this instance?

[lp:~canonical-platform-qa/ubuntu-ota-tests/check-for-running-service]

10. By Leo Arias on 2015-03-10

Reorg to bootstrap the selftests.

[Leo Arias (elopio)]

I moved the selftest to a directory, and made it a test case so we get nice results out of it.

[lp:~canonical-platform-qa/ubuntu-ota-tests/check-for-running-service]

11. By Leo Arias on 2015-03-10

Fixed the order of dependencies.

[Leo Arias (elopio)]

When using 3.0 from barry's branch, the test fails with:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 175, in activate_name_owner
    return self.get_name_owner(bus_name)
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 361, in get_name_owner
    's', (bus_name,), **keywords)
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.NameHasNoOwner: Could not get owner of name 'com.canonical.SystemImage': no such name

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/adt-run.cQbK74/build.KLU/real-tree/debian/tests/ubuntu_ota_tests/selftests/test_services.py", line 28, in test_ensure_system_image_dbus_not_runnig_must_stop_process
    services.get_system_image_interface()
  File "/tmp/adt-run.cQbK74/build.KLU/real-tree/debian/tests/ubuntu_ota_tests/services.py", line 56, in get_system_image_interface
    service = system_bus.get_object('com.canonical.SystemImage', '/Service')
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 241, in get_object
    follow_name_owner_changes=follow_name_owner_changes)
  File "/usr/lib/python3/dist-packages/dbus/proxies.py", line 248, in __init__
    self._named_service = conn.activate_name_owner(bus_name)
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 180, in activate_name_owner
    self.start_service_by_name(bus_name)
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 278, in start_service_by_name
    'su', (bus_name, flags)))
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.NoMemory: Launcher could not run (out of memory)

----------------------------------------------------------------------

[lp:~canonical-platform-qa/ubuntu-ota-tests/check-for-running-service]

12. By Leo Arias on 2015-03-10

Use the test command.

[Federico Gimenez (fgimenez)]

One inline comment, I think also that we should describe a bit in the README the layout that we have decided for the project in order to make it more understandable: if we are not building any debian package, why is the source code under debian/tests? It seems that you must know about autopkgtest to understand where to find things, the layout seems to be conditioned by the tool used to run the tests.

Cheers!

[Leo Arias (elopio)]

Now the tests are passing for me. Weird because I didn't change anything, just reflashed. Maybe my phone was broken before.

[Barry Warsaw (barry)]

On Mar 10, 2015, at 03:42 AM, Christopher Lee wrote:

>I came across an issue today where the example test would fail due to the
>process not being exited right at that moment. As a fix I'm using some
>functionality from autopilot, but I'm concerned by this for som ereasons:

Yeah, D-Bus is notoriously unreliable for tests like this. It's a source of
constant sorrow for getting system-image built in a PPA. I'm now testing some
new techniques using the org.freedesktop.DBus interface. It seems to improve
things but it's not perfect. There are still opportunities for race
conditions, and I suspect they're just inherent in D-Bus.

>1. This pulls in autopilot as a dep. Not sure if we want to do that for the
>core parts of the scripts.

I say "whatever works" :)

>2. There may be a better way to resolve the issue I use ap for.

Here's what I'm using now:

    bus = dbus.SystemBus()
    service = dbus.SystemBus().get_object('org.freedesktop.DBus', '/')
    iface = dbus.Interface(service, 'org.freedesktop.DBus')
    reply = 0
    # 2015-03-09 BAW: This could potentially spin forever, but we'll assume
    # D-Bus eventually is successful in starting the service.
    while reply != 2:
        reply = iface.StartServiceByName('com.canonical.SystemImage', 0)
        time.sleep(0.1)

`reply` could be 1 or 2. 1 means it's started but 2 means it's already
running, which is why I wait for that.

[Barry Warsaw (barry)]

On Mar 10, 2015, at 09:18 AM, Federico Gimenez wrote:

>> +def ensure_loop_ready():
>> + global _loop_running
>
>This global variable and the need for the client code of this module to know
>how it works internally (it has to request first a interface and later pass
>that object to the module methods) makes me wonder if we should use better a
>class here, it seems that the design would be more suited to the task.

I have a somewhat different approach in my branch. We'll need to decide which
way we want to go.

[Barry Warsaw (barry)]

On Mar 10, 2015, at 06:37 AM, Leo Arias wrote:

>When using 3.0 from barry's branch, the test fails with:

I can't tell you what's causing the NoMember error. As best I've been able to
determine, the NameHasNoOwner error occurs when the process
(system-image-dbus) associated with the name (com.canonical.SystemImage)
hasn't yet started. See my previous comment for a new approach I'm trying to
take, which works locally, although the proof will be in the PPA pudding.

[Leo Arias (elopio)]

This is good for me, lets use it as a bootstrap and improve things as we find them.

review: Approve

[Christopher Lee (veebers)]

Me too.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'debian/tests/control'
--- debian/tests/control	2015-03-05 00:36:08 +0000
+++ debian/tests/control	2015-03-10 06:38:09 +0000
@@ -1,2 +1,4 @@
-Tests: ota_basic
-Depends:
+Test-Command: cd debian/tests; python3 -m unittest discover ubuntu_ota_tests.selftests
+Restrictions: allow-stderr
+Depends: python3-autopilot,
+         python3-psutil,

=== added directory 'debian/tests/ubuntu_ota_tests'
=== added file 'debian/tests/ubuntu_ota_tests/__init__.py'
--- debian/tests/ubuntu_ota_tests/__init__.py	1970-01-01 00:00:00 +0000
+++ debian/tests/ubuntu_ota_tests/__init__.py	2015-03-10 06:38:09 +0000
@@ -0,0 +1,27 @@
+#
+# Ubuntu OTA Tests
+# Copyright (C) 2015 Canonical
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+from ubuntu_ota_tests import (
+    services,
+    system,
+)
+
+__all__ = [
+    'services',
+    'system',
+]

=== added directory 'debian/tests/ubuntu_ota_tests/selftests'
=== added file 'debian/tests/ubuntu_ota_tests/selftests/__init__.py'
=== added file 'debian/tests/ubuntu_ota_tests/selftests/test_services.py'
--- debian/tests/ubuntu_ota_tests/selftests/test_services.py	1970-01-01 00:00:00 +0000
+++ debian/tests/ubuntu_ota_tests/selftests/test_services.py	2015-03-10 06:38:09 +0000
@@ -0,0 +1,32 @@
+#
+# Ubuntu OTA Tests
+# Copyright (C) 2015 Canonical
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# 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 unittest
+
+from ubuntu_ota_tests import services
+
+
+class ServicesTestCase(unittest.TestCase):
+
+    def test_ensure_system_image_dbus_not_runnig_must_stop_process(self):
+        # start the dbus process if it isn't already
+        services.get_system_image_interface()
+        self.assertTrue(services._is_system_image_process_running())
+
+        services.ensure_system_image_dbus_not_running()
+        self.assertFalse(services._is_system_image_process_running())

=== added file 'debian/tests/ubuntu_ota_tests/services.py'
--- debian/tests/ubuntu_ota_tests/services.py	1970-01-01 00:00:00 +0000
+++ debian/tests/ubuntu_ota_tests/services.py	2015-03-10 06:38:09 +0000
@@ -0,0 +1,86 @@
+#
+# Ubuntu OTA Tests
+# Copyright (C) 2015 Canonical
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# 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 dbus
+import logging
+
+from autopilot.matchers import Eventually
+from dbus.mainloop.glib import DBusGMainLoop
+from testtools import matchers
+
+from ubuntu_ota_tests import system
+
+
+logger = logging.getLogger(__name__)
+
+# For some reason the process name is 'system-image-db' on the device.
+SYS_IMG_PROC_NAME = 'system-image-db'
+
+# Is there a better mechanism that we can use to ensure that DBusGMainLoop
+# isn't called more than once?
+_loop_running = False
+
+
+def ensure_loop_ready():
+    global _loop_running
+    if not _loop_running:
+        DBusGMainLoop(set_as_default=True)
+        _loop_running = True
+
+
+def get_system_image_interface():
+    """Return a dbus Interface for SystemImage Service.
+
+    Note calling this method will cause system-image-dbus to start (if it's not
+    already started).
+
+    """
+    ensure_loop_ready()
+
+    system_bus = dbus.SystemBus()
+    service = system_bus.get_object('com.canonical.SystemImage', '/Service')
+    iface = dbus.Interface(service, 'com.canonical.SystemImage')
+
+    return iface
+
+
+def ensure_system_image_dbus_not_running():
+    sys_image_iface = get_system_image_interface()
+    try:
+        sys_image_iface.Exit()
+        _ensure_system_image_process_not_running()
+    except dbus.DBusException:
+        logger.debug("Attempted to exit system dbus when not running.")
+
+
+def _ensure_system_image_process_not_running():
+    """Raise RuntimeError if process is still running after 10 seconds."""
+    result = Eventually(
+        matchers.Equals(False)
+    ).match(_is_system_image_process_running)
+    if result is not None:
+        raise RuntimeError(
+            'system image dbus process still running after 10.0 seconds')
+
+
+def _is_system_image_process_running():
+    """Return True if system-image-dbus process is running False otherwise."""
+    try:
+        return system.get_process_by_name(SYS_IMG_PROC_NAME) is not None
+    except OSError:
+        return False

=== added file 'debian/tests/ubuntu_ota_tests/system.py'
--- debian/tests/ubuntu_ota_tests/system.py	1970-01-01 00:00:00 +0000
+++ debian/tests/ubuntu_ota_tests/system.py	2015-03-10 06:38:09 +0000
@@ -0,0 +1,34 @@
+#
+# Ubuntu OTA Tests
+# Copyright (C) 2015 Canonical
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# 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 psutil
+
+
+def get_process_by_name(process_name):
+    """Return a psutil.Process for the process with the given name.
+
+    :raises err something: if a process cannot be found with the given name.
+
+    """
+    try:
+        for process in psutil.process_iter():
+            if process.name() == process_name:
+                return process
+    except IndexError:
+        raise OSError(
+            'No process found with the name "{}"'.format(process_name))