Repository: softlayer/softlayer-python

Issue:
slcli vs capture --all and metadata
### Expected Behavior: 
Capture all disks including metadata

### Actual Behavior
Invalid block device supplied. Please be sure to not include any metadata disk block devices.

### Environment Information
Operating System: 
RHEL 6.7
softlayer-python version (`slcli --version`):
slcli (SoftLayer Command-line), version 4.1.1

Hello, 
I am trying to automate the capture of several VSIs. I have two devices with multiple disks and a metadata block device. 

The following slcli cmd is failing with the behavior above:
slcli vs capture --name="description" --note="additional notes" 17800945 --all True

Is it possible to identify only the disks I want to capture? Example Disk, 0,2,4 and exclude disk 7 (metadata)

Thanks

Discussion:

Patch:
diff --git a/.gitignore b/.gitignore
index 41170939..438f1d70 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,3 +12,5 @@ docs/_build/*
 build/*
 dist/*
 *.egg-info
+.cache
+
diff --git a/SoftLayer/fixtures/SoftLayer_Virtual_Guest.py b/SoftLayer/fixtures/SoftLayer_Virtual_Guest.py
index 10440668..966e8a40 100644
--- a/SoftLayer/fixtures/SoftLayer_Virtual_Guest.py
+++ b/SoftLayer/fixtures/SoftLayer_Virtual_Guest.py
@@ -33,10 +33,13 @@
     "primaryNetworkComponent": {"speed": 10, "maxSpeed": 100},
     'hourlyBillingFlag': False,
     'createDate': '2013-08-01 15:23:45',
-    'blockDevices': [{"device": 0, 'mountType': 'Disk', "uuid": 1},
-                     {"device": 1, 'mountType': 'Disk'},
-                     {"device": 2, 'mountType': 'CD'},
-                     {"device": 3, 'mountType': 'Disk', "uuid": 3}],
+    'blockDevices': [{'device': 0, 'mountType': 'Disk', "uuid": 1},
+                     {'device': 1, 'mountType': 'Disk',
+                      'diskImage': {'type': {'keyName': 'SWAP'}}},
+                     {'device': 2, 'mountType': 'CD'},
+                     {'device': 3, 'mountType': 'Disk', 'uuid': 3},
+                     {'device': 4, 'mountType': 'Disk', 'uuid': 4,
+                      'diskImage': {'metadataFlag': True}}],
     'notes': 'notes',
     'networkVlans': [{'networkSpace': 'PUBLIC',
                       'vlanNumber': 23,
diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py
index 3a89eb8c..9d0741c0 100644
--- a/SoftLayer/managers/vs.py
+++ b/SoftLayer/managers/vs.py
@@ -721,19 +721,42 @@ def capture(self, instance_id, name, additional_disks=False, notes=None):
             notes = "Some notes about this image"
             result = mgr.capture(instance_id=12345, name=name, notes=notes)
         """
-        vsi = self.get_instance(instance_id)
 
-        disk_filter = lambda x: x['device'] == '0'
-        # Skip disk 1 (swap partition) and CD mounts
-        if additional_disks:
-            disk_filter = lambda x: (str(x['device']) != '1' and
-                                     x['mountType'] != 'CD')
+        vsi = self.client.call(
+            'Virtual_Guest',
+            'getObject',
+            id=instance_id,
+            mask="""id,
+            blockDevices[id,device,mountType,
+            diskImage[id,metadataFlag,type[keyName]]]""")
 
-        disks = [block_device for block_device in vsi['blockDevices']
-                 if disk_filter(block_device)]
+        disks_to_capture = []
+        for block_device in vsi['blockDevices']:
+
+            # We never want metadata disks
+            if utils.lookup(block_device, 'diskImage', 'metadataFlag'):
+                continue
+
+            # We never want swap devices
+            type_name = utils.lookup(block_device,
+                                     'diskImage',
+                                     'type',
+                                     'keyName')
+            if type_name == 'SWAP':
+                continue
+
+            # We never want CD images
+            if block_device['mountType'] == 'CD':
+                continue
+
+            # Only use the first block device if we don't want additional disks
+            if not additional_disks and str(block_device['device']) != '0':
+                continue
+
+            disks_to_capture.append(block_device)
 
         return self.guest.createArchiveTransaction(
-            name, disks, notes, id=instance_id)
+            name, disks_to_capture, notes, id=instance_id)
 
     def upgrade(self, instance_id, cpus=None, memory=None,
                 nic_speed=None, public=True):


Test patch:
diff --git a/tests/managers/vs_tests.py b/tests/managers/vs_tests.py
index a0cecb20..fb1a2c2a 100644
--- a/tests/managers/vs_tests.py
+++ b/tests/managers/vs_tests.py
@@ -615,7 +615,7 @@ def test_captures(self):
 
         expected = fixtures.SoftLayer_Virtual_Guest.createArchiveTransaction
         self.assertEqual(result, expected)
-        args = ('a', [], None)
+        args = ('a', [{'device': 0, 'uuid': 1, 'mountType': 'Disk'}], None)
         self.assert_called_with('SoftLayer_Virtual_Guest',
                                 'createArchiveTransaction',
                                 args=args,