Merge lp:~newell-jensen/maas/fix-1685835 into lp:~maas-committers/maas/trunk

[Newell Jensen]

Ignore remote storage if the InitiatorIQN is non-empty in pre-composed nodes. This information is taken into account for calculating the total, available, and used iscsi storage on the RSD pod.

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

=== modified file 'src/provisioningserver/drivers/pod/rsd.py'
--- src/provisioningserver/drivers/pod/rsd.py	2017-04-24 20:10:24 +0000
+++ src/provisioningserver/drivers/pod/rsd.py	2017-04-25 23:06:27 +0000
@@ -519,17 +519,36 @@
                 discovered_machine_block_device)
 
     def get_pod_machine_remote_storages(
-            self, node_data, url, headers, logical_drives,
+            self, node_data, url, headers, remote_drives, logical_drives,
             targets, discovered_machine, request=None):
         """Get pod machine remote storages."""
-        remote_drives = node_data.get('Links', {}).get('RemoteDrives', [])
-        for remote_drive in remote_drives:
+        node_remote_drives = node_data.get('Links', {}).get('RemoteDrives', [])
+        remote_drives_to_delete = []
+        logical_drives_to_delete = []
+        for node_remote_drive in node_remote_drives:
+            target_data = targets[
+                node_remote_drive['@odata.id'].lstrip('/').encode('utf-8')]
+            initiator = target_data.get('Initiator')[0]
+            initiator_iqn = initiator.get('iSCSI', {}).get('InitiatorIQN')
+            if initiator_iqn:
+                # Since InitiatorIQN is not an empty string we will not be
+                # including this storage into MAAS.
+                # Remove this remote drive, target, and associated logical
+                # drives associated with this target.
+                for lv, lv_data in logical_drives.items():
+                    lv_targets = lv_data.get('Links', {}).get('Targets', [])
+                    for lv_target in lv_targets:
+                        if (node_remote_drive['@odata.id'] ==
+                                lv_target['@odata.id']):
+                            remote_drives_to_delete.append(
+                                node_remote_drive['@odata.id'])
+                            logical_drives_to_delete.append(lv)
+                continue
+
             discovered_machine_block_device = (
                 DiscoveredMachineBlockDevice(
                     model=None, serial=None, size=0,
                     type=BlockDeviceType.ISCSI))
-            target_data = targets[
-                remote_drive['@odata.id'].lstrip('/').encode('utf-8')]
             addresses = target_data.get('Addresses')[0]
             host = addresses.get('iSCSI', {}).get('TargetPortalIP')
             proto = '6'  # curtin currently only supports TCP.
@@ -547,9 +566,11 @@
             # find which one contains this remote drive.
             for lv, lv_data in logical_drives.items():
                 lv_targets = lv_data.get('Links', {}).get('Targets', [])
-                if remote_drive in lv_targets:
-                    discovered_machine_block_device.size = float(
-                        lv_data['CapacityGiB']) * (1024 ** 3)
+                for lv_target in lv_targets:
+                    if (node_remote_drive['@odata.id'] ==
+                            lv_target['@odata.id']):
+                        discovered_machine_block_device.size = float(
+                            lv_data['CapacityGiB']) * (1024 ** 3)
 
             # Map the tags from the request block devices to the discovered
             # block devices. This ensures that the composed machine has the
@@ -570,6 +591,16 @@
             discovered_machine.block_devices.append(
                 discovered_machine_block_device)
 
+        # Remove the remote drives, targests, and logical drives that
+        # are no longer needed.  These will be used in later calculations
+        # for the total usable iscsi remote storage.
+        for remote_drive in set(remote_drives_to_delete):
+            del targets[
+                remote_drive.lstrip('/').encode('utf-8')]
+            remote_drives.remove(remote_drive)
+        for logical_drive in set(logical_drives_to_delete):
+            del logical_drives[logical_drive]
+
     @inlineCallbacks
     def get_pod_machine_interfaces(
             self, node_data, url, headers, discovered_machine):
@@ -627,7 +658,8 @@
 
     @inlineCallbacks
     def get_pod_machine(
-            self, node, url, headers, logical_drives, targets, request=None):
+            self, node, url, headers, remote_drives,
+            logical_drives, targets, request=None):
         """Get pod composed machine.
 
         If required resources cannot be found, this
@@ -661,7 +693,7 @@
             node_data, url, headers, discovered_machine, request)
         # Get remote storages.
         self.get_pod_machine_remote_storages(
-            node_data, url, headers, logical_drives,
+            node_data, url, headers, remote_drives, logical_drives,
             targets, discovered_machine, request)
         # Get interfaces.
         yield self.get_pod_machine_interfaces(
@@ -674,7 +706,8 @@
 
     @inlineCallbacks
     def get_pod_machines(
-            self, url, headers, logical_drives, targets, request=None):
+            self, url, headers, remote_drives,
+            logical_drives, targets, request=None):
         """Get pod composed machines.
 
         If required resources cannot be found, these
