Merge ~newell-jensen/maas:lp1800483 into maas:master

[Newell Jensen]

LP: #1800483 -- Do not use thin provisioning for LVM storage pools in virsh.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1800483 lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: ba48cee423bc0e43087c10c46c3188fe1aa45f9f

review: Approve

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

diff --git a/src/provisioningserver/drivers/pod/tests/test_virsh.py b/src/provisioningserver/drivers/pod/tests/test_virsh.py
index 502ec3d..0c9498a 100644
--- a/src/provisioningserver/drivers/pod/tests/test_virsh.py
+++ b/src/provisioningserver/drivers/pod/tests/test_virsh.py
@@ -1140,7 +1140,7 @@ class TestVirshSSH(MAASTestCase):
         self.patch(
             virsh.VirshSSH, "get_pod_storage_pools").return_value = pools
         self.assertEqual(
-            pools[1].name,
+            (pools[1].type, pools[1].name),
             conn.get_usable_pool(disk))
 
     def test_get_usable_pool_filters_on_disk_tags(self):
@@ -1160,7 +1160,7 @@ class TestVirshSSH(MAASTestCase):
         self.patch(
             virsh.VirshSSH, "get_pod_storage_pools").return_value = pools
         self.assertEqual(
-            selected_pool.name,
+            (selected_pool.type, selected_pool.name),
             conn.get_usable_pool(disk))
 
     def test_get_usable_pool_filters_on_disk_tags_raises_invalid(self):
@@ -1199,7 +1199,7 @@ class TestVirshSSH(MAASTestCase):
         self.patch(
             virsh.VirshSSH, "get_pod_storage_pools").return_value = pools
         self.assertEqual(
-            selected_pool.name,
+            (selected_pool.type, selected_pool.name),
             conn.get_usable_pool(disk, selected_pool.id))
 
     def test_get_usable_pool_filters_on_default_pool_id_raises_invalid(self):
@@ -1238,7 +1238,7 @@ class TestVirshSSH(MAASTestCase):
         self.patch(
             virsh.VirshSSH, "get_pod_storage_pools").return_value = pools
         self.assertEqual(
-            selected_pool.name,
+            (selected_pool.type, selected_pool.name),
             conn.get_usable_pool(disk, selected_pool.name))
 
     def test_get_usable_pool_filters_on_default_pool_name_raises_invalid(self):
@@ -1264,7 +1264,7 @@ class TestVirshSSH(MAASTestCase):
     def test_create_local_volume_returns_None(self):
         conn = self.configure_virshssh('')
         self.patch(
-            virsh.VirshSSH, "get_usable_pool").return_value = None
+            virsh.VirshSSH, "get_usable_pool").return_value = (None, None)
         self.assertIsNone(conn.create_local_volume(random.randint(1000, 2000)))
 
     def test_create_local_volume_returns_tagged_pool_and_volume(self):
@@ -1281,8 +1281,9 @@ class TestVirshSSH(MAASTestCase):
     def test_create_local_volume_makes_call_returns_pool_and_volume(self):
         conn = self.configure_virshssh('')
         pool = factory.make_name('pool')
+        pool_type = factory.make_name('pool_type')
         self.patch(
-            virsh.VirshSSH, "get_usable_pool").return_value = pool
+            virsh.VirshSSH, "get_usable_pool").return_value = (pool_type, pool)
         mock_run = self.patch(virsh.VirshSSH, "run")
         disk = RequestedMachineBlockDevice(
             size=random.randint(1000, 2000), tags=[])
@@ -1293,6 +1294,21 @@ class TestVirshSSH(MAASTestCase):
         self.assertEqual(pool, used_pool)
         self.assertIsNotNone(volume_name)
 
+    def test_create_local_volume_makes_call_returns_pool_and_volume_lvm(self):
+        conn = self.configure_virshssh('')
+        pool = factory.make_name('pool')
+        self.patch(
+            virsh.VirshSSH, "get_usable_pool").return_value = ('logical', pool)
+        mock_run = self.patch(virsh.VirshSSH, "run")
+        disk = RequestedMachineBlockDevice(
+            size=random.randint(1000, 2000), tags=[])
+        used_pool, volume_name = conn.create_local_volume(disk)
+        self.assertThat(mock_run, MockCalledOnceWith([
+            'vol-create-as', used_pool, volume_name, str(disk.size),
+            '--format', 'raw']))
+        self.assertEqual(pool, used_pool)
+        self.assertIsNotNone(volume_name)
+
     def test_delete_local_volume(self):
         conn = self.configure_virshssh('')
         pool = factory.make_name('pool')
diff --git a/src/provisioningserver/drivers/pod/virsh.py b/src/provisioningserver/drivers/pod/virsh.py
index 69629d7..bc947c5 100644
--- a/src/provisioningserver/drivers/pod/virsh.py
+++ b/src/provisioningserver/drivers/pod/virsh.py
@@ -818,7 +818,7 @@ class VirshSSH(pexpect.spawn):
         return True
 
     def get_usable_pool(self, disk, default_pool=None):
-        """Return the pool that has enough space for `disk.size`."""
+        """Return the pool and type that has enough space for `disk.size`."""
         pools = self.get_pod_storage_pools(with_available=True)
         filtered_pools = [
             pool
@@ -828,7 +828,7 @@ class VirshSSH(pexpect.spawn):
         if filtered_pools:
             for pool in filtered_pools:
                 if disk.size <= pool.available:
-                    return pool.name
+                    return pool.type, pool.name
             raise PodInvalidResources(
                 "Not enough storage space on storage pools: %s" % (
                     ', '.join([pool.name for pool in filtered_pools])))
@@ -847,7 +847,7 @@ class VirshSSH(pexpect.spawn):
             if filtered_pools:
                 default_pool = filtered_pools[0]
                 if disk.size <= default_pool.available:
-                    return default_pool.name
+                    return default_pool.type, default_pool.name
                 raise PodInvalidResources(
                     "Not enough space in default storage pool: %s" % (
                         default_pool.name))
@@ -855,20 +855,26 @@ class VirshSSH(pexpect.spawn):
                 "Default storage pool '%s' doesn't exist." % default_pool)
         for pool in pools:
             if disk.size <= pool.available:
-                return pool.name
+                return pool.type, pool.name
         raise PodInvalidResources(
             "Not enough storage space on any storage pools: %s" % (
                 ', '.join([pool.name for pool in pools])))
 
     def create_local_volume(self, disk, default_pool=None):
         """Create a local volume with `disk.size`."""
-        usable_pool = self.get_usable_pool(disk, default_pool)
+        usable_pool_type, usable_pool = self.get_usable_pool(
+            disk, default_pool)
         if usable_pool is None:
             return None
         volume = str(uuid4())
-        self.run([
-            'vol-create-as', usable_pool, volume, str(disk.size),
-            '--allocation', '0', '--format', 'raw'])
+        if usable_pool_type == 'logical':
+            self.run([
+                'vol-create-as', usable_pool, volume, str(disk.size),
+                '--format', 'raw'])
+        else:
+            self.run([
+                'vol-create-as', usable_pool, volume, str(disk.size),
+                '--allocation', '0', '--format', 'raw'])
         return usable_pool, volume
 
     def delete_local_volume(self, pool, volume):