Merge ~ltrager/maas:lp1813019 into maas:master

[Lee Trager]

LP: #1813019 - Add a blank storage layout.

[Andres Rodriguez (andreserl)]

There is no such thing as a empty storage layout. So there should be an api operation to clear the storage whatever that is, not a layout because the lack of storage config means there is no layout

review: Disapprove

[Lee Trager (ltrager)]

In the bug report the user is asking for an easy way to clear the storage layout so they can apply their own custom storage layout using the API. During discussing of the updated storage layout this was mentioned and added to the new design[1] which is currently being implemented.

[1] https://app.zeplin.io/project/5c814428fe915cbc9cf6ccfa/screen/5c94d39579d02b35ea051b7c

[Andres Rodriguez (andreserl)]

There is no such thing as a empty storage layout. So there should be an api operation to clear the storage whatever that is, not a layout because the lack of storage config means there is no layout

review: Disapprove

[Lee Trager (ltrager)]

As discussed during SU I've renamed this from empty to blank. Users can apply the layout with the existing set-storage-layout and the websocket call. Users may also set the global configuration option, default_storage_layout, to blank if they wish however the default remains flat.

[Andres Rodriguez (andreserl)]

Lgtm!

Comment inline before landing

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b lp1813019 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5490/consoleText

[MAAS Lander (maas-lander)]

LANDING
-b lp1813019 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5491/consoleText

[MAAS Lander (maas-lander)]

LANDING
-b lp1813019 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5494/consoleText

[Andres Rodriguez (andreserl)]

From your latest change... why this hasn't been an issue and it is now? Why does this require 100GB disks on sampledata when it dind't before?

review: Needs Information

[Lee Trager (ltrager)]

VMFS6 requires at least 10G of space. In the VMFS6 review I upped LARGE_BLOCK_DEVICE to 100GB in maasserver.tests.test_storage_layouts to ensure unit tests pass. sampledata uses factory.make_PhysicalBlockDevice which allows a disk to be created as small as 64MB. This caused intermittent fails such as 5491 as well as cause make sampledata to fail.

[Andres Rodriguez (andreserl)]

That's not my question though. Why did this test not fail before and it is now failing, when this branch is simply dealing with a Blank layout?

review: Needs Information

[Lee Trager (ltrager)]

Before the random number generator happened to create a value big enough for tests to pass. The failure in 5491 happened because it generated a number to small. If you look at the stack trace in 5491 you can see its failing in the VMFS6Layout.clean(). I could remove that fix and keep trying to land until it generates a large enough number but we'd still have the same problem.

[Blake Rouse (blake-rouse)]

Looks good to me.

Thanks for fixing the sampledata, that failing was getting annoying.

review: Approve

[Andres Rodriguez (andreserl)]

The right fix should really be creating a disk for the specific tests rather than absolutely everything, because that gives good flexibility when testing other stuff and allows us to find issues elsewhere.

I'm not gonna block this landing on that but I would much rather prefer we raise the storage for just the esxi side.

review: Approve

[Lee Trager (ltrager)]

The way sampledata works is it creates a number of nodes with 1-5 disks attached to each. Once the node is created it then applies a randomly selected storage_layout. We don't know which layout until the node and disks are created. So the only way we can raise the minimium disk size just for VMFS datastores is by trying it and when it fails recreating the disks with a larger size.

diff --git a/src/maasserver/storage_layouts.py b/src/maasserver/storage_layouts.py
index 475c9ff..2bb5b34 100644
--- a/src/maasserver/storage_layouts.py
+++ b/src/maasserver/storage_layouts.py
@@ -605,12 +605,27 @@ class VMFS6Layout(StorageLayoutBase):
         return "VMFS6"
 
 
+class BlankStorageLayout(StorageLayoutBase):
+    """Blank layout.
+
+    This layout ensures no disk is configured with any partition table or
+    filesystem. This helps users who want to have a custom storage layout
+    not based on any existing layout.
+    """
+
+    def configure_storage(self, allow_fallback):
+        # StorageLayoutBase has the code to ensure nothing is configured.
+        # Once that is done there is nothing left for us to do.
+        return "blank"
+
+
 # Holds all the storage layouts that can be used.
 STORAGE_LAYOUTS = {
     "flat": ("Flat layout", FlatStorageLayout),
     "lvm": ("LVM layout", LVMStorageLayout),
     "bcache": ("Bcache layout", BcacheStorageLayout),
     "vmfs6": ("VMFS6 layout", VMFS6Layout),
+    "blank": ("No storage (blank) layout", BlankStorageLayout),
     }
 
 
