Merge pull request #1569 from allmightyspiff/issues1568

fixed up snapshot-notification cli commands

Author: allmightyspiff

SoftLayer/CLI/block/count.py

@@ -30,7 +30,7 @@ def cli(env, sortby, datacenter):
         service_resource = volume['serviceResource']
         if 'datacenter' in service_resource:
             datacenter_name = service_resource['datacenter']['name']
-            if datacenter_name not in datacenters.keys():
+            if datacenter_name not in datacenters.keys():  # pylint: disable=consider-iterating-dictionary
                 datacenters[datacenter_name] = 1
             else:
                 datacenters[datacenter_name] += 1

SoftLayer/CLI/block/snapshot/get_notify_status.py

@@ -14,15 +14,7 @@ def cli(env, volume_id):
     block_manager = SoftLayer.BlockStorageManager(env.client)
     enabled = block_manager.get_volume_snapshot_notification_status(volume_id)
 
-    if enabled == '':
-        click.echo("""
-        Enabled:Snapshots space usage warning flag is null. Set to default value enable. For volume %s
-        """ % (volume_id))
-    elif enabled == 'True':
-        click.echo(
-            'Enabled:Snapshots space usage threshold warning flag setting is enabled for volume %s'
-            % (volume_id))
+    if enabled == 0:
+        click.echo("Disabled: Snapshots space usage threshold is disabled for volume {}".format(volume_id))
     else:
-        click.echo(
-            'Disabled:Snapshots space usage threshold warning flag setting is disabled for volume %s'
-            % (volume_id))
+        click.echo("Enabled: Snapshots space usage threshold is enabled for volume {}".format(volume_id))

SoftLayer/CLI/file/count.py

@@ -29,7 +29,7 @@ def cli(env, sortby, datacenter):
         service_resource = volume['serviceResource']
         if 'datacenter' in service_resource:
             datacenter_name = service_resource['datacenter']['name']
-            if datacenter_name not in datacenters.keys():
+            if datacenter_name not in datacenters.keys():  # pylint: disable=consider-iterating-dictionary
                 datacenters[datacenter_name] = 1
             else:
                 datacenters[datacenter_name] += 1

SoftLayer/CLI/file/snapshot/get_notify_status.py

@@ -15,15 +15,7 @@ def cli(env, volume_id):
     file_manager = SoftLayer.FileStorageManager(env.client)
     enabled = file_manager.get_volume_snapshot_notification_status(volume_id)
 
-    if enabled == '':
-        click.echo(
-            'Enabled:Snapshots space usage threshold warning flag is null. Set to default value enable. For volume %s'
-            % (volume_id))
-    elif enabled == 'True':
-        click.echo(
-            'Enabled:Snapshots space usage threshold warning flag setting is enabled for volume %s'
-            % (volume_id))
+    if enabled == 0:
+        click.echo("Disabled: Snapshots space usage threshold is disabled for volume {}".format(volume_id))
     else:
-        click.echo(
-            'Disabled:Snapshots space usage threshold warning flag setting is disabled for volume %s'
-            % (volume_id))
+        click.echo("Enabled: Snapshots space usage threshold is enabled for volume {}".format(volume_id))

SoftLayer/managers/storage.py

@@ -19,11 +19,16 @@ class StorageManager(utils.IdentifierMixin, object):
 
     :param SoftLayer.API.BaseClient client: the client instance
     """
+
     def __init__(self, client):
         self.configuration = {}
         self.client = client
         self.resolvers = [self._get_ids_from_username]
 
+    def _get_ids_from_username(self, username):  # pylint: disable=unused-argument,no-self-use
+        """Should only be actually called from the block/file manager"""
+        return []
+
     def get_volume_count_limits(self):
         """Returns a list of block volume count limit.
 
