Fix the way storage space is looked up for block and file storage ordering. Remove the OS type requirement from file storage ordering as it is not used.

camporter · camporter · commit f0840e302d48 · 2017-01-09T15:58:00.000-06:00
diff --git a/SoftLayer/CLI/file/order.py b/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:
diff --git a/SoftLayer/managers/block.py b/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':
diff --git a/SoftLayer/managers/file.py b/SoftLayer/managers/file.py
@@ -228,15 +228,15 @@ 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,
@@ -259,7 +259,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 +288,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 +312,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 +348,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 +398,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 +430,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 +445,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 +462,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 +474,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
         """
 
diff --git a/SoftLayer/managers/storage_utils.py b/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.
diff --git a/tests/managers/file_tests.py b/tests/managers/file_tests.py
@@ -187,7 +187,7 @@ def test_order_file_volume_no_package(self):
             "performance_storage_nfs",
             "dal05",
             40,
-            "LINUX",
+            None,
             iops=100,
         )
 
@@ -201,7 +201,7 @@ def test_order_file_volume_too_many_packages(self):
             "performance_storage_nfs",
             "dal05",
             40,
-            "LINUX",
+            None,
             iops=100,
         )
 
@@ -246,7 +246,7 @@ def test_order_file_volume_performance(self):
             "performance_storage_nfs",
             "dal05",
             100,
-            "LINUX",
+            None,
             iops=100,
             )
 
@@ -330,7 +330,7 @@ def test_order_file_volume_endurance(self):
             "storage_service_enterprise",
             "dal05",
             100,
-            "LINUX",
+            None,
             tier_level=0.25,
             )
 
@@ -417,7 +417,7 @@ def test_order_file_volume_endurance_with_snapshot(self):
             "storage_service_enterprise",
             "dal05",
             100,
-            "LINUX",
+            None,
             tier_level=0.25,
             snapshot_size=10,
             )

Original file line number	Diff line number	Diff line change
`@@ -271,7 +271,7 @@ def order_block_volume(self, storage_type, location, size, os_type,`
`271`	`271`	`package,`
`272`	`272`	`'performance_storage_iscsi'`
`273`	`273`	`),`
`274`		`- storage_utils.find_performance_space_price(package, iops),`
	`274`	`+ storage_utils.find_performance_space_price(package, size),`
`275`	`275`	`storage_utils.find_performance_iops_price(package, size, iops),`
`276`	`276`	`]`
`277`	`277`	`elif storage_type == 'storage_service_enterprise':`