Repository: softlayer/softlayer-python
Pull request #848 (2017-07-26 21:40:54)

Add support for dedicated host instances to virtual server upgrades
Changing to use SL_Virtual_Guest.getUpgradeItemPrices which returns only the prices available for upgrade. This allows for upgrading without duplicating business logic in the client.


Patch:
diff --git a/.gitignore b/.gitignore
index 438f1d70..ebf9932d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,4 +13,4 @@ build/*
 dist/*
 *.egg-info
 .cache
-
+.idea
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 11f5abea..f6b33453 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,11 +1,5 @@
 # Change Log
 
-## [5.2.9] - TBD
- - Changes: https://github.com/softlayer/softlayer-python/compare/v5.2.8...master
- 
-#### Added to CLI
-* block volume-set-lun-id
-
 ## [5.2.8] - 2017-07-19
  - Changes: https://github.com/softlayer/softlayer-python/compare/v5.2.7...master
  
diff --git a/SoftLayer/CLI/block/lun.py b/SoftLayer/CLI/block/lun.py
deleted file mode 100644
index ee33a23b..00000000
--- a/SoftLayer/CLI/block/lun.py
+++ /dev/null
@@ -1,36 +0,0 @@
-"""Set the LUN ID on an iSCSI volume."""
-# :license: MIT, see LICENSE for more details.
-
-import click
-
-import SoftLayer
-from SoftLayer.CLI import environment
-
-
-@click.command()
-@click.argument('volume-id')
-@click.argument('lun-id')
-@environment.pass_env
-def cli(env, volume_id, lun_id):
-    """Set the LUN ID on an existing block storage volume.
-
-    The LUN ID only takes effect during the Host Authorization process. It is
-    recommended (but not necessary) to de-authorize all hosts before using this
-    method. See `block access-revoke`.
-
-    VOLUME_ID - the volume ID on which to set the LUN ID.
-
-    LUN_ID - recommended range is an integer between 0 and 255. Advanced users
-    can use an integer between 0 and 4095.
-    """
-
-    block_storage_manager = SoftLayer.BlockStorageManager(env.client)
-
-    res = block_storage_manager.create_or_update_lun_id(volume_id, lun_id)
-
-    if 'value' in res and lun_id == res['value']:
-        click.echo(
-            'Block volume with id %s is reporting LUN ID %s' % (res['volumeId'], res['value']))
-    else:
-        click.echo(
-            'Failed to confirm the new LUN ID on volume %s' % (volume_id))
diff --git a/SoftLayer/CLI/routes.py b/SoftLayer/CLI/routes.py
index fe2bb12f..541c9d98 100644
--- a/SoftLayer/CLI/routes.py
+++ b/SoftLayer/CLI/routes.py
@@ -80,7 +80,6 @@
     ('block:volume-duplicate', 'SoftLayer.CLI.block.duplicate:cli'),
     ('block:volume-list', 'SoftLayer.CLI.block.list:cli'),
     ('block:volume-order', 'SoftLayer.CLI.block.order:cli'),
-    ('block:volume-set-lun-id', 'SoftLayer.CLI.block.lun:cli'),
 
     ('file', 'SoftLayer.CLI.file'),
     ('file:access-authorize', 'SoftLayer.CLI.file.access.authorize:cli'),
diff --git a/SoftLayer/fixtures/SoftLayer_Virtual_Guest.py b/SoftLayer/fixtures/SoftLayer_Virtual_Guest.py
index 966e8a40..6f284950 100644
--- a/SoftLayer/fixtures/SoftLayer_Virtual_Guest.py
+++ b/SoftLayer/fixtures/SoftLayer_Virtual_Guest.py
@@ -252,3 +252,85 @@
 setTags = True
 createArchiveTransaction = {}
 executeRescueLayer = True
+
+getUpgradeItemPrices = [
+    {
+        'id': 1007,
+        'categories': [{'id': 80,
+                        'name': 'Computing Instance',
+                        'categoryCode': 'guest_core'}],
+        'item': {
+            'capacity': '4',
+            'units': 'PRIVATE_CORE',
+            'description': 'Computing Instance (Dedicated)',
+        }
+    },
+    {
+        'id': 1144,
+        'locationGroupId': None,
+        'categories': [{'id': 80,
+                        'name': 'Computing Instance',
+                        'categoryCode': 'guest_core'}],
+        'item': {
+            'capacity': '4',
+            'units': 'CORE',
+            'description': 'Computing Instance',
+        }
+    },
+    {
+        'id': 332211,
+        'locationGroupId': 1,
+        'categories': [{'id': 80,
+                        'name': 'Computing Instance',
+                        'categoryCode': 'guest_core'}],
+        'item': {
+            'capacity': '4',
+            'units': 'CORE',
+            'description': 'Computing Instance',
+        }
+    },
+    {
+        'id': 1122,
+        'categories': [{'id': 26,
+                        'name': 'Uplink Port Speeds',
+                        'categoryCode': 'port_speed'}],
+        'item': {
+            'capacity': '1000',
+            'description': 'Public & Private Networks',
+        }
+    },
+    {
+        'id': 1144,
+        'categories': [{'id': 26,
+                        'name': 'Uplink Port Speeds',
+                        'categoryCode': 'port_speed'}],
+        'item': {
+            'capacity': '1000',
+            'description': 'Private Networks',
+        }
+    },
+    {
+        'id': 1133,
+        'categories': [{'id': 3,
+                        'name': 'RAM',
+                        'categoryCode': 'ram'}],
+        'item': {
+            'capacity': '2',
+            'description': 'RAM',
+        }
+    },
+]
+
+DEDICATED_GET_UPGRADE_ITEM_PRICES = [
+    {
+        'id': 115566,
+        'categories': [{'id': 80,
+                        'name': 'Computing Instance',
+                        'categoryCode': 'guest_core'}],
+        'item': {
+            'capacity': '4',
+            'units': 'DEDICATED_CORE',
+            'description': 'Computing Instance (Dedicated Host)',
+        }
+    },
+]
diff --git a/SoftLayer/managers/block.py b/SoftLayer/managers/block.py
index 37e73d6f..817232a4 100644
--- a/SoftLayer/managers/block.py
+++ b/SoftLayer/managers/block.py
@@ -547,7 +547,7 @@ 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 replicant_id: ID of replicant to failback from
+        :param integer: ID of replicant to failback from
         :return: Returns whether failback was successful or not
         """
 
@@ -563,13 +563,3 @@ def set_credential_password(self, access_id, password):
 
         return self.client.call('Network_Storage_Allowed_Host', 'setCredentialPassword',
                                 password, id=access_id)
-
-    def create_or_update_lun_id(self, volume_id, lun_id):
-        """Set the LUN ID on a volume.
-
-        :param integer volume_id: The id of the volume
-        :param integer lun_id: LUN ID to set on the volume
-        :return: a SoftLayer_Network_Storage_Property object
-        """
-        return self.client.call('Network_Storage', 'createOrUpdateLunId',
-                                lun_id, id=volume_id)
diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py
index 2e27b99f..6bcb4b20 100644
--- a/SoftLayer/managers/vs.py
+++ b/SoftLayer/managers/vs.py
@@ -9,6 +9,7 @@
 import itertools
 import socket
 import time
+import warnings
 
 from SoftLayer import exceptions
 from SoftLayer.managers import ordering
@@ -773,13 +774,13 @@ def upgrade(self, instance_id, cpus=None, memory=None,
         :param int instance_id: Instance id of the VS to be upgraded
         :param int cpus: The number of virtual CPUs to upgrade to
                             of a VS instance.
-        :param bool public: CPU will be in Private/Public Node.
         :param int memory: RAM of the VS to be upgraded to.
         :param int nic_speed: The port speed to set
+        :param bool public: CPU will be in Private/Public Node.
 
         :returns: bool
         """
-        package_items = self._get_package_items()
+        upgrade_prices = self._get_upgrade_prices(instance_id)
         prices = []
 
         for option, value in {'cpus': cpus,
@@ -787,10 +788,10 @@ def upgrade(self, instance_id, cpus=None, memory=None,
                               'nic_speed': nic_speed}.items():
             if not value:
                 continue
-            price_id = self._get_price_id_for_upgrade(package_items,
-                                                      option,
-                                                      value,
-                                                      public)
+            price_id = self._get_price_id_for_upgrade_option(upgrade_prices,
+                                                             option,
+                                                             value,
+                                                             public)
             if not price_id:
                 # Every option provided is expected to have a price
                 raise exceptions.SoftLayerError(
@@ -815,7 +816,12 @@ def upgrade(self, instance_id, cpus=None, memory=None,
         return False
 
     def _get_package_items(self):
-        """Following Method gets all the item ids related to VS."""
+        """Following Method gets all the item ids related to VS.
+
+        Deprecated in favor of _get_upgrade_prices()
+        """
+        warnings.warn("use _get_upgrade_prices() instead",
+                      DeprecationWarning)
         mask = [
             'description',
             'capacity',
@@ -833,15 +839,76 @@ def _get_package_items(self):
         package_service = self.client['Product_Package']
         return package_service.getItems(id=package['id'], mask=mask)
 
+    def _get_upgrade_prices(self, instance_id, include_downgrade_options=True):
+        """Following Method gets all the price ids related to upgrading a VS.
+
+        :param int instance_id: Instance id of the VS to be upgraded
+
+        :returns: list
+        """
+        mask = [
+            'id',
+            'locationGroupId',
+            'categories[name,id,categoryCode]',
+            'item[description,capacity,units]'
+        ]
+        mask = "mask[%s]" % ','.join(mask)
+        return self.guest.getUpgradeItemPrices(include_downgrade_options, id=instance_id, mask=mask)
+
+    def _get_price_id_for_upgrade_option(self, upgrade_prices, option, value,
+                                         public=True):
+        """Find the price id for the option and value to upgrade. This
+
+        :param list upgrade_prices: Contains all the prices related to a VS upgrade
+        :param string option: Describes type of parameter to be upgraded
+        :param int value: The value of the parameter to be upgraded
+        :param bool public: CPU will be in Private/Public Node.
+        """
+        option_category = {
+            'memory': 'ram',
+            'cpus': 'guest_core',
+            'nic_speed': 'port_speed'
+        }
+        category_code = option_category.get(option)
+        for price in upgrade_prices:
+            if price.get('categories') is None or price.get('item') is None:
+                continue
+
+            product = price.get('item')
+            is_private = (product.get('units') == 'PRIVATE_CORE'
+                          or product.get('units') == 'DEDICATED_CORE')
+
+            for category in price.get('categories'):
+                if not (category.get('categoryCode') == category_code
+                        and str(product.get('capacity')) == str(value)):
+                    continue
+
+                if option == 'cpus':
+                    # Public upgrade and public guest_core price
+                    if public and not is_private:
+                        return price.get('id')
+                    # Private upgrade and private guest_core price
+                    elif not public and is_private:
+                        return price.get('id')
+                elif option == 'nic_speed':
+                    if 'Public' in product.get('description'):
+                        return price.get('id')
+                else:
+                    return price.get('id')
+
     def _get_price_id_for_upgrade(self, package_items, option, value,
                                   public=True):
         """Find the price id for the option and value to upgrade.
 
+        Deprecated in favor of _get_price_id_for_upgrade_option()
+
         :param list package_items: Contains all the items related to an VS
         :param string option: Describes type of parameter to be upgraded
         :param int value: The value of the parameter to be upgraded
         :param bool public: CPU will be in Private/Public Node.
         """
+        warnings.warn("use _get_price_id_for_upgrade_option() instead",
+                      DeprecationWarning)
         option_category = {
             'memory': 'ram',
             'cpus': 'guest_core',


Test patch:
diff --git a/tests/CLI/modules/block_tests.py b/tests/CLI/modules/block_tests.py
index c754e25d..944acb84 100644
--- a/tests/CLI/modules/block_tests.py
+++ b/tests/CLI/modules/block_tests.py
@@ -4,7 +4,6 @@
 
     :license: MIT, see LICENSE for more details.
 """
-from SoftLayer import exceptions
 from SoftLayer import testing
 
 import json
@@ -29,32 +28,6 @@ def test_volume_cancel(self):
         self.assert_called_with('SoftLayer_Billing_Item', 'cancelItem',
                                 args=(False, True, None))
 
-    def test_volume_set_lun_id_in_range(self):
-        lun_mock = self.set_mock('SoftLayer_Network_Storage', 'createOrUpdateLunId')
-        lun_mock.return_value = dict(volumeId=1234, value='42')
-        result = self.run_command('block volume-set-lun-id 1234 42'.split())
-        self.assert_no_fail(result)
-        self.assertEqual('Block volume with id 1234 is reporting LUN ID 42\n',
-                         result.output)
-
-    def test_volume_set_lun_id_in_range_missing_value(self):
-        lun_mock = self.set_mock('SoftLayer_Network_Storage', 'createOrUpdateLunId')
-        lun_mock.return_value = dict(volumeId=1234)
-        result = self.run_command('block volume-set-lun-id 1234 42'.split())
-        self.assert_no_fail(result)
-        self.assertEqual('Failed to confirm the new LUN ID on volume 1234\n',
-                         result.output)
-
-    def test_volume_set_lun_id_not_in_range(self):
-        value = '-1'
-        lun_mock = self.set_mock('SoftLayer_Network_Storage', 'createOrUpdateLunId')
-        lun_mock.side_effect = exceptions.SoftLayerAPIError(
-            'SoftLayer_Exception_Network_Storage_Iscsi_InvalidLunId',
-            'The LUN ID specified is out of the valid range: %s [min: 0 max: 4095]' % (value))
-        result = self.run_command('block volume-set-lun-id 1234 42'.split())
-        self.assertIsNotNone(result.exception)
-        self.assertIn('The LUN ID specified is out of the valid range', result.exception.faultString)
-
     def test_volume_detail(self):
         result = self.run_command(['block', 'volume-detail', '1234'])
 
diff --git a/tests/managers/vs_tests.py b/tests/managers/vs_tests.py
index fb1a2c2a..8773f6ae 100644
--- a/tests/managers/vs_tests.py
+++ b/tests/managers/vs_tests.py
@@ -7,7 +7,6 @@
 import mock
 
 import SoftLayer
-from SoftLayer import exceptions
 from SoftLayer import fixtures
 from SoftLayer import testing
 
@@ -636,13 +635,6 @@ def test_capture_additional_disks(self):
                                 identifier=1)
 
     def test_upgrade(self):
-        mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
-        mock.return_value = [
-            {'id': 46, 'name': 'Virtual Servers',
-             'description': 'Virtual Server Instances',
-             'type': {'keyName': 'VIRTUAL_SERVER_INSTANCE'}, 'isActive': 1},
-        ]
-
         # test single upgrade
         result = self.vs.upgrade(1, cpus=4, public=False)
 
@@ -678,10 +670,19 @@ def test_upgrade_full(self):
         self.assertIn({'id': 1122}, order_container['prices'])
         self.assertEqual(order_container['virtualGuests'], [{'id': 1}])
 
-    def test_upgrade_skips_location_based_prices(self):
-        # Test that no prices that have locationGroupId set are used
-        self.assertRaises(exceptions.SoftLayerError,
-                          self.vs.upgrade, 1, cpus=55, memory=2, public=True)
+    def test_upgrade_dedicated_host_instance(self):
+        mock = self.set_mock('SoftLayer_Virtual_Guest', 'getUpgradeItemPrices')
+        mock.return_value = fixtures.SoftLayer_Virtual_Guest.DEDICATED_GET_UPGRADE_ITEM_PRICES
+
+        # test single upgrade
+        result = self.vs.upgrade(1, cpus=4, public=False)
+
+        self.assertEqual(result, True)
+        self.assert_called_with('SoftLayer_Product_Order', 'placeOrder')
+        call = self.calls('SoftLayer_Product_Order', 'placeOrder')[0]
+        order_container = call.args[0]
+        self.assertEqual(order_container['prices'], [{'id': 115566}])
+        self.assertEqual(order_container['virtualGuests'], [{'id': 1}])
 
     def test_get_item_id_for_upgrade(self):
         item_id = 0
@@ -693,6 +694,48 @@ def test_get_item_id_for_upgrade(self):
                 break
         self.assertEqual(1133, item_id)
 
+    def test_get_package_items(self):
+        self.vs._get_package_items()
+        self.assert_called_with('SoftLayer_Product_Package', 'getItems')
+
+    def test_get_package_items_errors(self):
+        mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
+        mock.return_value = []
+
+        self.assertRaises(ValueError, self.vs._get_package_items)
+
+    def test_get_price_id_for_upgrade(self):
+        package_items = self.vs._get_package_items()
+
+        price_id = self.vs._get_price_id_for_upgrade(package_items=package_items,
+                                                     option='cpus',
+                                                     value='4')
+        self.assertEqual(1144, price_id)
+
+    def test_get_price_id_for_upgrade_skips_location_price(self):
+        package_items = self.vs._get_package_items()
+
+        price_id = self.vs._get_price_id_for_upgrade(package_items=package_items,
+                                                     option='cpus',
+                                                     value='55')
+        self.assertEqual(None, price_id)
+
+    def test_get_price_id_for_upgrade_finds_nic_price(self):
+        package_items = self.vs._get_package_items()
+
+        price_id = self.vs._get_price_id_for_upgrade(package_items=package_items,
+                                                     option='memory',
+                                                     value='2')
+        self.assertEqual(1133, price_id)
+
+    def test_get_price_id_for_upgrade_finds_memory_price(self):
+        package_items = self.vs._get_package_items()
+
+        price_id = self.vs._get_price_id_for_upgrade(package_items=package_items,
+                                                     option='nic_speed',
+                                                     value='1000')
+        self.assertEqual(1122, price_id)
+
 
 class VSWaitReadyGoTests(testing.TestCase):