diff --git a/src/maasserver/testing/sampledata.py b/src/maasserver/testing/sampledata.py
index 62f3e1c..285212a 100644
--- a/src/maasserver/testing/sampledata.py
+++ b/src/maasserver/testing/sampledata.py
@@ -1,4 +1,4 @@
-# Copyright 2016-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Construct sample application data dynamically."""
@@ -30,6 +30,7 @@ from maasserver.models import (
 )
 from maasserver.storage_layouts import STORAGE_LAYOUTS
 from maasserver.testing.factory import factory
+from maasserver.tests.test_storage_layouts import LARGE_BLOCK_DEVICE
 from maasserver.utils.orm import (
     get_one,
     transactional,
@@ -437,7 +438,9 @@ def populate_main():
 
         # Add random storage devices and set a random layout.
         for _ in range(random.randint(1, 5)):
-            factory.make_PhysicalBlockDevice(node=machine)
+            factory.make_PhysicalBlockDevice(
+                node=machine, size=random.randint(
+                    LARGE_BLOCK_DEVICE, LARGE_BLOCK_DEVICE * 10))
         if status in [
                 NODE_STATUS.READY,
                 NODE_STATUS.ALLOCATED,
diff --git a/src/maasserver/tests/test_storage_layouts.py b/src/maasserver/tests/test_storage_layouts.py
index 4fcd911..ef2df86 100644
--- a/src/maasserver/tests/test_storage_layouts.py
+++ b/src/maasserver/tests/test_storage_layouts.py
@@ -24,6 +24,7 @@ from maasserver.models.partitiontable import (
 from maasserver.storage_layouts import (
     BcacheStorageLayout,
     BcacheStorageLayoutBase,
+    BlankStorageLayout,
     calculate_size_from_percentage,
     EFI_PARTITION_SIZE,
     FlatStorageLayout,
@@ -33,6 +34,7 @@ from maasserver.storage_layouts import (
     LVMStorageLayout,
     MIN_BOOT_PARTITION_SIZE,
     MIN_ROOT_PARTITION_SIZE,
+    STORAGE_LAYOUTS,
     StorageLayoutBase,
     StorageLayoutFieldsError,
     StorageLayoutForm,
@@ -84,6 +86,7 @@ class TestFormHelpers(MAASServerTestCase):
             ("lvm", "LVM layout"),
             ("bcache", "Bcache layout"),
             ("vmfs6", "VMFS6 layout"),
+            ('blank', 'No storage (blank) layout'),
             ], get_storage_layout_choices())
 
     def test_get_storage_layout_for_node(self):
@@ -1648,3 +1651,30 @@ class TestVMFS6Layout(MAASServerTestCase):
         self.assertEqual({
             "size": ["Boot disk must be atleast 10G."],
             }, error.message_dict)
+
+
+class TestBlankStorageLayout(MAASServerTestCase):
+
+    def __init__(self, *args, **kwargs):
+        # Make sure any existing storage layout can be cleared.
+        self.scenarios = [
+            (layout_name, {'layout_class': layout_class})
+            for layout_name, layout_class in STORAGE_LAYOUTS.values()
+        ]
+        super().__init__(*args, **kwargs)
+
+    def test__creates_blank_layout(self):
+        node = factory.make_Node(with_boot_disk=False)
+        for _ in range(5):
+            factory.make_PhysicalBlockDevice(
+                node=node, size=LARGE_BLOCK_DEVICE)
+        # Apply another layout to test clearing it
+        other_layout = self.layout_class(node)
+        other_layout.configure()
+
+        layout = BlankStorageLayout(node)
+        self.assertEquals("blank", layout.configure())
+        self.assertFalse(node.virtualblockdevice_set.exists())
+        for bd in node.blockdevice_set.all():
+            self.assertFalse(bd.filesystem_set.exists())
+            self.assertFalse(bd.partitiontable_set.exists())