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

Proposed by Leo Arias
Status: Merged
Approved by: Brendan Donegan
Approved revision: 68
Merged at revision: 15
Proposed branch: lp://staging/~canonical-platform-qa/ubuntu-ota-tests/dbus-upgrade
Merge into: lp://staging/ubuntu-ota-tests
Prerequisite: lp://staging/~barry/ubuntu-ota-tests/packaging
Diff against target: 237 lines (+127/-10)
7 files modified
debian/tests/control (+4/-2)
debian/tests/ota-dbus-upgrade (+35/-0)
ubuntu_ota_tests/hooks.py (+4/-0)
ubuntu_ota_tests/reactors.py (+1/-0)
ubuntu_ota_tests/selftests/test_services.py (+2/-1)
ubuntu_ota_tests/tests/test_basic_upgrade.py (+27/-5)
ubuntu_ota_tests/upgrade.py (+54/-2)
To merge this branch: bzr merge lp://staging/~canonical-platform-qa/ubuntu-ota-tests/dbus-upgrade
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Christopher Lee (community) Approve
Barry Warsaw (community) Approve
Review via email: mp+253423@code.staging.launchpad.net

This proposal supersedes a proposal from 2015-03-17.

Commit message

Added the upgrade through dbus.

Description of the change

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

To post a comment you must log in.
Revision history for this message
Barry Warsaw (barry) wrote :

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.

Revision history for this message
Barry Warsaw (barry) wrote :

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

review: Approve
Revision history for this message
Barry Warsaw (barry) wrote :

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.

Revision history for this message
Federico Gimenez (fgimenez) 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

Revision history for this message
Barry Warsaw (barry) wrote :

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.

Revision history for this message
Federico Gimenez (fgimenez) wrote :

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

Revision history for this message
Barry Warsaw (barry) wrote :

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.

Revision history for this message
Federico Gimenez (fgimenez) wrote :

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!

65. By Federico Gimenez

Disabled upgrade assertion for subtask

Revision history for this message
Christopher Lee (veebers) wrote :

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

Ran it locally and it works fine.

review: Needs Fixing
66. By Federico Gimenez

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

67. By Federico Gimenez

flake8

68. By Federico Gimenez

Better log message

Revision history for this message
Federico Gimenez (fgimenez) wrote :

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

Revision history for this message
Christopher Lee (veebers) wrote :

LGTM

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: