Merge lp:~allenap/maas/shared-to-per-tenant-storage into lp:~maas-committers/maas/trunk

[Gavin Panella]

Add the mechanism for migrating shared-namespace file storage usage over to the per-tenant model.

There is a carefully constructed 4-step process that's intended to minimise disruption. Ultimately some eggs do need to be broken, but this branch catches their contents and tries damn hard to bake a tasty cake with it.

[Gavin Panella (allenap)]

This is a work in progress for the shared to per-tenant namespace migration. It might actually be complete, but it's 1am here and I can't tell any more.

[Raphaël Badin (rvb)]

Great work!

[0]

I'd suggest adding a test in TestMigrate to make sure the files change ownership properly when there is a valid 'provider-state' file (and a related node with an owner). I know you're sort of testing this at a lower level in 'TestFunctions' but since we suspect this is going to be one of the most used code paths, I'd add that test anyway.

[1]

Can you please reference the "steps" you're testing in 'TestMigrate' the same way you did it in "def migrate():"?

more to come…

[Raphaël Badin (rvb)]

[2]

+# -*- coding: utf-8 -*-

I don't see why this is required…?
(fwiw, the only file with that are the auto-generated migration files)

PS: I suspect this is just because of → … s/→/->/ ?

[3]

76 + try:
77 + legacy_user = User.objects.get(username=legacy_user_name)

If a migrations runs and we are in a position to run that code (i.e. more than one user, no 'provider-state' file), if it happens that there is a *real* user with that username, it won't do the right thing. I reckon this is pretty improbable but maybe we should at least raise and exception in that case. What do you think?

[Gavin Panella (allenap)]

> [2]
>
> +# -*- coding: utf-8 -*-
>
> I don't see why this is required…?
> (fwiw, the only file with that are the auto-generated migration files)
>
> PS: I suspect this is just because of → … s/→/->/ ?

Yes, that's exactly why. I'll remove it for the benefit of
encoding-challenged text editors :)

>
> [3]
>
> 76 + try:
> 77 + legacy_user = User.objects.get(username=legacy_user_name)
>
> If a migrations runs and we are in a position to run that code
> (i.e. more than one user, no 'provider-state' file), if it happens
> that there is a *real* user with that username, it won't do the
> right thing. I reckon this is pretty improbable but maybe we should
> at least raise and exception in that case. What do you think?

I want the code to be safe to run multiple times, so I think it's
important to be able to pick up an existing legacy user. However, I'll
make the legacy user similar to the other system users, in that it
won't have a UserProfile. Then I can check that here and raise an
exception if it appears to be a regular user with the same name.

Sound okay?

I

[Raphaël Badin (rvb)]

[4]

149 +def give_credentials_to_user(user_from, user_dest):
150 + """Gives one user's credentials to another.

'credentials' is a bit vague don't you think? How about 'API credentials'?

[5]

57 +from maasserver.models import (
58 + FileStorage,
59 + Node,
60 + SSHKey,
61 + User,
62 + )

There is no 'User' class in maasserver.models, it's simply an alias for django.contrib.auth.models.User. I think it's clearer to import the class from Django.

[6]

106 +def get_owned_nodes_owners():
107 + """Returns a `QuerySet` of the owners of nodes owned by real users."""
108 + owner_ids = get_owned_nodes().values_list("owner", flat=True)
109 + return User.objects.filter(id__in=owner_ids)

Adding '.distinct()' to the query on line 108 might speed things up a bit if there is lots of nodes belonging to just a few users.

[Raphaël Badin (rvb)]

> > If a migrations runs and we are in a position to run that code
> > (i.e. more than one user, no 'provider-state' file), if it happens
> > that there is a *real* user with that username, it won't do the
> > right thing. I reckon this is pretty improbable but maybe we should
> > at least raise and exception in that case. What do you think?
>
> I want the code to be safe to run multiple times, so I think it's
> important to be able to pick up an existing legacy user. However, I'll
> make the legacy user similar to the other system users, in that it
> won't have a UserProfile. Then I can check that here and raise an
> exception if it appears to be a regular user with the same name.
>
> Sound okay?

Sounds good to me. Thanks!

[Gavin Panella (allenap)]

> [0]
>
> I'd suggest adding a test in TestMigrate to make sure the files change
> ownership properly when there is a valid 'provider-state' file (and a related
> node with an owner). I know you're sort of testing this at a lower level in
> 'TestFunctions' but since we suspect this is going to be one of the most used
> code paths, I'd add that test anyway.

Good idea, done.

>
> [1]
>
> Can you please reference the "steps" you're testing in 'TestMigrate' the same
> way you did it in "def migrate():"?

Sure, done.

[Gavin Panella (allenap)]

> [4]
>
> 149     +def give_credentials_to_user(user_from, user_dest):
> 150     +    """Gives one user's credentials to another.
>
> 'credentials' is a bit vague don't you think?  How about 'API credentials'?

I've renamed the function and updated the docstring.

> [5]
>
> 57      +from maasserver.models import (
> 58      +    FileStorage,
> 59      +    Node,
> 60      +    SSHKey,
> 61      +    User,
> 62      +    )
>
> There is no 'User' class in maasserver.models, it's simply an alias for
> django.contrib.auth.models.User.  I think it's clearer to import the class
> from Django.

Done.

> [6]
>
> 106     +def get_owned_nodes_owners():
> 107     +    """Returns a `QuerySet` of the owners of nodes owned by real
> users."""
> 108     +    owner_ids = get_owned_nodes().values_list("owner", flat=True)
> 109     +    return User.objects.filter(id__in=owner_ids)
>
> Adding '.distinct()' to the query on line 108 might speed things up a bit if
> there is lots of nodes belonging to just a few users.

Done.

[Raphaël Badin (rvb)]

[7]

433 + def test_give(self):
434 + user1 = factory.make_user()
435 + user2 = factory.make_user()
436 + profile = user1.get_profile()
437 + consumer, token = profile.create_authorisation_token()
438 + give_credentials_to_user(user1, user2)
439 + self.assertEqual(user2, reload_object(consumer).user)
440 + self.assertEqual(user2, reload_object(token).user)
441 +
442 + def test_consumer_and_token_saved(self):
443 + user1 = factory.make_user()
444 + user2 = factory.make_user()
445 + profile = user1.get_profile()
446 + consumer, token = profile.create_authorisation_token()
447 + self.patch(consumer, "save")
448 + self.patch(token, "save")
449 + give_credentials_to_user(user1, user2)
450 + consumer.save.assert_called_once()
451 + token.save.assert_called_once()

I don't really see what the second test adds… since you're reloading the objects in the first test and you're getting what you expect, you can be sure the objects have been saved… ?

[8]

456 + def test_give(self):
457 + user1 = factory.make_user()
458 + user2 = factory.make_user()
459 + node = factory.make_node(owner=user1)
460 + give_node_to_user(node, user2)
461 + self.assertEqual(user2, node.owner)
462 +
463 + def test_node_saved(self):
464 + user1 = factory.make_user()
465 + user2 = factory.make_user()
466 + node = factory.make_node(owner=user1)
467 + save = self.patch(node, "save")
468 + give_node_to_user(node, user2)
469 + save.assert_called_once()

Similar remark here. Although I'd refactor the first test to reload the object before checking the owner, and I'd get rid of the second test.

[Raphaël Badin (rvb)]

[9]

580 + self.assertEqual(
581 + (legacy_user, legacy_user, legacy_user, legacy_user),

"(legacy_user, legacy_user, legacy_user, legacy_user)" => "[legacy_user] * 4" ?

I think it's more readable but this is really a detail.

[10]

558 + self.assertEqual(user1, reload_object(node1).owner)
559 + self.assertEqual(user1, reload_object(node2).owner)

Probably better to make that one check.

[11]

472 +class TestMigrateToUser(TestCase):
473 +
474 + def test_migrate(self):

That test is a bit… huge :) . Don't you think it could test for behavior instead of patching methods… and maybe share some code with TestMigrate.test_migrate_ancillary_data_to_legacy_user_when_multiple_users ?

[Gavin Panella (allenap)]

> ... I'll make the legacy user similar to the other system users, in
> that it won't have a UserProfile. Then I can check that here and
> raise an exception if it appears to be a regular user with the same
> name.

Actually, I'm not 100% sure this is a good idea. The legacy user needs
to be the same as a regular user. It's not unreasonable for someone to
set its password and log-in to the web UI, at which point the lack of
a UserProfile will cause problems, I think.

I'm inclined to either (a) not check or (b) check only that the user's
full name is "Shared Environment".

[Raphaël Badin (rvb)]

> > ... I'll make the legacy user similar to the other system users, in
> > that it won't have a UserProfile. Then I can check that here and
> > raise an exception if it appears to be a regular user with the same
> > name.
>
> Actually, I'm not 100% sure this is a good idea. The legacy user needs
> to be the same as a regular user. It's not unreasonable for someone to
> set its password and log-in to the web UI, at which point the lack of
> a UserProfile will cause problems, I think.

Indeed, you're right, I lost track of that. (I think we need a profile to log in the UI so a user probably won't be able to log in, but as you pointed out, this is a problem.)

>
> I'm inclined to either (a) not check or (b) check only that the user's
> full name is "Shared Environment".

b) does not feel very precise :/

[Gavin Panella (allenap)]

> [7]
...
> I don't really see what the second test adds… since you're reloading the
> objects in the first test and you're getting what you expect, you can be sure
> the objects have been saved… ?

Good point. I was adding belts and braces where dungarees were being
worn :)

> [8]
...
> Similar remark here. Although I'd refactor the first test to reload the
> object before checking the owner, and I'd get rid of the second test.

Done.

[Gavin Panella (allenap)]

> [9]
>
> 580     +        self.assertEqual(
> 581     +            (legacy_user, legacy_user, legacy_user, legacy_user),
>
> "(legacy_user, legacy_user, legacy_user, legacy_user)" => "[legacy_user] * 4"
> ?
>
> I think it's more readable but this is really a detail.

We'll agree to differ on this one :)

> [10]
>
> 558     +        self.assertEqual(user1, reload_object(node1).owner)
> 559     +        self.assertEqual(user1, reload_object(node2).owner)
>
> Probably better to make that one check.

Done.

>
> [11]
>
> 472     +class TestMigrateToUser(TestCase):
> 473     +
> 474     +    def test_migrate(self):
>
> That test is a bit… huge :) .  Don't you think it could test for behavior
> instead of patching methods… and maybe share some code with
> TestMigrate.test_migrate_ancillary_data_to_legacy_user_when_multiple_users ?

The behaviour of the individual components is tested elsewhere, and
the overall behaviour is tested in TestMigrate, so I sought only to
show that things were wired up correctly. In a way it's also a
reminder not to put more logic into migrate_to_user(); it should be
factored into separate functions.

It was a bit of a what-if experiment too. I've added comments to the
function to explain its rationale, but I'm open to more discussion.
It's interesting to consider the value of this kind of test; perhaps
not very much.

[Gavin Panella (allenap)]

> > I'm inclined to either (a) not check or (b) check only that the user's
> > full name is "Shared Environment".
>
> b) does not feel very precise :/

I agree, but it's a small problem in the scale of things here. I say
we just stick with what we've got, i.e. no checks. If someone already
has a user called shared-environment then I'm inclined to think that
it's the user they'd want us to choose anyway!

[Raphaël Badin (rvb)]

> I agree, but it's a small problem in the scale of things here. I say
> we just stick with what we've got, i.e. no checks. If someone already
> has a user called shared-environment then I'm inclined to think that
> it's the user they'd want us to choose anyway!

Okay… the consequences of picking up the wrong user are not devastating anyway, plus it's very unlikely that someone would have created a user precisely with that name.

[Raphaël Badin (rvb)]

Looks good, thanks for all your work on this branch!

review: Approve

[Raphaël Badin (rvb)]

A fallout from renaming the ssh keys:

======================================================================
FAIL: maasserver.tests.test_sshkey.SSHKeyTest.test_sshkey_display_with_real_life_key
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/rvb/canonical/shared-to-per-tenant-storage/src/maasserver/tests/test_sshkey.py", line 229, in test_sshkey_display_with_real_life_key
    'ssh-rsa AAAAB3NzaC1yc2E… ubuntu@server-7476', display)
MismatchError: !=:
reference = u'ssh-rsa AAAAB3NzaC1yc2E… ubuntu@server-7476'
actual = u'ssh-rsa AAAAB3NzaC1yc… ubuntu@test_rsa0.pub'

[Gavin Panella (allenap)]

> Looks good, thanks for all your work on this branch!

Thank you for all your help :)

> A fallout from renaming the ssh keys:

Thanks for pointing that out. Fixed.

=== modified file 'src/maasserver/models/user.py'
--- src/maasserver/models/user.py	2012-08-13 04:47:10 +0000
+++ src/maasserver/models/user.py	2013-03-06 15:28:20 +0000
@@ -15,6 +15,7 @@
     'create_user',
     'get_auth_tokens',
     'get_creds_tuple',
+    'SYSTEM_USERS',
     ]
 
 from maasserver import worker_user

=== added file 'src/maasserver/support/pertenant/migration.py'
--- src/maasserver/support/pertenant/migration.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/support/pertenant/migration.py	2013-03-06 15:28:20 +0000
@@ -0,0 +1,179 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Shared namespace --> per-tenant namespace migration.
+
+Perform the following steps to migrate:
+
+1. When no files exist (i.e. no Juju environments exist): do nothing
+1a. When no *unowned* files exist: do nothing.
+
+2. When there's only one user: assign ownership of all files to user.
+
+3. When there are multiple users and a `provider-state` file: parse that file
+   to extract the instance id of the bootstrap node. From that instance id,
+   get the identity of the user who deployed this environment (that's the
+   owner of the bootstrap node). Then proceed as in 4, using that user as the
+   "legacy" user.
+
+4. When there are multiple users: create a new "legacy" user, assign ownership
+   of all files and allocated/owned nodes to this user, copy all public SSH
+   keys to this user, and move all API credentials to this user.
+
+There's not a lot we can do about SSH keys authorised to connect to the
+already deployed nodes in #3, but this set will only ever decrease: nodes
+allocated after this migration will permit access from any of the users with
+SSH keys prior to the migration.
+"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    "migrate",
+    ]
+
+from django.contrib.auth.models import User
+from maasserver.models import (
+    FileStorage,
+    Node,
+    SSHKey,
+    )
+from maasserver.models.user import (
+    get_auth_tokens,
+    SYSTEM_USERS,
+    )
+from maasserver.support.pertenant.utils import get_bootstrap_node_owner
+from maasserver.utils.orm import get_one
+
+
+legacy_user_name = "shared-environment"
+
+
+def get_legacy_user():
+    """Return the legacy namespace user, creating it if need be."""
+    try:
+        legacy_user = User.objects.get(username=legacy_user_name)
+    except User.DoesNotExist:
+        # Create the legacy user with a local, probably non-working, email
+        # address, and an unusable password.
+        legacy_user = User.objects.create_user(
+            email="%s@localhost" % legacy_user_name,
+            username=legacy_user_name)
+        legacy_user.first_name = "Shared"
+        legacy_user.last_name = "Environment"
+        legacy_user.is_active = True
+    return legacy_user
+
+
+def get_unowned_files():
+    """Returns a `QuerySet` of unowned files."""
+    return FileStorage.objects.filter(owner=None)
+
+
+def get_real_users():
+    """Returns a `QuerySet` of real. not system, users."""
+    users = User.objects.exclude(username__in=SYSTEM_USERS)
+    users = users.exclude(username=legacy_user_name)
+    return users
+
+
+def get_owned_nodes():
+    """Returns a `QuerySet` of nodes owned by real users."""
+    return Node.objects.filter(owner__in=get_real_users())
+
+
+def get_owned_nodes_owners():
+    """Returns a `QuerySet` of the owners of nodes owned by real users."""
+    owner_ids = get_owned_nodes().values_list("owner", flat=True)
+    return User.objects.filter(id__in=owner_ids.distinct())
+
+
+def get_destination_user():
+    """Return the user to which resources should be assigned."""
+    real_users = get_real_users()
+    if real_users.count() == 1:
+        return get_one(real_users)
+    else:
+        bootstrap_user = get_bootstrap_node_owner()
+        if bootstrap_user is None:
+            return get_legacy_user()
+        else:
+            return bootstrap_user
+
+
+def get_ssh_keys(user):
+    """Return the SSH key strings belonging to the specified user."""
+    return SSHKey.objects.filter(user=user).values_list("key", flat=True)
+
+
+def copy_ssh_keys(user_from, user_dest):
+    """Copies SSH keys from one user to another.
+
+    This is idempotent, and does not clobber the destination user's existing
+    keys.
+    """
+    user_from_keys = get_ssh_keys(user_from)
+    user_dest_keys = get_ssh_keys(user_dest)
+    for key in set(user_from_keys).difference(user_dest_keys):
+        ssh_key = SSHKey(user=user_dest, key=key)
+        ssh_key.save()
+
+
+def give_file_to_user(file, user):
+    """Give a file to a user."""
+    file.owner = user
+    file.save()
+
+
+def give_api_credentials_to_user(user_from, user_dest):
+    """Gives one user's API credentials to another.
+
+    This ensures that users of the shared namespace environment continue to
+    operate within the legacy shared namespace environment by default via the
+    API (e.g. maas-cli and Juju).
+    """
+    for token in get_auth_tokens(user_from):
+        consumer = token.consumer
+        consumer.user = user_dest
+        consumer.save()
+        token.user = user_dest
+        token.save()
+
+
+def give_node_to_user(node, user):
+    """Changes a node's ownership for the legacy shared environment."""
+    node.owner = user
+    node.save()
+
+
+def migrate_to_user(user):
+    """Migrate files and nodes to the specified user.
+
+    This also copies, to the destination user, the public SSH keys of any
+    owned nodes' owners. This is so that those users who had allocated nodes
+    (i.e. active users of a shared-namespace environment) can access newly
+    created nodes in the legacy shared-namespace environment.
+    """
+    for unowned_file in get_unowned_files():
+        give_file_to_user(unowned_file, user)
+    for node_owner in get_owned_nodes_owners():
+        copy_ssh_keys(node_owner, user)
+        give_api_credentials_to_user(node_owner, user)
+    for owned_node in get_owned_nodes():
+        give_node_to_user(owned_node, user)
+
+
+def migrate():
+    """Migrate files to a per-tenant namespace."""
+    if get_unowned_files().exists():
+        # 2, 3, and 4
+        user = get_destination_user()
+        migrate_to_user(user)
+    else:
+        # 1 and 1a
+        pass

