Merge lp:~canonical-platform-qa/ubuntu-ota-tests/dbus-upgrade into lp:ubuntu-ota-tests

[Leo Arias]

Added the upgrade through dbus.

To get this working with si 3.0:
$ ubuntu-device-flash --revision=-1 touch --developer-mode --password 0000 --channel="ubuntu-touch/devel-proposed" --wipe
$ phablet-network
$ phablet-shell
phablet@ubuntu-phablet:~$ sudo mount -o remount,rw /
phablet@ubuntu-phablet:~$ sudo apt-add-repository -y ppa:barry/systemimage
phablet@ubuntu-phablet:~$ sudo apt-get --no-list-cleanup update -o Dir::Etc::SourceList=/dev/null
phablet@ubuntu-phablet:~$ sudo apt-get install system-image-common system-image-dbus system-image-cli
phablet@ubuntu-phablet:~$ sudo mount -o remount,ro /
phablet@ubuntu-phablet:~$ exit
$ adt-run -B --unbuilt-tree=. --testname command1 --- ssh -s ./adb-reboot-to-recovery

[Barry Warsaw (barry)]

After installing si 3.0:

$ sudo -s
$ cd /etc/system-image
$ mkdir config.d
$ cp client.ini config.d/00_default.ini
$ cp channel.ini config.d/01_channel.ini
$ exit

Eventually that wouldn't be needed.

[Barry Warsaw (barry)]

A couple of minor comments, but nothing to hold up approval.

review: Approve

[Barry Warsaw (barry)]

Hmm, I am getting some test failures. My device is flashed to 154, but I hacked /etc/system-image/config.d/01_channel.ini to pretend it's at 153. system-image-cli --dry-run shows an upgrade to 154 available. However:

