Fix the way storage space is looked up, remove os type requirement for file storage ordering

SoftLayer/CLI/file/order.py

@@ -28,17 +28,6 @@
               help='Endurance Storage Tier (IOP per GB)'
               '  [required for storage-type endurance]',
               type=click.Choice(['0.25', '2', '4', '10']))
-@click.option('--os-type',
-              help='Operating System',
-              type=click.Choice([
-                  'HYPER_V',
-                  'LINUX',
-                  'VMWARE',
-                  'WINDOWS_2008',
-                  'WINDOWS_GPT',
-                  'WINDOWS',
-                  'XEN']),
-              required=True)
 @click.option('--location',
               help='Datacenter short name (e.g.: dal09)',
               required=True)
@@ -48,7 +37,7 @@
               'space along with endurance file storage; specifies '
               'the size (in GB) of snapshot space to order')
 @environment.pass_env
-def cli(env, storage_type, size, iops, tier, os_type,
+def cli(env, storage_type, size, iops, tier,
         location, snapshot_size):
     """Order a file storage volume."""
     file_manager = SoftLayer.FileStorageManager(env.client)
@@ -79,8 +68,7 @@ def cli(env, storage_type, size, iops, tier, os_type,
                 storage_type='performance_storage_nfs',
                 location=location,
                 size=size,
-                iops=iops,
-                os_type=os_type
+                iops=iops
             )
         except ValueError as ex:
             raise exceptions.ArgumentError(str(ex))
@@ -98,7 +86,6 @@ def cli(env, storage_type, size, iops, tier, os_type,
                 location=location,
                 size=size,
                 tier_level=float(tier),
-                os_type=os_type,
                 snapshot_size=snapshot_size
             )
         except ValueError as ex:

SoftLayer/managers/block.py

@@ -271,7 +271,7 @@ def order_block_volume(self, storage_type, location, size, os_type,
                     package,
                     'performance_storage_iscsi'
                     ),
-                storage_utils.find_performance_space_price(package, iops),
+                storage_utils.find_performance_space_price(package, size),
                 storage_utils.find_performance_iops_price(package, size, iops),
             ]
         elif storage_type == 'storage_service_enterprise':

SoftLayer/managers/file.py

@@ -228,20 +228,23 @@ def delete_snapshot(self, snapshot_id):
         return self.client.call('Network_Storage', 'deleteObject',
                                 id=snapshot_id)
 
-    def order_file_volume(self, storage_type, location, size, os_type,
+    def order_file_volume(self, storage_type, location, size, os_type=None,
                           iops=None, tier_level=None, snapshot_size=None):
         """Places an order for a file volume.
 
         :param storage_type: "performance_storage_iscsi" (performance)
                              or "storage_service_enterprise" (endurance)
         :param location: Datacenter in which to order iSCSI volume
         :param size: Size of the desired volume, in GB
-        :param os_type: OS Type to use for volume alignment, see help for list
+        :param os_type: Not used for file storage orders, leave None
         :param iops: Number of IOPs for a "Performance" order
         :param tier_level: Tier level to use for an "Endurance" order
         :param snapshot_size: The size of optional snapshot space,
         if snapshot space should also be ordered (None if not ordered)
         """
+        if os_type:
+            raise exceptions.SoftLayerError(
+                'OS type is not used on file storage orders')
 
         try:
             location_id = storage_utils.get_location_id(self, location)
@@ -259,7 +262,7 @@ def order_file_volume(self, storage_type, location, size, os_type,
                     package,
                     'performance_storage_nfs'
                     ),
-                storage_utils.find_performance_space_price(package, iops),
+                storage_utils.find_performance_space_price(package, size),
                 storage_utils.find_performance_iops_price(package, size, iops),
             ]
         elif storage_type == 'storage_service_enterprise':