=== added file 'src/maasserver/support/pertenant/tests/test_migration.py'
--- src/maasserver/support/pertenant/tests/test_migration.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/support/pertenant/tests/test_migration.py	2013-03-06 15:28:20 +0000
@@ -0,0 +1,384 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test `maasserver.support.pertenant.migration."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+from django.contrib.auth.models import User
+from maasserver.models import (
+    Node,
+    SSHKey,
+    )
+from maasserver.support.pertenant import migration
+from maasserver.support.pertenant.migration import (
+    copy_ssh_keys,
+    get_destination_user,
+    get_legacy_user,
+    get_owned_nodes,
+    get_owned_nodes_owners,
+    get_real_users,
+    get_ssh_keys,
+    get_unowned_files,
+    give_api_credentials_to_user,
+    give_file_to_user,
+    give_node_to_user,
+    legacy_user_name,
+    migrate,
+    migrate_to_user,
+    )
+from maasserver.support.pertenant.tests.test_utils import (
+    make_provider_state_file,
+    )
+from maasserver.testing import (
+    get_data,
+    reload_object,
+    )
+from maasserver.testing.factory import factory
+from maasserver.testing.testcase import TestCase
+from mock import (
+    call,
+    sentinel,
+    )
+from testtools.matchers import MatchesStructure
+
+
+def get_ssh_key_string(num=0):
+    return get_data('data/test_rsa%d.pub' % num)
+
+
+class TestFunctions(TestCase):
+
+    def find_legacy_user(self):
+        return User.objects.filter(username=legacy_user_name)
+
+    def test_get_legacy_user_creates_user(self):
+        self.assertEqual([], list(self.find_legacy_user()))
+        legacy_user = get_legacy_user()
+        self.assertEqual([legacy_user], list(self.find_legacy_user()))
+        self.assertThat(
+            legacy_user, MatchesStructure.byEquality(
+                first_name="Shared", last_name="Environment",
+                email=legacy_user_name + "@localhost", is_active=True))
+
+    def test_get_legacy_user_creates_user_only_once(self):
+        legacy_user1 = get_legacy_user()
+        self.assertEqual([legacy_user1], list(self.find_legacy_user()))
+        legacy_user2 = get_legacy_user()
+        self.assertEqual([legacy_user2], list(self.find_legacy_user()))
+        self.assertEqual(legacy_user1, legacy_user2)
+
+    def test_get_unowned_files_no_files(self):
+        self.assertEqual([], list(get_unowned_files()))
+
+    def test_get_unowned_files(self):
+        user = factory.make_user()
+        files = [
+            factory.make_file_storage(owner=None),
+            factory.make_file_storage(owner=user),
+            factory.make_file_storage(owner=None),
+            ]
+        self.assertSetEqual(
+            {files[0], files[2]},
+            set(get_unowned_files()))
+
+    def test_get_real_users_no_users(self):
+        get_legacy_user()  # Ensure at least the legacy user exists.
+        self.assertEqual([], list(get_real_users()))
+
+    def test_get_real_users(self):
+        get_legacy_user()  # Ensure at least the legacy user exists.
+        users = [
+            factory.make_user(),
+            factory.make_user(),
+            ]
+        self.assertSetEqual(set(users), set(get_real_users()))
+
+    def test_get_owned_nodes_no_nodes(self):
+        self.assertEqual([], list(get_owned_nodes()))
+
+    def test_get_owned_nodes_no_owned_nodes(self):
+        factory.make_node()
+        self.assertEqual([], list(get_owned_nodes()))
+
+    def test_get_owned_nodes_with_owned_nodes(self):
+        nodes = {
+            factory.make_node(owner=factory.make_user()),
+            factory.make_node(owner=factory.make_user()),
+            }
+        self.assertSetEqual(nodes, set(get_owned_nodes()))
+
+    def test_get_owned_nodes_with_nodes_owned_by_system_users(self):
+        factory.make_node(owner=get_legacy_user()),
+        self.assertEqual([], list(get_owned_nodes()))
+
+    def test_get_owned_nodes_owners_no_users(self):
+        self.assertEqual([], list(get_owned_nodes_owners()))
+
+    def test_get_owned_nodes_owners_no_nodes(self):
+        factory.make_user()
+        self.assertEqual([], list(get_owned_nodes_owners()))
+
+    def test_get_owned_nodes_owners_no_owned_nodes(self):
+        factory.make_user()
+        factory.make_node(owner=None)
+        self.assertEqual([], list(get_owned_nodes_owners()))
+
+    def test_get_owned_nodes_owners(self):
+        user1 = factory.make_user()
+        user2 = factory.make_user()
+        factory.make_user()
+        factory.make_node(owner=user1)
+        factory.make_node(owner=user2)
+        factory.make_node(owner=None)
+        self.assertSetEqual({user1, user2}, set(get_owned_nodes_owners()))
+
+    def test_get_destination_user_one_real_user(self):
+        user = factory.make_user()
+        self.assertEqual(user, get_destination_user())
+
+    def test_get_destination_user_two_real_users(self):
+        factory.make_user()
+        factory.make_user()
+        self.assertEqual(get_legacy_user(), get_destination_user())
+
+    def test_get_destination_user_no_real_users(self):
+        self.assertEqual(get_legacy_user(), get_destination_user())
+
+    def test_get_destination_user_with_user_from_juju_state(self):
+        user1, user2 = factory.make_user(), factory.make_user()
+        node = factory.make_node(owner=user1)
+        make_provider_state_file(node)
+        self.assertEqual(user1, get_destination_user())
+
+    def test_get_destination_user_with_orphaned_juju_state(self):
+        user1, user2 = factory.make_user(), factory.make_user()
+        node = factory.make_node(owner=user1)
+        make_provider_state_file(node)
+        node.delete()  # Orphan the state.
+        self.assertEqual(get_legacy_user(), get_destination_user())
+
+
+class TestCopySSHKeys(TestCase):
+    """Tests for copy_ssh_keys()."""
+
+    def test_noop_when_there_are_no_keys(self):
+        user1 = factory.make_user()
+        user2 = factory.make_user()
+        copy_ssh_keys(user1, user2)
+        ssh_keys = SSHKey.objects.filter(user__in={user1, user2})
+        self.assertEqual([], list(ssh_keys))
+
+    def test_copy(self):
+        user1 = factory.make_user()
+        key1 = factory.make_sshkey(user1)
+        user2 = factory.make_user()
+        copy_ssh_keys(user1, user2)
+        user2s_ssh_keys = SSHKey.objects.filter(user=user2)
+        self.assertSetEqual(
+            {key1.key}, {ssh_key.key for ssh_key in user2s_ssh_keys})
+
+    def test_copy_is_idempotent(self):
+        # When the destination user already has a key, copy_ssh_keys() is a
+        # noop for that key.
+        user1 = factory.make_user()
+        key1 = factory.make_sshkey(user1)
+        user2 = factory.make_user()
+        key2 = factory.make_sshkey(user2, key1.key)
+        copy_ssh_keys(user1, user2)
+        user2s_ssh_keys = SSHKey.objects.filter(user=user2)
+        self.assertSetEqual(
+            {key2.key}, {ssh_key.key for ssh_key in user2s_ssh_keys})
+
+    def test_copy_does_not_clobber(self):
+        # When the destination user already has some keys, copy_ssh_keys()
+        # adds to them; it does not remove them.
+        user1 = factory.make_user()
+        key1 = factory.make_sshkey(user1, get_ssh_key_string(1))
+        user2 = factory.make_user()
+        key2 = factory.make_sshkey(user2, get_ssh_key_string(2))
+        copy_ssh_keys(user1, user2)
+        user2s_ssh_keys = SSHKey.objects.filter(user=user2)
+        self.assertSetEqual(
+            {key1.key, key2.key},
+            {ssh_key.key for ssh_key in user2s_ssh_keys})
+
+
+class TestGiveFileToUser(TestCase):
+
+    def test_give_unowned_file(self):
+        user = factory.make_user()
+        file = factory.make_file_storage(owner=None)
+        give_file_to_user(file, user)
+        self.assertEqual(user, file.owner)
+
+    def test_give_owned_file(self):
+        user1 = factory.make_user()
+        user2 = factory.make_user()
+        file = factory.make_file_storage(owner=user1)
+        give_file_to_user(file, user2)
+        self.assertEqual(user2, file.owner)
+
+    def test_file_saved(self):
+        user = factory.make_user()
+        file = factory.make_file_storage(owner=None)
+        save = self.patch(file, "save")
+        give_file_to_user(file, user)
+        save.assert_called_once()
+
+
+class TestGiveCredentialsToUser(TestCase):
+
+    def test_give(self):
+        user1 = factory.make_user()
+        user2 = factory.make_user()
+        profile = user1.get_profile()
+        consumer, token = profile.create_authorisation_token()
+        give_api_credentials_to_user(user1, user2)
+        self.assertEqual(user2, reload_object(consumer).user)
+        self.assertEqual(user2, reload_object(token).user)
+
+
+class TestGiveNodeToUser(TestCase):
+
+    def test_give(self):
+        user1 = factory.make_user()
+        user2 = factory.make_user()
+        node = factory.make_node(owner=user1)
+        give_node_to_user(node, user2)
+        self.assertEqual(user2, reload_object(node).owner)
+
+
+class TestMigrateToUser(TestCase):
+
+    def test_migrate(self):
+        # This is a mechanical test, to demonstrate that migrate_to_user() is
+        # wired up correctly: it should not really contain much logic because
+        # it is meant only as a convenient wrapper around other functions.
+        # Those functions are unit tested individually, and the overall
+        # behaviour of migrate() is tested too; this is another layer of
+        # verification. It's also a reminder not to stuff logic into
+        # migrate_to_user(); extract it into functions instead and unit test
+        # those.
+
+        # migrate_to_user() will give all unowned files to a specified user.
+        get_unowned_files = self.patch(migration, "get_unowned_files")
+        get_unowned_files.return_value = [sentinel.file1, sentinel.file2]
+        give_file_to_user = self.patch(migration, "give_file_to_user")
+        # migrate_to_user() will copy all SSH keys and give all API
+        # credentials belonging to node owners over to a specified user.
+        get_owned_nodes_owners = self.patch(
+            migration, "get_owned_nodes_owners")
+        get_owned_nodes_owners.return_value = [
+            sentinel.node_owner1, sentinel.node_owner2]
+        copy_ssh_keys = self.patch(migration, "copy_ssh_keys")
+        give_api_credentials_to_user = self.patch(
+            migration, "give_api_credentials_to_user")
+        # migrate_to_user() will give all owned nodes to a specified user.
+        get_owned_nodes = self.patch(migration, "get_owned_nodes")
+        get_owned_nodes.return_value = [sentinel.node1, sentinel.node2]
+        give_node_to_user = self.patch(migration, "give_node_to_user")
+
+        migrate_to_user(sentinel.user)
+
+        # Each unowned file is given to the destination user one at a time.
+        get_unowned_files.assert_called_once()
+        self.assertEqual(
+            [call(sentinel.file1, sentinel.user),
+             call(sentinel.file2, sentinel.user)],
+            give_file_to_user.call_args_list)
+        # The SSH keys of each node owner are copied to the destination user,
+        # one at a time, and the credentials of these users are given to the
+        # destination user.
+        get_owned_nodes_owners.assert_called_once()
+        self.assertEqual(
+            [call(sentinel.node_owner1, sentinel.user),
+             call(sentinel.node_owner2, sentinel.user)],
+            copy_ssh_keys.call_args_list)
+        self.assertEqual(
+            [call(sentinel.node_owner1, sentinel.user),
+             call(sentinel.node_owner2, sentinel.user)],
+            give_api_credentials_to_user.call_args_list)
+        # Each owned node is given to the destination user one at a time.
+        get_owned_nodes.assert_called_once()
+        self.assertEqual(
+            [call(sentinel.node1, sentinel.user),
+             call(sentinel.node2, sentinel.user)],
+            give_node_to_user.call_args_list)
+
+
+class TestMigrate(TestCase):
+
+    def test_migrate_runs_when_no_files_exist(self):
+        migrate()
+
+    def test_migrate_runs_when_no_unowned_files_exist(self):
+        factory.make_file_storage(owner=factory.make_user())
+        migrate()
+
+    def test_migrate_all_files_to_single_user_when_only_one_user(self):
+        user = factory.make_user()
+        stored = factory.make_file_storage(owner=None)
+        migrate()
+        self.assertEqual(user, reload_object(stored).owner)
+
+    def test_migrate_all_files_to_new_legacy_user_when_multiple_users(self):
+        stored = factory.make_file_storage(owner=None)
+        user1 = factory.make_user()
+        user2 = factory.make_user()
+        migrate()
+        stored = reload_object(stored)
+        self.assertNotIn(stored.owner, {user1, user2, None})
+
+    def test_migrate_all_nodes_to_new_legacy_user_when_multiple_users(self):
+        factory.make_file_storage(owner=None)
+        user1 = factory.make_user()
+        node1 = factory.make_node(owner=user1)
+        user2 = factory.make_user()
+        node2 = factory.make_node(owner=user2)
+        migrate()
+        self.assertNotIn(reload_object(node1).owner, {user1, user2, None})
+        self.assertNotIn(reload_object(node2).owner, {user1, user2, None})
+
+    def test_migrate_all_nodes_to_bootstrap_owner_when_multiple_users(self):
+        user1 = factory.make_user()
+        node1 = factory.make_node(owner=user1)
+        user2 = factory.make_user()
+        node2 = factory.make_node(owner=user2)
+        make_provider_state_file(node1)
+        migrate()
+        self.assertEqual(
+            (user1, user1),
+            (reload_object(node1).owner,
+             reload_object(node2).owner))
+
+    def test_migrate_ancillary_data_to_legacy_user_when_multiple_users(self):
+        factory.make_file_storage(owner=None)
+        # Create two users, both with API credentials, an SSH key and a node.
+        user1 = factory.make_user()
+        consumer1, token1 = user1.get_profile().create_authorisation_token()
+        key1 = factory.make_sshkey(user1, get_ssh_key_string(1))
+        node1 = factory.make_node(owner=user1)
+        user2 = factory.make_user()
+        consumer2, token2 = user2.get_profile().create_authorisation_token()
+        key2 = factory.make_sshkey(user2, get_ssh_key_string(2))
+        node2 = factory.make_node(owner=user2)
+        migrate()
+        # The SSH keys have been copied to the legacy user.
+        legacy_user = get_legacy_user()
+        legacy_users_ssh_keys = get_ssh_keys(legacy_user)
+        self.assertSetEqual({key1.key, key2.key}, set(legacy_users_ssh_keys))
+        # The API credentials have been moved to the legacy user.
+        legacy_users_nodes = Node.objects.filter(owner=legacy_user)
+        self.assertSetEqual({node1, node2}, set(legacy_users_nodes))
+        self.assertEqual(
+            (legacy_user, legacy_user, legacy_user, legacy_user),
+            (reload_object(consumer1).user, reload_object(token1).user,
+             reload_object(consumer2).user, reload_object(token2).user))

=== modified file 'src/maasserver/tests/data/test_rsa0.pub'
--- src/maasserver/tests/data/test_rsa0.pub	2012-04-06 09:52:45 +0000
+++ src/maasserver/tests/data/test_rsa0.pub	2013-03-06 15:28:20 +0000
@@ -1,1 +1,1 @@
-ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDdrzzDZNwyMVBvBTT6kBnrfPZv/AUbkxj7G5CaMTdw6xkKthV22EntD3lxaQxRKzQTfCc2d/CC1K4ushCcRs1S6SQ2zJ2jDq1UmOUkDMgvNh4JVhJYSKc6mu8i3s7oGSmBado5wvtlpSzMrscOpf8Qe/wmT5fH12KB9ipJqoFNQMVbVcVarE/v6wpn3GZC62YRb5iaz9/M+t92Qhu50W2u+KfouqtKB2lwIDDKZMww38ExtdMouh2FZpxaoh4Uey5bRp3tM3JgnWcX6fyUOp2gxJRPIlD9rrZhX5IkEkZM8MQbdPTQLgIf98oFph5RG6w1t02BvI9nJKM7KkKEfBHt ubuntu@server-7476
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDdrzzDZNwyMVBvBTT6kBnrfPZv/AUbkxj7G5CaMTdw6xkKthV22EntD3lxaQxRKzQTfCc2d/CC1K4ushCcRs1S6SQ2zJ2jDq1UmOUkDMgvNh4JVhJYSKc6mu8i3s7oGSmBado5wvtlpSzMrscOpf8Qe/wmT5fH12KB9ipJqoFNQMVbVcVarE/v6wpn3GZC62YRb5iaz9/M+t92Qhu50W2u+KfouqtKB2lwIDDKZMww38ExtdMouh2FZpxaoh4Uey5bRp3tM3JgnWcX6fyUOp2gxJRPIlD9rrZhX5IkEkZM8MQbdPTQLgIf98oFph5RG6w1t02BvI9nJKM7KkKEfBHt ubuntu@test_rsa0.pub

=== modified file 'src/maasserver/tests/data/test_rsa1.pub'
--- src/maasserver/tests/data/test_rsa1.pub	2012-04-06 09:52:45 +0000
+++ src/maasserver/tests/data/test_rsa1.pub	2013-03-06 15:28:20 +0000
@@ -1,1 +1,1 @@
-ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC6Gkj1y8/0T7q/FqBSr9xRBO9GzT+JeoWNXaqhUBg179Zd53XM4qblVwz/rsMa70te8CYNIFU+GbcNY1tNCo78NlHjQA8H98COnbVWKxvABECHrJ8nbYB4lWH9wI8/uvR0um6yUb/tZYbiSqnQxhoGAF/uQQfhqzc+tc7uTjnsa6krrNqQCdpFbAVVy+vZzvcJl6CX8nu5uJ8jedWfXOZJFcQPH+VwkUT0oV+1zVeLpE4LFkRO52JrC9Dy1xgrYM0EhcrShBdD1GQx9IXdW4Z9PIaVcq/y4Qv574yHMvi+6hwG6xpCtRXmy0lG0LiG60c1yOredkO6U0MJIVbeZ/+r ubuntu@server-7493
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC6Gkj1y8/0T7q/FqBSr9xRBO9GzT+JeoWNXaqhUBg179Zd53XM4qblVwz/rsMa70te8CYNIFU+GbcNY1tNCo78NlHjQA8H98COnbVWKxvABECHrJ8nbYB4lWH9wI8/uvR0um6yUb/tZYbiSqnQxhoGAF/uQQfhqzc+tc7uTjnsa6krrNqQCdpFbAVVy+vZzvcJl6CX8nu5uJ8jedWfXOZJFcQPH+VwkUT0oV+1zVeLpE4LFkRO52JrC9Dy1xgrYM0EhcrShBdD1GQx9IXdW4Z9PIaVcq/y4Qv574yHMvi+6hwG6xpCtRXmy0lG0LiG60c1yOredkO6U0MJIVbeZ/+r ubuntu@test_rsa1.pub

=== modified file 'src/maasserver/tests/data/test_rsa2.pub'
--- src/maasserver/tests/data/test_rsa2.pub	2012-04-06 09:52:45 +0000
+++ src/maasserver/tests/data/test_rsa2.pub	2013-03-06 15:28:20 +0000
@@ -1,1 +1,1 @@
-ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDKVdMk4Q+13uUvXjb6iU+oB2Auk0HpaILZ8Pw/V63PTJ+QXtEp0vTe6DEvr9uF2vl6tF+AosiG4krEwqBNGx/h8MmFO7BgNTxn9eU2VwfHzmQ2nqkXHsXgp66cNT0Yd0nfvVV/fsMpKN9fUaYrXjAlFxvC9iQ33Rp6vj/X+oqDvYf3xZjbuZy+BxdJnmiTAJcFouTyrdy1Em1EZITq5M4EXw93/O2vAPYSFPAeELBE+mIMJxOCY1Fm101oAqO0qof3Rb2hZxc2WINjmqZIxoi+sviU0ny/dIFknhYEg1Xh2hObPn0nN5+4VHjBTdRmpRXqggotc53sYC5udVmFsW8B ubuntu@server-7493
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDKVdMk4Q+13uUvXjb6iU+oB2Auk0HpaILZ8Pw/V63PTJ+QXtEp0vTe6DEvr9uF2vl6tF+AosiG4krEwqBNGx/h8MmFO7BgNTxn9eU2VwfHzmQ2nqkXHsXgp66cNT0Yd0nfvVV/fsMpKN9fUaYrXjAlFxvC9iQ33Rp6vj/X+oqDvYf3xZjbuZy+BxdJnmiTAJcFouTyrdy1Em1EZITq5M4EXw93/O2vAPYSFPAeELBE+mIMJxOCY1Fm101oAqO0qof3Rb2hZxc2WINjmqZIxoi+sviU0ny/dIFknhYEg1Xh2hObPn0nN5+4VHjBTdRmpRXqggotc53sYC5udVmFsW8B ubuntu@test_rsa2.pub

=== modified file 'src/maasserver/tests/data/test_rsa3.pub'
--- src/maasserver/tests/data/test_rsa3.pub	2012-04-06 09:52:45 +0000
+++ src/maasserver/tests/data/test_rsa3.pub	2013-03-06 15:28:20 +0000
@@ -1,1 +1,1 @@
-ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDai2ir5yxckoYTHUbFL6pe01Kx+Dy6nw9p7LhFaBixUOh8G7eIgFBguYcir2ZKBfM/lbTnW+MSiGF2VMlXX0+X9Ux2iwPSJa2wIA7Cc5prCz/RnMRKQ+2S1JJuORoi8tDI0p1R0sGWMXCwaj30oRN0THWz884+d3YlDD/O39h74gnLNEx/TQig/r/Aev3VfeKO6dlbbX81vSad2JVncislyMq1TgJdhn2/JI8t+LW0xVc6ZgQr94YB2M2DNjFSisP2vDrV5LWM+IqiF8T/YHkcSsANr8WWvZWa79uHyRBU3xr2qZZqMjMVL0B/NOJYXyGBIJ7HQnlVLmqFenKl8ZtL ubuntu@server-7493
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDai2ir5yxckoYTHUbFL6pe01Kx+Dy6nw9p7LhFaBixUOh8G7eIgFBguYcir2ZKBfM/lbTnW+MSiGF2VMlXX0+X9Ux2iwPSJa2wIA7Cc5prCz/RnMRKQ+2S1JJuORoi8tDI0p1R0sGWMXCwaj30oRN0THWz884+d3YlDD/O39h74gnLNEx/TQig/r/Aev3VfeKO6dlbbX81vSad2JVncislyMq1TgJdhn2/JI8t+LW0xVc6ZgQr94YB2M2DNjFSisP2vDrV5LWM+IqiF8T/YHkcSsANr8WWvZWa79uHyRBU3xr2qZZqMjMVL0B/NOJYXyGBIJ7HQnlVLmqFenKl8ZtL ubuntu@test_rsa3.pub

=== modified file 'src/maasserver/tests/data/test_rsa4.pub'
--- src/maasserver/tests/data/test_rsa4.pub	2012-04-06 09:52:45 +0000
+++ src/maasserver/tests/data/test_rsa4.pub	2013-03-06 15:28:20 +0000
@@ -1,1 +1,1 @@
-ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDNDA4vXVTxHuKikIXeA6/K/X7hKpJcOJV0HcXUHlSNa9phNW0f8vbci+BxcLAqIz/U+BPiQ9lCxz7so+qCTFrM4poOdkTyup8VUxUqntiaxgiCJZ1of+eMe39+S9XQk6RogiCpExanhD9xPLkK/mLr5phnQwDjEDJwD4OOF0rYsbYoqje/0Pd+Tm0PIepq/qwsu5PAKPJU8dfnp8BWLCuIJ+DA2lfRUjmxWwLczfM/4hu1bZlYp1mzJJgMIOY92/pUToYxvBiIiKs3qWh6HC5Vxo5Vz4w5WLnTnIPDvpYBvWj8LGXJwHuhqlzed2icwPk8krip2BzwsHotru3UXtKf ubuntu@server-7493
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDNDA4vXVTxHuKikIXeA6/K/X7hKpJcOJV0HcXUHlSNa9phNW0f8vbci+BxcLAqIz/U+BPiQ9lCxz7so+qCTFrM4poOdkTyup8VUxUqntiaxgiCJZ1of+eMe39+S9XQk6RogiCpExanhD9xPLkK/mLr5phnQwDjEDJwD4OOF0rYsbYoqje/0Pd+Tm0PIepq/qwsu5PAKPJU8dfnp8BWLCuIJ+DA2lfRUjmxWwLczfM/4hu1bZlYp1mzJJgMIOY92/pUToYxvBiIiKs3qWh6HC5Vxo5Vz4w5WLnTnIPDvpYBvWj8LGXJwHuhqlzed2icwPk8krip2BzwsHotru3UXtKf ubuntu@test_rsa4.pub

=== modified file 'src/maasserver/tests/test_sshkey.py'
--- src/maasserver/tests/test_sshkey.py	2012-05-29 10:24:47 +0000
+++ src/maasserver/tests/test_sshkey.py	2013-03-06 15:28:20 +0000
@@ -226,7 +226,7 @@
         key = SSHKey(key=key_string, user=user)
         display = key.display_html()
         self.assertEqual(
-            'ssh-rsa AAAAB3NzaC1yc2E… ubuntu@server-7476', display)
+            'ssh-rsa AAAAB3NzaC1yc… ubuntu@test_rsa0.pub', display)
 
     def test_sshkey_display_is_marked_as_HTML_safe(self):
         key_string = get_data('data/test_rsa0.pub')