@@ -688,7 +721,8 @@
         # Iterate over all composed nodes in the pod.
         for node in nodes:
             discovered_machine = yield self.get_pod_machine(
-                node, url, headers, logical_drives, targets, request)
+                node, url, headers, remote_drives,
+                logical_drives, targets, request)
             discovered_machines.append(discovered_machine)
         return discovered_machines
 
@@ -729,21 +763,24 @@
         logical_drives, targets = yield self.scrape_logical_drives_and_targets(
             url, headers)
         remote_drives = yield self.scrape_remote_drives(url, headers)
+
+        # Discover composed machines.
+        pod_machines = yield self.get_pod_machines(
+            url, headers, remote_drives, logical_drives, targets)
+
+        # Discover pod resources.
         pod_remote_storage, pod_hints_remote_storage = (
             self.calculate_pod_remote_storage(
                 remote_drives, logical_drives, targets))
-
-        # Discover pod resources.
         discovered_pod = yield self.get_pod_resources(url, headers)
 
+        # Add machines to pod.
+        discovered_pod.machines = pod_machines
+
         # Discover pod remote storage resources.
         discovered_pod.capabilities.append(Capabilities.ISCSI_STORAGE)
         discovered_pod.iscsi_storage = pod_remote_storage
 
-        # Discover composed machines.
-        discovered_pod.machines = yield self.get_pod_machines(
-            url, headers, logical_drives, targets)
-
         # Discover pod hints.
         discovered_pod.hints = self.get_pod_hints(discovered_pod)
         discovered_pod.hints.iscsi_storage = pod_hints_remote_storage

=== modified file 'src/provisioningserver/drivers/pod/tests/test_rsd.py'
--- src/provisioningserver/drivers/pod/tests/test_rsd.py	2017-04-24 20:10:24 +0000
+++ src/provisioningserver/drivers/pod/tests/test_rsd.py	2017-04-25 23:06:27 +0000
@@ -1013,7 +1013,7 @@
             b"redfish/v1/Services/1/Targets/3": SAMPLE_JSON_TARGET,
         }
         remote_drives = set(
-            "redfish/v1/Services/1/Targets/1")
+            b"/redfish/v1/Services/1/Targets/1")
 
         remote_storage = driver.calculate_remote_storage(
             remote_drives, logical_drives, targets)
@@ -1313,23 +1313,51 @@
         context = make_context()
         url = driver.get_url(context)
         headers = driver.make_auth_headers(**context)
-        node_data = SAMPLE_JSON_NODE
         discovered_machine = make_discovered_machine(block_devices=[])
-        TARGET_CHANGED = deepcopy(SAMPLE_JSON_TARGET)
-        TARGET_CHANGED['Addresses'][0]['iSCSI']['TargetLUN'].append({'LUN': 3})
+        node_data = deepcopy(SAMPLE_JSON_NODE)
+        node_data['Links']['RemoteDrives'].append({
+            "@odata.id": "/redfish/v1/Services/1/Targets/3"})
+        LV_CHANGED = deepcopy(SAMPLE_JSON_LV)
+        LV_CHANGED['Links']['Targets'].append({
+            "@odata.id": "/redfish/v1/Services/1/Targets/3"})
+        TARGET_CHANGED_1 = deepcopy(SAMPLE_JSON_TARGET)
+        TARGET_CHANGED_2 = deepcopy(SAMPLE_JSON_TARGET)
+        TARGET_CHANGED_1['Addresses'][0]['iSCSI']['TargetLUN'].append(
+            {'LUN': 3})
+        TARGET_CHANGED_2['Initiator'][0]['iSCSI']['InitiatorIQN'] = "ALL"
+        remote_drives = set([
+            "/redfish/v1/Services/1/Targets/1",
+            "/redfish/v1/Services/1/Targets/2",
+            "/redfish/v1/Services/1/Targets/3",
+        ])
         logical_drives = {
             b"redfish/v1/Services/1/LogicalDrives/1": SAMPLE_JSON_LV,
             b"redfish/v1/Services/1/LogicalDrives/2": SAMPLE_JSON_LVG,
-            b"redfish/v1/Services/1/LogicalDrives/3": SAMPLE_JSON_LV
+            b"redfish/v1/Services/1/LogicalDrives/3": LV_CHANGED,
         }
         targets = {
             b"redfish/v1/Services/1/Targets/1": SAMPLE_JSON_TARGET,
-            b"redfish/v1/Services/1/Targets/2": TARGET_CHANGED,
+            b"redfish/v1/Services/1/Targets/2": TARGET_CHANGED_1,
+            b"redfish/v1/Services/1/Targets/3": TARGET_CHANGED_2,
         }
 
         driver.get_pod_machine_remote_storages(
-            node_data, url, headers, logical_drives,
+            node_data, url, headers, remote_drives, logical_drives,
             targets, discovered_machine)