@@ -288,7 +291,6 @@ def order_file_volume(self, storage_type, location, size, os_type,
         order = {
             'complexType': complex_type,
             'packageId': package['id'],
-            'osFormatType': {'keyName': os_type},
             'prices': prices,
             'quantity': 1,
             'location': location_id,
@@ -313,6 +315,15 @@ def enable_snapshots(self, volume_id, schedule_type, retention_count,
 
         :param integer volume_id: The id of the volume
         :param string schedule_type: 'HOURLY'|'DAILY'|'WEEKLY'
+        :param integer retention_count: The number of snapshots to attempt to
+        retain in this schedule
+        :param integer minute: The minute of the hour at which HOURLY, DAILY,
+        and WEEKLY snapshots should be taken
+        :param integer hour: The hour of the day at which DAILY and WEEKLY
+        snapshots should be taken
+        :param string|integer day_of_week: The day of the week on which WEEKLY
+        snapshots should be taken, either as a string ('SUNDAY') or integer
+        ('0' is Sunday)
         :return: Returns whether successfully scheduled or not
         """
 
@@ -340,7 +351,7 @@ def order_snapshot_space(self, volume_id, capacity, tier,
                              upgrade, **kwargs):
         """Orders snapshot space for the given file volume.
 
-        :param integer volume_id: The id of the volume
+        :param integer volume_id: The ID of the volume
         :param integer capacity: The capacity to order, in GB
         :param float tier: The tier level of the file volume, in IOPS per GB
         :param boolean upgrade: Flag to indicate if this order is an upgrade
@@ -390,7 +401,7 @@ def cancel_snapshot_space(self, volume_id,
 
         :param integer volume_id: The volume ID
         :param string reason: The reason for cancellation
-        :param boolean immediate_flag: Cancel immediately or
+        :param boolean immediate: Cancel immediately or
         on anniversary date
         """
 
@@ -422,9 +433,9 @@ def cancel_snapshot_space(self, volume_id,
     def restore_from_snapshot(self, volume_id, snapshot_id):
         """Restores a specific volume from a snapshot
 
-        :param integer volume_id: The id of the volume
+        :param integer volume_id: The ID of the volume
         :param integer snapshot_id: The id of the restore point
-        :return: Returns whether succesfully restored or not
+        :return: Returns whether successfully restored or not
         """
 
         return self.client.call('Network_Storage', 'restoreFromSnapshot',
@@ -437,7 +448,7 @@ def cancel_file_volume(self, volume_id,
 
         :param integer volume_id: The volume ID
         :param string reason: The reason for cancellation
-        :param boolean immediate_flag: Cancel immediately or
+        :param boolean immediate: Cancel immediately or
         on anniversary date
         """
         file_volume = self.get_file_volume_details(
@@ -454,7 +465,7 @@ def cancel_file_volume(self, volume_id,
     def failover_to_replicant(self, volume_id, replicant_id, immediate=False):
         """Failover to a volume replicant.
 
-        :param integer volume_id: The id of the volume
+        :param integer volume_id: The ID of the volume
         :param integer replicant_id: ID of replicant to failover to
         :param boolean immediate: Flag indicating if failover is immediate
         :return: Returns whether failover was successful or not
@@ -466,8 +477,8 @@ def failover_to_replicant(self, volume_id, replicant_id, immediate=False):
     def failback_from_replicant(self, volume_id, replicant_id):
         """Failback from a volume replicant.
 
-        :param integer volume_id: The id of the volume
-        :param integer: ID of replicant to failback from
+        :param integer volume_id: The ID of the volume
+        :param integer replicant_id: ID of replicant to failback from
         :return: Returns whether failback was successful or not
         """
 

SoftLayer/managers/storage_utils.py

@@ -60,7 +60,7 @@ def populate_host_templates(host_templates,
 
 
 def get_package(manager, category_code):
-    """Returns a product packaged based on type of storage.
+    """Returns a product package based on type of storage.
 
     :param manager: The storage manager which calls this function.
     :param category_code: Category code of product package.

tests/CLI/modules/block_tests.py

@@ -217,6 +217,30 @@ def test_volume_order_order_not_placed(self, order_mock):
                          'Order could not be placed! Please verify '
                          'your options and try again.\n')
 
+    @mock.patch('SoftLayer.BlockStorageManager.order_block_volume')
+    def test_volume_order_performance_manager_error(self, order_mock):
+        order_mock.side_effect = ValueError('failure!')
+
+        result = self.run_command(['block', 'volume-order',
+                                   '--storage-type=performance', '--size=20',
+                                   '--iops=100', '--os-type=linux',
+                                   '--location=dal05'])
+
+        self.assertEqual(2, result.exit_code)
+        self.assertEqual('Argument Error: failure!', result.exception.message)
+
+    @mock.patch('SoftLayer.BlockStorageManager.order_block_volume')
+    def test_volume_order_endurance_manager_error(self, order_mock):
+        order_mock.side_effect = ValueError('failure!')
+
+        result = self.run_command(['block', 'volume-order',
+                                   '--storage-type=endurance', '--size=20',
+                                   '--tier=0.25', '--os-type=linux',
+                                   '--location=dal05'])
+
+        self.assertEqual(2, result.exit_code)
+        self.assertEqual('Argument Error: failure!', result.exception.message)
+
     def test_enable_snapshots(self):
         result = self.run_command(['block', 'snapshot-enable', '12345678',
                                    '--schedule-type=HOURLY', '--minute=10',

tests/CLI/modules/file_tests.py

@@ -151,31 +151,29 @@ def test_volume_detail(self):
     def test_volume_order_performance_iops_not_given(self):
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=performance', '--size=20',
-                                   '--os-type=linux', '--location=dal05'])
+                                   '--location=dal05'])
 
         self.assertEqual(2, result.exit_code)
 
     def test_volume_order_performance_iops_out_of_range(self):
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=performance', '--size=20',
-                                   '--iops=80000', '--os-type=linux',
-                                   '--location=dal05'])
+                                   '--iops=80000', '--location=dal05'])
 
         self.assertEqual(2, result.exit_code)
 
     def test_volume_order_performance_iops_not_multiple_of_100(self):
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=performance', '--size=20',
-                                   '--iops=122', '--os-type=linux',
-                                   '--location=dal05'])
+                                   '--iops=122', '--location=dal05'])
 
         self.assertEqual(2, result.exit_code)
 
     def test_volume_order_performance_snapshot_error(self):
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=performance', '--size=20',
-                                   '--iops=100', '--os-type=linux',
-                                   '--location=dal05', '--snapshot-size=10'])
+                                   '--iops=100', '--location=dal05',
+                                   '--snapshot-size=10'])
 
         self.assertEqual(2, result.exit_code)
 
@@ -194,8 +192,7 @@ def test_volume_order_performance(self, order_mock):
 
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=performance', '--size=20',
-                                   '--iops=100', '--os-type=linux',
-                                   '--location=dal05'])
+                                   '--iops=100', '--location=dal05'])
 
         self.assert_no_fail(result)
         self.assertEqual(result.output,
@@ -206,7 +203,7 @@ def test_volume_order_performance(self, order_mock):
     def test_volume_order_endurance_tier_not_given(self):
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=endurance', '--size=20',
-                                   '--os-type=linux', '--location=dal05'])
+                                   '--location=dal05'])
 
         self.assertEqual(2, result.exit_code)
 
@@ -226,8 +223,8 @@ def test_volume_order_endurance(self, order_mock):
 
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=endurance', '--size=20',
-                                   '--tier=0.25', '--os-type=linux',
-                                   '--location=dal05', '--snapshot-size=10'])
+                                   '--tier=0.25', '--location=dal05',
+                                   '--snapshot-size=10'])
 
         self.assert_no_fail(result)
         self.assertEqual(result.output,
@@ -242,14 +239,35 @@ def test_volume_order_order_not_placed(self, order_mock):
 
         result = self.run_command(['file', 'volume-order',
                                    '--storage-type=endurance', '--size=20',
-                                   '--tier=0.25', '--os-type=linux',
-                                   '--location=dal05'])
+                                   '--tier=0.25', '--location=dal05'])
 
         self.assert_no_fail(result)
         self.assertEqual(result.output,
                          'Order could not be placed! Please verify '
                          'your options and try again.\n')
 
+    @mock.patch('SoftLayer.FileStorageManager.order_file_volume')
+    def test_volume_order_performance_manager_error(self, order_mock):
+        order_mock.side_effect = ValueError('failure!')
+
+        result = self.run_command(['file', 'volume-order',
+                                   '--storage-type=performance', '--size=20',
+                                   '--iops=100', '--location=dal05'])
+
+        self.assertEqual(2, result.exit_code)
+        self.assertEqual('Argument Error: failure!', result.exception.message)
+
+    @mock.patch('SoftLayer.FileStorageManager.order_file_volume')
+    def test_volume_order_endurance_manager_error(self, order_mock):
+        order_mock.side_effect = ValueError('failure!')
+
+        result = self.run_command(['file', 'volume-order',
+                                   '--storage-type=endurance', '--size=20',
+                                   '--tier=0.25', '--location=dal05'])
+
+        self.assertEqual(2, result.exit_code)
+        self.assertEqual('Argument Error: failure!', result.exception.message)
+
     def test_enable_snapshots(self):
         result = self.run_command(['file', 'snapshot-enable', '12345678',
                                    '--schedule-type=HOURLY', '--minute=10',