======================================================================
FAIL: test_basic_over_the_air_upgrade (ubuntu_ota_tests.tests.test_basic_upgrade.OTABasicUpgradeTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/adt-run.eQlSKM/build.IKc/real-tree/ubuntu_ota_tests/tests/test_basic_upgrade.py", line 47, in test_basic_over_the_air_upgrade
    self.upgrade()
  File "/tmp/adt-run.eQlSKM/build.IKc/real-tree/ubuntu_ota_tests/tests/test_basic_upgrade.py", line 35, in upgrade
    upgrade.upgrade_with_system_image_dbus()
  File "/tmp/adt-run.eQlSKM/build.IKc/real-tree/ubuntu_ota_tests/upgrade.py", line 46, in upgrade_with_system_image_dbus
    assert status.is_available, 'Update not available'
AssertionError: Update not available

I also see check-for-update failing on an ImportError, but that's probably because it doesn't do the PYTHONPATH workaround.

[Federico Gimenez (fgimenez)]

It seems that the command

  system-image-dbus -v -C {config_dir}

in ubuntu_ota_tests.upgrade#upgrade_with_system_image_dbus is causing that the dbus service methods are not able to find the new update. If it's executed without the -C switch it seems to work well.

Could anyone please confirm this? I'm running the tests with:

$ ubuntu-device-flash --revision=-1 touch --developer-mode --password 0000 --channel="ubuntu-touch/devel-proposed" --wipe

$ adt-run -B --unbuilt-tree=. --setup-commands 'mount -o remount,rw /' --setup-commands 'apt-add-repository -y ppa:barry/systemimage' --setup-commands 'apt-get --no-list-cleanup update -o Dir::Etc::SourceList=/dev/null' --setup-commands 'apt-get install -y system-image-dbus system-image-common system-image-cli' --setup-commands 'if [ ! -d "/etc/system-image/config.d" ]; then mkdir -p /etc/system-image/config.d && cd /etc/system-image && cp client.ini config.d/00_default.ini && cp channel.ini config.d/01_channel.ini; fi' --setup-commands 'mount -o remount,ro /' --- ssh -s ./adb-reboot-to-recovery

[Barry Warsaw (barry)]

On Mar 24, 2015, at 11:33 AM, Federico Gimenez wrote:

>It seems that the command
>
> system-image-dbus -v -C {config_dir}
>
>in ubuntu_ota_tests.upgrade#upgrade_with_system_image_dbus is causing that the dbus service methods are not able to find the new update. If it's executed without the -C switch it seems to work well.
>
>Could anyone please confirm this? I'm running the tests with:
>
>$ ubuntu-device-flash --revision=-1 touch --developer-mode --password 0000 --channel="ubuntu-touch/devel-proposed" --wipe
>
>$ adt-run -B --unbuilt-tree=. --setup-commands 'mount -o remount,rw /' --setup-commands 'apt-add-repository -y ppa:barry/systemimage' --setup-commands 'apt-get --no-list-cleanup update -o Dir::Etc::SourceList=/dev/null' --setup-commands 'apt-get install -y system-image-dbus system-image-common system-image-cli' --setup-commands 'if [ ! -d "/etc/system-image/config.d" ]; then mkdir -p /etc/system-image/config.d && cd /etc/system-image && cp client.ini config.d/00_default.ini && cp channel.ini config.d/01_channel.ini; fi' --setup-commands 'mount -o remount,ro /' --- ssh -s ./adb-reboot-to-recovery
>

When I run the above adt-run command, I get this:

======================================================================
FAIL: test_basic_over_the_air_upgrade (ubuntu_ota_tests.tests.test_basic_upgrade.OTABasicUpgradeTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/adt-run.Pryv10/build.mPu/real-tree/ubuntu_ota_tests/tests/test_basic_upgrade.py", line 47, in test_basic_over_the_air_upgrade
    self.upgrade()
  File "/tmp/adt-run.Pryv10/build.mPu/real-tree/ubuntu_ota_tests/tests/test_basic_upgrade.py", line 35, in upgrade
    upgrade.upgrade_with_system_image_dbus()
  File "/tmp/adt-run.Pryv10/build.mPu/real-tree/ubuntu_ota_tests/upgrade.py", line 52, in upgrade_with_system_image_dbus
    assert status.is_available, 'Update not available'
AssertionError: Update not available

which is telling me that there's no upgrade available, possibly because a
previous test actually performed the update, such that the device is at the
current revision. This doesn't seem right though because:

root@ubuntu-phablet:~# system-image-cli --dry-run
Upgrade path is 154:155:156:157:158
Target phase: 33%

Looking back through the logs though I see:

[systemimage] Mar 24 14:00:08 2015 (10604) no matching channel: daily

and yet:

root@ubuntu-phablet:~# system-image-cli --info
current build number: 153
device name: krillin
channel: ubuntu-touch/devel-proposed
alias: ubuntu-touch/vivid-proposed
last update: 2015-03-19 18:27:46
version version: 153
version ubuntu: 20150319.1
version device: 20150310-ae1ddec
version custom: 20150319.1

I'm not sure where this 'daily' channel is coming from though.

[Federico Gimenez (fgimenez)]

Thanks Barry, yes, this seems to be the same behaviour that you described above, there's an update available but services.check_for_update is not able to detect it.

Could you please replace the 'command' variable definition at [1] with:

  command = 'system-image-dbus -v'

and run the test again, including the reflash? In that case, I'm seeing that the update is detected correctly and all seems to go well, but the apply hook is not defined. So it seems that when -C is passed to system-image-dbus (or when there's a .ini file like the one we are creating) the detection of the update doesn't work the same way, could you please confirm this?

Cheers!

[1] http://bazaar.launchpad.net/~canonical-platform-qa/ubuntu-ota-tests/dbus-upgrade/view/head:/ubuntu_ota_tests/upgrade.py#L43

[Barry Warsaw (barry)]

On Mar 24, 2015, at 02:40 PM, Federico Gimenez wrote:

>Could you please replace the 'command' variable definition at [1] with:
>
> command = 'system-image-dbus -v'
>
>and run the test again, including the reflash? In that case, I'm seeing that
>the update is detected correctly and all seems to go well, but the apply hook
>is not defined. So it seems that when -C is passed to system-image-dbus (or
>when there's a .ini file like the one we are creating) the detection of the
>update doesn't work the same way, could you please confirm this?

Before running the above, I made this change:

=== modified file 'ubuntu_ota_tests/upgrade.py'
--- ubuntu_ota_tests/upgrade.py 2015-03-24 11:01:07 +0000
+++ ubuntu_ota_tests/upgrade.py 2015-03-24 18:06:41 +0000
@@ -39,10 +39,15 @@
 def upgrade_with_system_image_dbus():
     if systemimage.service.__version__.startswith('3.0'):
         _write_adt_apply_config()
+ config_dir = _get_system_image_config_dir()
+ command = 'system-image-dbus -v -C {}'.format(config_dir)
+ print('Starting system-image with', command)
+ for filename in sorted(os.listdir(config_dir)):
+ print('=====', filename, '=====')
+ with open(os.path.join(config_dir, filename), 'r',
+ encoding='utf-8') as fp:
+ print(fp.read())
         services.ensure_system_image_dbus_not_running()
- command = 'system-image-dbus -v -C {config_dir}'.format(
- config_dir=_get_system_image_config_dir())
- print('Starting system-image with', command)
         subprocess.Popen(command.split())
         # as Popen doesn't block without the following sleep this is printed:
         # [systemimage] Mar 23 16:18:10 2015 (26139) Cannot get exclusive ownership of bus name.

Then I ran the test and watched the output. What I see makes me think that
_get_system_image_config_dir() is not working as expected.

Starting system-image with system-image-dbus -v -C /tmp/adt-run.dGsx3q/ota-dbus-upgrade-artifacts
===== 99_adt_apply.ini =====
[hooks]
apply: ubuntu_ota_tests.hooks.ADTRebootToRecovery

See? 99_adt_apply.ini appears to be the *only* .ini file in
/tmp/adt-run.dGsx3q/ota-dbus-upgrade-artifacts. Now the log message:

[systemimage] Mar 24 18:16:27 2015 (5682) no matching channel: daily

makes sense because 'daily' is the baked-in default channel when there is none
defined in an on-system config file.

So I think the test setup is not happening as you expect. Note that the
adt-run command described in d/t/ota-dbus-upgrade, and which I ran, does
correctly set up /etc/systemimage-config.d from /etc/system-iamge, but the
$ADT_ARTIFACTS directory is *not* set up correctly.

I'm not sure what the intent of using $ADT_ARTIFACTS is, but if you really
want the -C config dir to point there, you should at least copy all the .ini
files from /etc/system-image/config.d to $ADT_ARTIFACTS, and then add
99_adt_apply.ini

I haven't investigated, but I wonder if this is the same reason that
test_download_update() times out for me.

[Federico Gimenez (fgimenez)]

Thanks Barry, with the default config files in the custom directory all goes fine. We are using this directory because during the test execution the filesystem is in ro mode, this way we can add the file for defining the apply hook there.

I'm now looking into the download timeout, the services' module download_update function seems to be broken. Anyway with a manual download from the device the reboot doesn't seem to apply it, it seems to boot into recovery mode without applying the update. I'll go for this once the timeout is fixed.

Cheers!

[lp:~canonical-platform-qa/ubuntu-ota-tests/dbus-upgrade]

65. By Federico Gimenez on 2015-03-25

Disabled upgrade assertion for subtask

[Christopher Lee (veebers)]

Looking good to me, a couple of minor comments inline.

Ran it locally and it works fine.

review: Needs Fixing

[lp:~canonical-platform-qa/ubuntu-ota-tests/dbus-upgrade]

66. By Federico Gimenez on 2015-03-26

Added bug references for test command comment and the required fix for apply upgrade

67. By Federico Gimenez on 2015-03-26

flake8

68. By Federico Gimenez on 2015-03-26

Better log message

[Federico Gimenez (fgimenez)]

Thanks Chris, added log messages describing the type of upgrade and references to the bugs, they are [1] and [2], I'll refer to them also from the trello cards.

I'll make another branch for the apply upgrade fix with this as a prerequisite.

Cheers!

[1] https://bugs.launchpad.net/ubuntu-ota-tests/+bug/1436753
[2] https://bugs.launchpad.net/ubuntu-ota-tests/+bug/1436730

[Christopher Lee (veebers)]

LGTM

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'debian/tests/control'
--- debian/tests/control	2015-03-18 19:55:33 +0000
+++ debian/tests/control	2015-03-26 09:27:19 +0000
@@ -5,11 +5,13 @@
          python3-psutil,
          system-image-dbus
 
-Test-Command: python3 -m unittest ubuntu_ota_tests.tests.test_basic_upgrade
+Tests: ota-dbus-upgrade
 Restrictions: needs-root allow-stderr
 Depends: python3-autopilot,
          python3-psutil,
-         system-image-cli
+         system-image-cli,
+         system-image-common,
+         system-image-dbus
 
 Tests: check-for-update
 Depends: system-image-common,

=== added file 'debian/tests/ota-dbus-upgrade'
--- debian/tests/ota-dbus-upgrade	1970-01-01 00:00:00 +0000
+++ debian/tests/ota-dbus-upgrade	2015-03-26 09:27:19 +0000
@@ -0,0 +1,35 @@
+#!/bin/bash
+
+#
+# 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/>.
+#
+
+# This test requires a device upgradable, for example with:
+#
+# $ ubuntu-device-flash --revision=-1 touch --developer-mode --password 0000 --channel="ubuntu-touch/devel-proposed" --wipe
+#
+# The test can be executed with this command:
+#
+# $ adt-run -B --unbuilt-tree=. --setup-commands 'mount -o remount,rw /' --setup-commands 'apt-add-repository -y ppa:barry/systemimage' --setup-commands 'apt-get --no-list-cleanup update -o Dir::Etc::SourceList=/dev/null' --setup-commands 'apt-get install -y -q system-image-cli system-image-common system-image-dbus' --setup-commands 'if [ ! -d "/etc/system-image/config.d" ]; then mkdir -p /etc/system-image/config.d && cd /etc/system-image && cp client.ini config.d/00_default.ini && cp channel.ini config.d/01_channel.ini; fi' --setup-commands 'sync; sleep 2; mount -o remount,ro /' --- ssh -s ./adb-reboot-to-recovery
+#
+# When si 3.0 reaches the archive this can be executed normally
+# Ref. bug: https://bugs.launchpad.net/ubuntu-ota-tests/+bug/1436730
+#
+
+set -e
+
+PYTHONPATH=$PYTHONPATH:$(pwd) python3 -m unittest ubuntu_ota_tests.tests.test_basic_upgrade

=== modified file 'ubuntu_ota_tests/hooks.py'
--- ubuntu_ota_tests/hooks.py	2015-03-24 11:32:24 +0000
+++ ubuntu_ota_tests/hooks.py	2015-03-26 09:27:19 +0000
@@ -26,6 +26,7 @@
     # si 2.5
     from systemimage.reboot import BaseReboot as Base
 
+from systemimage.config import config
 
 log = logging.getLogger('systemimage')
 
@@ -59,6 +60,9 @@
             log.exception('reboot exit status: {}'.format(error.returncode))
             log.exception('reboot command output: {}'.format(error.output))
             raise
+        # This code may or may not run.  We're racing against the system
+        # reboot procedure.
+        config.dbus_service.Rebooting(True)
 
     # si 2.5
     reboot = apply

=== modified file 'ubuntu_ota_tests/reactors.py'
--- ubuntu_ota_tests/reactors.py	2015-03-18 19:58:04 +0000
+++ ubuntu_ota_tests/reactors.py	2015-03-26 09:27:19 +0000
@@ -75,6 +75,7 @@
 
     def _do_Applied(self, signal, path, *args):
         self.signals.append(AppliedRecord(*args))
+        self.quit()
 
 
 class DownloadReactor(BaseReactor):

=== modified file 'ubuntu_ota_tests/selftests/test_services.py'
--- ubuntu_ota_tests/selftests/test_services.py	2015-03-24 11:32:24 +0000
+++ ubuntu_ota_tests/selftests/test_services.py	2015-03-26 09:27:19 +0000
@@ -59,6 +59,8 @@
         services.set_auto_download(True)
         self.assertEqual(True, services.get_auto_download())
 
+    @unittest.skipIf(is_update_available(),
+                     'Update available, would make DownloadUpdate fail')
     def test_check_for_update(self):
         status = services.check_for_update()
         utcnow = datetime.utcnow().replace(microsecond=0).isoformat(' ')
@@ -89,7 +91,6 @@
     def test_download_update(self):
         sys_image_iface = services.get_system_image_interface()
         self.assertEqual(sys_image_iface.CancelUpdate(), '')
-        self.assertTrue(is_update_available())
         signals = services.download_update()
         # validate that last record was a successful download signal
         self.validate_successful_download(signals[-1])

=== modified file 'ubuntu_ota_tests/tests/test_basic_upgrade.py'
--- ubuntu_ota_tests/tests/test_basic_upgrade.py	2015-03-12 15:37:04 +0000
+++ ubuntu_ota_tests/tests/test_basic_upgrade.py	2015-03-26 09:27:19 +0000
@@ -16,19 +16,41 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import logging
 import os
 import unittest
 
-from ubuntu_ota_tests import upgrade
+from ubuntu_ota_tests import (
+    services,
+    upgrade
+)
+
+logger = logging.getLogger(__name__)
 
 
 class OTABasicUpgradeTestCase(unittest.TestCase):
 
+    def before_upgrade(self):
+        pass
+
+    def upgrade(self):
+        try:
+            logger.info('Trying to upgrade with system-image-dbus')
+            upgrade.upgrade_with_system_image_dbus()
+        except NotImplementedError:
+            logger.info('Upgrading with system-image-cli')
+            upgrade.upgrade_with_system_image_cli()
+
+    def after_upgrade(self):
+        services.check_for_update()
+        # XXX: enable this test when the upgrade will work
+        # Ref. bug: https://bugs.launchpad.net/ubuntu-ota-tests/+bug/1436753
+        # self.assertFalse(status.is_available, 'Update available')
+
     def test_basic_over_the_air_upgrade(self):
         rebooted_mark = os.environ.get('ADT_REBOOT_MARK', None)
         if not rebooted_mark:
-            # The upgrade with dbus needs more work, it's getting stuck.
-            # upgrade.upgrade_with_system_image_dbus()
-            upgrade.upgrade_with_system_image_cli()
+            self.before_upgrade()
+            self.upgrade()
         else:
-            print('Rebooted')
+            self.after_upgrade()

=== modified file 'ubuntu_ota_tests/upgrade.py'
--- ubuntu_ota_tests/upgrade.py	2015-03-10 21:46:22 +0000
+++ ubuntu_ota_tests/upgrade.py	2015-03-26 09:27:19 +0000
@@ -16,13 +16,65 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import glob
+import os
 import subprocess
 
-from ubuntu_ota_tests import hooks
+import systemimage.service
+import shutil
+import time
+
+from ubuntu_ota_tests import (
+    hooks,
+    services
+)
 
 
 def upgrade_with_system_image_cli():
     command = 'system-image-cli -v -g'
-    print('Upgrading with ' + command)
+    print('Upgrading with ', command)
     subprocess.check_call(command.split())
     hooks.ADTRebootToRecovery().reboot()
+
+
+def upgrade_with_system_image_dbus():
+    if systemimage.service.__version__.startswith('3.0'):
+        _write_adt_apply_config()
+        _copy_default_system_image_config()
+        services.ensure_system_image_dbus_not_running()
+        command = 'system-image-dbus -v -C {config_dir}'.format(
+            config_dir=_get_system_image_config_dir())
+        print('Starting system-image with', command)
+        subprocess.Popen(command.split())
+        # as Popen doesn't block without the following sleep this is printed:
+        # [systemimage] (26139) Cannot get exclusive ownership of bus name.
+        time.sleep(1)
+        services.set_auto_download(False)
+        status = services.check_for_update()
+        assert status.is_available, 'Update not available'
+        services.download_update()
+        services.apply_update()
+    else:
+        raise NotImplementedError(
+            'dbus upgrade not implemented for system-image 2.5')
+
+
+def _write_adt_apply_config():
+    config_path = _get_system_image_config_dir()
+    adt_apply_config_path = os.path.join(config_path, '99_adt_apply.ini')
+    with open(adt_apply_config_path, 'w+') as apply_config_file:
+        print("""\
+[hooks]
+apply: ubuntu_ota_tests.hooks.ADTRebootToRecovery
+""", file=apply_config_file)
+
+
+def _copy_default_system_image_config():
+    config_path = _get_system_image_config_dir()
+    default_path = '/etc/system-image/config.d'
+    for file in glob.glob('{path}/*.ini'.format(path=default_path)):
+        shutil.copy(file, config_path)
+
+
+def _get_system_image_config_dir():
+    return os.environ.get('ADT_ARTIFACTS')