+        self.assertEquals(
+            set([
+                "/redfish/v1/Services/1/Targets/1",
+                "/redfish/v1/Services/1/Targets/2"]), remote_drives)
+        self.assertEquals(
+            {
+                b"redfish/v1/Services/1/LogicalDrives/1": SAMPLE_JSON_LV,
+                b"redfish/v1/Services/1/LogicalDrives/2": SAMPLE_JSON_LVG
+            }, logical_drives)
+        self.assertEquals(
+            {
+                b"redfish/v1/Services/1/Targets/1": SAMPLE_JSON_TARGET,
+                b"redfish/v1/Services/1/Targets/2": TARGET_CHANGED_1
+            }, targets)
         self.assertThat(
             discovered_machine.block_devices, MatchesListwise([
                 MatchesStructure(
@@ -1368,6 +1396,8 @@
         discovered_machine = make_discovered_machine(block_devices=[])
         TARGET_CHANGED = deepcopy(SAMPLE_JSON_TARGET)
         TARGET_CHANGED['Addresses'][0]['iSCSI']['TargetLUN'].append({'LUN': 3})
+        remote_drives = set(
+            b"/redfish/v1/Services/1/Targets/1")
         logical_drives = {
             b"redfish/v1/Services/1/LogicalDrives/1": SAMPLE_JSON_LV,
             b"redfish/v1/Services/1/LogicalDrives/2": SAMPLE_JSON_LVG,
@@ -1379,7 +1409,7 @@
         }
 
         driver.get_pod_machine_remote_storages(
-            node_data, url, headers, logical_drives,
+            node_data, url, headers, remote_drives, logical_drives,
             targets, discovered_machine, request)
         self.assertThat(
             discovered_machine.block_devices, MatchesListwise([
@@ -1481,6 +1511,8 @@
         node_data = SAMPLE_JSON_NODE
         TARGET_CHANGED = deepcopy(SAMPLE_JSON_TARGET)
         TARGET_CHANGED['Addresses'][0]['iSCSI']['TargetLUN'].append({'LUN': 3})
+        remote_drives = set(
+            b"/redfish/v1/Services/1/Targets/1")
         logical_drives = {
             b"redfish/v1/Services/1/LogicalDrives/1": SAMPLE_JSON_LV,
             b"redfish/v1/Services/1/LogicalDrives/2": SAMPLE_JSON_LVG,
@@ -1506,7 +1538,7 @@
             driver, 'get_pod_machine_interfaces')
 
         machine = yield driver.get_pod_machine(
-            b"redfish/v1/Nodes/1", url, headers,
+            b"redfish/v1/Nodes/1", url, headers, remote_drives,
             logical_drives, targets, request)
         self.assertEquals(node_data['Name'], machine.hostname)
         self.assertEquals(
@@ -1523,8 +1555,8 @@
             MockCalledOnceWith(node_data, url, headers, machine, request))
         self.assertThat(
             mock_get_pod_machine_remote_storages,
-            MockCalledOnceWith(node_data, url, headers, logical_drives,
-                               targets, machine, request))
+            MockCalledOnceWith(node_data, url, headers, remote_drives,
+                               logical_drives, targets, machine, request))
         self.assertThat(
             mock_get_pod_machine_interfaces,
             MockCalledOnceWith(node_data, url, headers, machine))
@@ -1535,6 +1567,8 @@
         context = make_context()
         url = driver.get_url(context)
         headers = driver.make_auth_headers(**context)
+        remote_drives = set(
+            b"redfish/v1/Services/1/Targets/1")
         logical_drives = {
             factory.make_name('lv_path'): factory.make_name('lv_data')
             for _ in range(3)
@@ -1557,11 +1591,11 @@
         mock_get_pod_machine.return_value = expected_machines
 
         discovered_machines = yield driver.get_pod_machines(
-            url, headers, logical_drives, targets)
+            url, headers, remote_drives, logical_drives, targets)
         self.assertEquals(1, len(discovered_machines))
         self.assertThat(mock_get_pod_machine, MockCalledOnceWith(
             b"redfish/v1/Nodes/1", url, headers,
-            logical_drives, targets, None))
+            remote_drives, logical_drives, targets, None))
 
     def test__get_pod_hints(self):
         driver = RSDPodDriver()
@@ -1638,7 +1672,7 @@
             mock_get_pod_resources, MockCalledOnceWith(url, headers))
         self.assertThat(
             mock_get_pod_machines, MockCalledOnceWith(
-                url, headers, logical_drives, targets))
+                url, headers, remote_drives, logical_drives, targets))
         self.assertThat(mock_get_pod_hints, MockCalledOnceWith(
             mock_get_pod_resources.return_value))
 
@@ -1815,7 +1849,7 @@
             for _ in range(3)
         }
         remote_drives = set([
-            factory.make_name('target_path')
+            factory.make_name('target_path').encode('utf-8')
             for _ in range(3)
         ])
         mock_scrape_logical_drives_and_targets = self.patch(