@@ -122,20 +127,21 @@ def set_volume_snapshot_notification(self, volume_id, enable):
         :return:  Enables/Disables snapshot space usage threshold warning for a given volume.
         """
 
-        return self.client.call('Network_Storage',
-                                'setSnapshotNotification',
-                                enable,
-                                id=volume_id)
+        return self.client.call('Network_Storage', 'setSnapshotNotification', enable, id=volume_id)
 
     def get_volume_snapshot_notification_status(self, volume_id):
         """returns Enabled/Disabled status of snapshot space usage threshold warning for a given volume.
 
         :param volume_id: ID of volume.
         :return:  Enables/Disables snapshot space usage threshold warning for a given volume.
         """
-        return self.client.call('Network_Storage',
-                                'getSnapshotNotificationStatus',
-                                id=volume_id)
+        status = self.client.call('Network_Storage', 'getSnapshotNotificationStatus', id=volume_id)
+        # A None status is enabled as well.
+        if status is None:
+            status = 1
+        # We need to force int on the return because otherwise the API will return the string '0'
+        # instead of either a boolean or real int...
+        return int(status)
 
     def authorize_host_to_volume(self,
                                  volume_id,

tests/CLI/modules/block_tests.py

@@ -812,3 +812,13 @@ def test_volume_not_set_note(self, set_note):
 
         self.assert_no_fail(result)
         self.assertIn("Note could not be set!", result.output)
+
+    @mock.patch('SoftLayer.BlockStorageManager.get_volume_snapshot_notification_status')
+    def test_snapshot_get_notification_status(self, status):
+        status.side_effect = [None, 1, 0]
+        expected = ['Enabled', 'Enabled', 'Disabled']
+
+        for expect in expected:
+            result = self.run_command(['block', 'snapshot-get-notification-status', '999'])
+            self.assert_no_fail(result)
+            self.assertIn(expect, result.output)

tests/CLI/modules/file_tests.py

@@ -791,3 +791,13 @@ def test_volume_not_set_note(self, set_note):
 
         self.assert_no_fail(result)
         self.assertIn("Note could not be set!", result.output)
+
+    @mock.patch('SoftLayer.FileStorageManager.get_volume_snapshot_notification_status')
+    def test_snapshot_get_notification_status(self, status):
+        status.side_effect = [None, 1, 0]
+        expected = ['Enabled', 'Enabled', 'Disabled']
+
+        for expect in expected:
+            result = self.run_command(['file', 'snapshot-get-notification-status', '999'])
+            self.assert_no_fail(result)
+            self.assertIn(expect, result.output)

tests/managers/storage_generic_tests.py

@@ -0,0 +1,34 @@
+"""
+    SoftLayer.tests.managers.storage_generic_tests
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    :license: MIT, see LICENSE for more details.
+"""
+
+import SoftLayer
+from SoftLayer import testing
+
+
+class StorageGenericTests(testing.TestCase):
+    def set_up(self):
+        self.storage = SoftLayer.managers.storage.StorageManager(self.client)
+
+    def test_get_volume_snapshot_notification_status(self):
+        mock = self.set_mock('SoftLayer_Network_Storage', 'getSnapshotNotificationStatus')
+        # These are the values we expect from the API as of 2021-12-01, FBLOCK4193
+        mock.side_effect = [None, '1', '0']
+        expected = [1, 1, 0]
+
+        for expect in expected:
+            result = self.storage.get_volume_snapshot_notification_status(12345)
+            self.assert_called_with('SoftLayer_Network_Storage', 'getSnapshotNotificationStatus', identifier=12345)
+            self.assertEqual(expect, result)
+
+    def test_set_volume_snapshot_notification(self):
+        mock = self.set_mock('SoftLayer_Network_Storage', 'setSnapshotNotification')
+        mock.return_value = None
+
+        result = self.storage.set_volume_snapshot_notification(12345, False)
+        self.assert_called_with('SoftLayer_Network_Storage', 'setSnapshotNotification',
+                                identifier=12345, args=(False,))
+        self.assertEqual(None, result)