Merge lp:~blake-rouse/maas/fix-migration-181-1.9 into lp:maas/1.9

[Blake Rouse]

Fix migration 181 to not call into the current model code. This was originally implement as a short cut and this does not work. As soon as any little thing changes in the model the migration cannot complete.

Includes drive-by fix for "make package".

This doesn't have a bug because it wasn't really a problem in 1.9, but could become one as things get changes as bugs are fixed.

[MAAS Lander (maas-lander)]

Voting does not meet specified criteria. Required: Approve >= 1, Disapprove == 0. Got: 1 Pending.

[Blake Rouse (blake-rouse)]

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~blake-rouse/maas/fix-migration-181-1.9 into lp:maas/1.9 failed. Below is the output from the failed tests.

Ign http://prodstack-zone-2.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://security.ubuntu.com trusty-security InRelease [64.4 kB]
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates InRelease [64.4 kB]
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports InRelease
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty Release
Get:3 http://security.ubuntu.com trusty-security/main Sources [100 kB]
Get:4 http://security.ubuntu.com trusty-security/universe Sources [31.5 kB]
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main Sources [247 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [380 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted Sources [5,359 B]
Get:8 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe Sources [144 kB]
Get:9 http://security.ubuntu.com trusty-security/universe amd64 Packages [120 kB]
Get:10 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse Sources [5,162 B]
Get:11 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [663 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:12 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted amd64 Packages [15.9 kB]
Get:13 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [328 kB]
Get:14 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse amd64 Packages [13.0 kB]
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse Translation-en
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted Translation-en
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/restricted Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/universe Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/multiverse Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/restricted amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/universe amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/multiverse amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main Translation-en
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/multiverse Translation-en
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-ba...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~blake-rouse/maas/fix-migration-181-1.9 into lp:maas/1.9 failed. Below is the output from the failed tests.

Ign http://prodstack-zone-2.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates InRelease [64.4 kB]
Get:2 http://security.ubuntu.com trusty-security InRelease [64.4 kB]
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports InRelease
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty Release
Get:3 http://security.ubuntu.com trusty-security/main Sources [100 kB]
Get:4 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main Sources [247 kB]
Get:5 http://security.ubuntu.com trusty-security/universe Sources [31.5 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [380 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted Sources [5,359 B]
Get:8 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe Sources [144 kB]
Get:9 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse Sources [5,162 B]
Get:10 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [663 kB]
Get:11 http://security.ubuntu.com trusty-security/universe amd64 Packages [120 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:12 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted amd64 Packages [15.9 kB]
Get:13 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [329 kB]
Get:14 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse amd64 Packages [13.0 kB]
Get:15 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main Translation-en [327 kB]
Get:16 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse Translation-en [6,832 B]
Get:17 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted Translation-en [3,699 B]
Get:18 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe Translation-en [173 kB]
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/restricted Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/universe Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/multiverse Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/restricted amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/universe amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/multiverse amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main Translation-en
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/multiverse Translation-en
Hit http://pr...

=== modified file 'src/maasserver/migrations/0181_initial_storage_layouts.py'
--- src/maasserver/migrations/0181_initial_storage_layouts.py	2015-09-30 07:29:27 +0000
+++ src/maasserver/migrations/0181_initial_storage_layouts.py	2015-12-02 15:05:52 +0000
@@ -1,6 +1,9 @@
 from django.db import models
 from maasserver.enum import NODE_STATUS
-from maasserver.models.node import Node
+from maasserver.models.migrations.create_default_storage_layout import (
+    clear_full_storage_configuration,
+    create_lvm_layout,
+)
 from south.db import db
 from south.utils import datetime_utils as datetime
 from south.v2 import DataMigration
@@ -20,19 +23,41 @@
 class Migration(DataMigration):
 
     def forwards(self, orm):
-        # We don't use orm here because we want the current Node model to
-        # include all of the storage methods we require. This will break
-        # if the node these methods get removed from the Node, so we assert
-        # that they at least exists.
-        assert hasattr(Node, "_clear_full_storage_configuration")
-        assert hasattr(Node, "set_default_storage_layout")
-        for node in Node.objects.filter(installable=True):
+        PartitionTable = orm['maasserver.PartitionTable']
+        Partition = orm['maasserver.Partition']
+        Filesystem = orm['maasserver.Filesystem']
+        FilesystemGroup = orm['maasserver.FilesystemGroup']
+        PhysicalBlockDevice = orm['maasserver.PhysicalBlockDevice']
+        VirtualBlockDevice = orm['maasserver.VirtualBlockDevice']
+        for node in orm['maasserver.Node'].objects.filter(installable=True):
             if (node.status in IGNORE_STATUS and
                     node.blockdevice_set.count() == 0):
                 # Node needs to be the correct status and have storage devices.
                 continue
-            node._clear_full_storage_configuration()
-            node.set_default_storage_layout()
+
+            # Clear the current storage configration just to be sure.
+            clear_full_storage_configuration(
+                node,
+                PhysicalBlockDevice=PhysicalBlockDevice,
+                VirtualBlockDevice=VirtualBlockDevice,
+                PartitionTable=PartitionTable,
+                Filesystem=Filesystem,
+                FilesystemGroup=FilesystemGroup)
+
+            # Create the LVM layout on the boot disk.
+            boot_device = node.boot_disk
+            if boot_device is None:
+                boot_device = PhysicalBlockDevice.objects.filter(
+                    node=node).order_by('id').first()
+            if boot_device is not None:
+                create_lvm_layout(
+                    node,
+                    boot_device,
+                    PartitionTable=PartitionTable,
+                    Partition=Partition,
+                    Filesystem=Filesystem,
+                    FilesystemGroup=FilesystemGroup,
+                    VirtualBlockDevice=VirtualBlockDevice)
 
     def backwards(self, orm):
         # No need to go backward.

=== added file 'src/maasserver/models/migrations/create_default_storage_layout.py'
--- src/maasserver/models/migrations/create_default_storage_layout.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/models/migrations/create_default_storage_layout.py	2015-12-02 15:05:52 +0000
@@ -0,0 +1,128 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Migration to create the default storage layout on the boot disk.
+
+WARNING: Although these methods will become obsolete very quickly, they
+cannot be removed, since they are used by the
+0181_initial_storage_layouts DataMigration. (changing them might also
+be futile unless a customer restores from a backup, since any bugs that occur
+will have already occurred, and this code will not be executed again.)
+
+Note: Each helper must have its dependencies on any model classes injected,
+since the migration environment is a skeletal replication of the 'real'
+database model. So each function takes as parameters the model classes it
+requires. Importing from the model is not allowed here. (but the unit tests
+do it, to ensure that the migrations meet validation requirements.)
+"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    "clear_full_storage_configuration",
+    "create_lvm_layout",
+]
+
+from datetime import datetime
+
+from maasserver.enum import (
+    FILESYSTEM_GROUP_TYPE,
+    FILESYSTEM_TYPE,
+    PARTITION_TABLE_TYPE,
+)
+from maasserver.models.filesystemgroup import LVM_PE_SIZE
+from maasserver.models.partition import (
+    MAX_PARTITION_SIZE_FOR_MBR,
+    MIN_PARTITION_SIZE,
+    PARTITION_ALIGNMENT_SIZE,
+)
+from maasserver.models.partitiontable import PARTITION_TABLE_EXTRA_SPACE
+from maasserver.utils.converters import round_size_to_nearest_block
+
+
+def clear_full_storage_configuration(
+        node,
+        PhysicalBlockDevice, VirtualBlockDevice,
+        PartitionTable, Filesystem, FilesystemGroup):
+    """Clear's the full storage configuration for this node."""
+    physical_block_devices = PhysicalBlockDevice.objects.filter(node=node)
+    PartitionTable.objects.filter(
+        block_device__in=physical_block_devices).delete()
+    Filesystem.objects.filter(
+        block_device__in=physical_block_devices).delete()
+    for block_device in VirtualBlockDevice.objects.filter(node=node):
+        try:
+            block_device.filesystem_group.virtual_devices.all().delete()
+            block_device.filesystem_group.delete()
+        except FilesystemGroup.DoesNotExist:
+            # When a filesystem group has multiple virtual block devices
+            # it is possible that accessing `filesystem_group` will
+            # result in it already being deleted.
+            pass
+
+
+def create_lvm_layout(
+        node, boot_disk,
+        PartitionTable, Partition, Filesystem, FilesystemGroup,
+        VirtualBlockDevice):
+    """Create the lvm layout for the boot disk."""
+    # Create the partition table and root partition.
+    now = datetime.now()
+    partition_table = PartitionTable.objects.create(
+        block_device=boot_disk, table_type=PARTITION_TABLE_TYPE.MBR,
+        created=now, updated=now)
+    total_size = 0
+    available_size = boot_disk.size - PARTITION_TABLE_EXTRA_SPACE
+    partition_size = round_size_to_nearest_block(
+        available_size, PARTITION_ALIGNMENT_SIZE, False)
+    max_mbr_size = round_size_to_nearest_block(
+        MAX_PARTITION_SIZE_FOR_MBR, PARTITION_ALIGNMENT_SIZE, False)
+    if partition_size > max_mbr_size:
+        partition_size = max_mbr_size
+    available_size -= partition_size
+    total_size += partition_size
+    root_partition = Partition.objects.create(
+        partition_table=partition_table, size=partition_size, bootable=True,
+        created=now, updated=now)
+
+    # Add the extra partitions if there is more space.
+    partitions = [root_partition]
+    while available_size > MIN_PARTITION_SIZE:
+        size = round_size_to_nearest_block(
+            available_size, PARTITION_ALIGNMENT_SIZE, False)
+        if size > max_mbr_size:
+            size = max_mbr_size
+        partitions.append(
+            Partition.objects.create(
+                partition_table=partition_table, size=size, bootable=False,
+                created=now, updated=now))
+        available_size -= size
+        total_size += size
+
+    # Create the volume group and logical volume.
+    volume_group = FilesystemGroup.objects.create(
+        name="vgroot", group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
+        created=now, updated=now)
+    for partition in partitions:
+        Filesystem.objects.create(
+            fstype=FILESYSTEM_TYPE.LVM_PV, partition=partition,
+            filesystem_group=volume_group, created=now, updated=now)
+    number_of_extents, _ = divmod(total_size, LVM_PE_SIZE)
+    lv_size = (number_of_extents - len(partitions)) * LVM_PE_SIZE
+    logical_volume = VirtualBlockDevice.objects.create(
+        node=node, name="lvroot", size=lv_size, block_size=4096,
+        filesystem_group=volume_group, created=now, updated=now)
+    Filesystem.objects.create(
+        block_device=logical_volume,
+        fstype=FILESYSTEM_TYPE.EXT4,
+        label="root",
+        mount_point="/",
+        created=now,
+        updated=now)

=== added file 'src/maasserver/models/migrations/tests/test_create_default_storage_layout.py'
--- src/maasserver/models/migrations/tests/test_create_default_storage_layout.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/models/migrations/tests/test_create_default_storage_layout.py	2015-12-02 15:05:52 +0000
@@ -0,0 +1,204 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the `0181_initial_storage_layouts` migration.
+
+WARNING: These tests will become obsolete very quickly, as they are testing
+migrations against fields that may be removed. When these tests become
+obsolete, they should be skipped. The tests should be kept until at least
+the next release cycle (through MAAS 1.9) in case any bugs with this migration
+occur.
+"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+from maasserver.enum import (
+    FILESYSTEM_GROUP_TYPE,
+    FILESYSTEM_TYPE,
+)
+from maasserver.models import (
+    Filesystem,
+    FilesystemGroup,
+    Partition,
+    PartitionTable,
+    PhysicalBlockDevice,
+    VirtualBlockDevice,
+    VolumeGroup,
+)
+from maasserver.models.migrations.create_default_storage_layout import (
+    clear_full_storage_configuration,
+    create_lvm_layout,
+)
+from maasserver.testing.factory import factory
+from maasserver.testing.orm import reload_object
+from maasserver.testing.testcase import MAASServerTestCase
+from testtools.matchers import (
+    Is,
+    MatchesStructure,
+    Not,
+)
+
+
+class TestClearFullStorageConfigration(MAASServerTestCase):
+
+    def test__clears_all_objects(self):
+        node = factory.make_Node()
+        physical_block_devices = [
+            factory.make_PhysicalBlockDevice(node=node, size=10 * 1000 ** 3)
+            for _ in range(3)
+            ]
+        filesystem = factory.make_Filesystem(
+            block_device=physical_block_devices[0])
+        partition_table = factory.make_PartitionTable(
+            block_device=physical_block_devices[1])
+        partition = factory.make_Partition(partition_table=partition_table)
+        fslvm = factory.make_Filesystem(
+            block_device=physical_block_devices[2],
+            fstype=FILESYSTEM_TYPE.LVM_PV)
+        vgroup = factory.make_FilesystemGroup(
+            group_type=FILESYSTEM_GROUP_TYPE.LVM_VG, filesystems=[fslvm])
+        vbd1 = factory.make_VirtualBlockDevice(
+            filesystem_group=vgroup, size=2 * 1000 ** 3)
+        vbd2 = factory.make_VirtualBlockDevice(
+            filesystem_group=vgroup, size=3 * 1000 ** 3)
+        filesystem_on_vbd1 = factory.make_Filesystem(
+            block_device=vbd1, fstype=FILESYSTEM_TYPE.LVM_PV)
+        vgroup_on_vgroup = factory.make_FilesystemGroup(
+            group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
+            filesystems=[filesystem_on_vbd1])
+        vbd3_on_vbd1 = factory.make_VirtualBlockDevice(
+            filesystem_group=vgroup_on_vgroup, size=1 * 1000 ** 3)
+        clear_full_storage_configuration(
+            node,
+            PhysicalBlockDevice=PhysicalBlockDevice,
+            VirtualBlockDevice=VirtualBlockDevice,
+            PartitionTable=PartitionTable,
+            Filesystem=Filesystem,
+            FilesystemGroup=FilesystemGroup)
+        for pbd in physical_block_devices:
+            self.expectThat(
+                reload_object(pbd), Not(Is(None)),
+                "Physical block device should not have been deleted.")
+        self.expectThat(
+            reload_object(filesystem), Is(None),
+            "Filesystem should have been removed.")
+        self.expectThat(
+            reload_object(partition_table), Is(None),
+            "PartitionTable should have been removed.")
+        self.expectThat(
+            reload_object(partition), Is(None),
+            "Partition should have been removed.")
+        self.expectThat(
+            reload_object(fslvm), Is(None),
+            "LVM PV Filesystem should have been removed.")
+        self.expectThat(
+            reload_object(vgroup), Is(None),
+            "Volume group should have been removed.")
+        self.expectThat(
+            reload_object(vbd1), Is(None),
+            "Virtual block device should have been removed.")
+        self.expectThat(
+            reload_object(vbd2), Is(None),
+            "Virtual block device should have been removed.")
+        self.expectThat(
+            reload_object(filesystem_on_vbd1), Is(None),
+            "Filesystem on virtual block device should have been removed.")
+        self.expectThat(
+            reload_object(vgroup_on_vgroup), Is(None),
+            "Volume group on virtual block device should have been removed.")
+        self.expectThat(
+            reload_object(vbd3_on_vbd1), Is(None),
+            "Virtual block device on another virtual block device should have "
+            "been removed.")
+
+
+class TestCreateLVMLayout(MAASServerTestCase):
+
+    def test__creates_layout_for_1TiB_disk(self):
+        node = factory.make_Node(with_boot_disk=False)
+        boot_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=1024 ** 4, block_size=512)
+        create_lvm_layout(
+            node,
+            boot_disk,
+            PartitionTable=PartitionTable,
+            Partition=Partition,
+            Filesystem=Filesystem,
+            FilesystemGroup=FilesystemGroup,
+            VirtualBlockDevice=VirtualBlockDevice)
+
+        # Validate the volume group on root partition.
+        partition_table = boot_disk.get_partitiontable()
+        partitions = partition_table.partitions.order_by('id').all()
+        root_partition = partitions[0]
+        volume_group = VolumeGroup.objects.get(
+            filesystems__partition=root_partition)
+        self.assertIsNotNone(volume_group)
+        self.assertEquals("vgroot", volume_group.name)
+
+        # Validate one logical volume on volume group.
+        self.assertEquals(
+            1, volume_group.virtual_devices.count(),
+            "Should have only 1 logical volume.")
+        logical_volume = volume_group.virtual_devices.first()
+        self.assertEquals(volume_group.get_size(), logical_volume.size)
+        self.assertEquals("lvroot", logical_volume.name)
+        self.assertThat(
+            logical_volume.get_effective_filesystem(),
+            MatchesStructure.byEquality(
+                fstype=FILESYSTEM_TYPE.EXT4,
+                label="root",
+                mount_point="/",
+                ))
+
+    def test__creates_layout_for_3TiB_disk(self):
+        node = factory.make_Node(with_boot_disk=False)
+        boot_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=3 * (1024 ** 4), block_size=512)
+        create_lvm_layout(
+            node,
+            boot_disk,
+            PartitionTable=PartitionTable,
+            Partition=Partition,
+            Filesystem=Filesystem,
+            FilesystemGroup=FilesystemGroup,
+            VirtualBlockDevice=VirtualBlockDevice)
+
+        # Validate the volume group on root partition.
+        partition_table = boot_disk.get_partitiontable()
+        partitions = partition_table.partitions.order_by('id').all()
+        root_partition = partitions[0]
+        volume_group = VolumeGroup.objects.get(
+            filesystems__partition=root_partition)
+        self.assertIsNotNone(volume_group)
+        self.assertEquals("vgroot", volume_group.name)
+
+        # Since its a 3TiB disk it should have 2 partitions in
+        # the volume group.
+        self.assertEquals(
+            2, volume_group.filesystems.count(),
+            "Should have 2 partitions in volume group.")
+
+        # Validate one logical volume on volume group.
+        self.assertEquals(
+            1, volume_group.virtual_devices.count(),
+            "Should have only 1 logical volume.")
+        logical_volume = volume_group.virtual_devices.first()
+        self.assertEquals(volume_group.get_size(), logical_volume.size)
+        self.assertEquals("lvroot", logical_volume.name)
+        self.assertThat(
+            logical_volume.get_effective_filesystem(),
+            MatchesStructure.byEquality(
+                fstype=FILESYSTEM_TYPE.EXT4,
+                label="root",
+                mount_point="/",
+                ))