Merge ~alexsander-souza/maas:lp1943444_fix_lxd_boot_order into maas:master

[Alexsander de Souza]

Preserve request's device order

Whenever is possible preserve the original device order from the request.

Fixes LP#1943444

The disk with the path set to root ('/') is always reported last by LXD, while MAAS expects that the order in the request is preserved.

This patch fixes this by building the discovered devices list in the original order, as currently there's no way to change the order LXD reports the devices (devices are sorted by path, the default storage must have the path set to root while the others must not have a path defined)

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: d2941a0d402e583094274301dc6af368dd30dc01

review: Approve

[Alberto Donato (ack)]

Can't you just use the device name in the config rather than indexes?
AFAIK LXD requires the first disk to be called "root".

Also, please add at least one test for the change

review: Needs Information

[Alexsander de Souza (alexsander-souza)]

> Can't you just use the device name in the config rather than indexes?
The existing code uses indexes, so I just followed it. I'll check if more robust approach is possible.

> AFAIK LXD requires the first disk to be called "root".
No, LXD doesn't care about the labels. The root disk is identified by `path = /`.

>
> Also, please add at least one test for the change
Will do

[Alexsander de Souza (alexsander-souza)]

The request doesn't have the device names, so we can only match by index

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 460c45e75beaddd625bdfc45942809c8fc2cae2c

review: Approve

[Alberto Donato (ack)]

+1

one minor nit inline

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/11125/console
COMMIT: 04efcb54749d32f8c981ca295342009249aea96d

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 61f5a129bf72849792e212796f55c0ef63a7124c

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

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

[Alexsander de Souza (alexsander-souza)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/11131/console
COMMIT: 61f5a129bf72849792e212796f55c0ef63a7124c

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 9f5b0d345fda64e5742d94d536d347b72812be3c

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

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

[Alexsander de Souza (alexsander-souza)]

jenkins: !test

[MAAS Lander (maas-lander)]

LANDING
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

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

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1943444_fix_lxd_boot_order lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 9f5b0d345fda64e5742d94d536d347b72812be3c

review: Approve

diff --git a/src/provisioningserver/drivers/pod/lxd.py b/src/provisioningserver/drivers/pod/lxd.py
index 9261931..0b54702 100644
--- a/src/provisioningserver/drivers/pod/lxd.py
+++ b/src/provisioningserver/drivers/pod/lxd.py
@@ -731,6 +731,7 @@ class LXDPodDriver(PodDriver):
         for name, device in machine.expanded_devices.items():
             if device["type"] == "disk":
                 requested_device = None
+                index = -1
                 if request:
                     # for composed VMs, the root disk is always the first
                     # one. Adjust the index so that it matches the requested
@@ -741,11 +742,16 @@ class LXDPodDriver(PodDriver):
                         extra_block_devices += 1
                         index = extra_block_devices
                     requested_device = request.block_devices[index]
-                block_devices.append(
-                    _get_discovered_block_device(
-                        name, device, requested_device=requested_device
-                    )
+
+                blkdev = _get_discovered_block_device(
+                    name, device, requested_device=requested_device
                 )
+
+                if index >= 0:
+                    block_devices.insert(index, blkdev)
+                else:
+                    block_devices.append(blkdev)
+
             elif device["type"] == "nic":
                 interfaces.append(
                     _get_discovered_interface(name, device, not interfaces)
diff --git a/src/provisioningserver/drivers/pod/tests/test_lxd.py b/src/provisioningserver/drivers/pod/tests/test_lxd.py
index 05e8397..a337955 100644
--- a/src/provisioningserver/drivers/pod/tests/test_lxd.py
+++ b/src/provisioningserver/drivers/pod/tests/test_lxd.py
@@ -1391,24 +1391,55 @@ class TestLXDPodDriver(MAASTestCase):
         pod_id = factory.make_name("pod_id")
         request = make_requested_machine(num_disks=2)
         self.fake_lxd.profiles.exists.return_value = False
-        mock_storage_pools = Mock()
-        self.fake_lxd.storage_pools.all.return_value = mock_storage_pools
+
         mock_get_usable_storage_pool = self.patch(
             self.driver, "_get_usable_storage_pool"
         )
         # a volume is created for the second disk
         volume = Mock()
+        volume.config = {
+            "size": str(request.block_devices[1].size),
+        }
         volume.name = factory.make_name("vol")
         usable_pool = Mock()
         usable_pool.name = factory.make_name("pool")
         usable_pool.volumes.create.return_value = volume
+        usable_pool.volumes.get.return_value = volume
         mock_get_usable_storage_pool.return_value = usable_pool
-        mock_machine = Mock()
-        self.fake_lxd.virtual_machines.create.return_value = mock_machine
-        mock_get_discovered_machine = self.patch(
-            self.driver, "_get_discovered_machine"
+        self.fake_lxd.storage_pools.get.return_value = usable_pool
+
+        # LXD sorts the devices in a way the root dist is always last
+        expanded_devices = {
+            "disk1": {
+                "path": "",
+                "pool": usable_pool.name,
+                "source": volume.name,
+                "type": "disk",
+            },
+            "eth0": {
+                "boot.priority": "1",
+                "name": "eth0",
+                "nictype": "bridged",
+                "parent": "maas-kvm",
+                "type": "nic",
+            },
+            "root": {
+                "path": "/",
+                "pool": usable_pool.name,
+                "size": str(request.block_devices[0].size),
+                "type": "disk",
+                "boot.priority": 0,
+            },
+        }
+        mock_machine = Mock(
+            name=factory.make_name(),
+            architecture=debian_to_kernel_architecture(request.architecture),
+            expanded_devices=expanded_devices,
+            expanded_config={},
+            status_code=101,
         )
-        mock_get_discovered_machine.return_value = sentinel.discovered_machine
+        self.fake_lxd.virtual_machines.create.return_value = mock_machine
+
         definition = {
             "name": request.hostname,
             "architecture": debian_to_kernel_architecture(
@@ -1452,7 +1483,17 @@ class TestLXDPodDriver(MAASTestCase):
         self.fake_lxd.virtual_machines.create.assert_called_once_with(
             definition, wait=True
         )
-        self.assertEqual(sentinel.discovered_machine, discovered_machine)
+
+        # assert the device order was preserved
+        self.assertEqual(
+            request.block_devices[0].size,
+            discovered_machine.block_devices[0].size,
+        )
+        self.assertEqual(
+            request.block_devices[1].size,
+            discovered_machine.block_devices[1].size,
+        )
+
         # a volume for the additional disk is created
         usable_pool.volumes.create.assert_called_with(
             "custom",