Repository: softlayer/softlayer-python

Issue:
Slcli hardware update-firmware should do networkcards too now
https://sldn.softlayer.com/reference/services/SoftLayer_Hardware_Server/createFirmwareUpdateTransaction/ has an option for networkCards now, need to support that

Discussion:

Patch:
diff --git a/SoftLayer/CLI/formatting.py b/SoftLayer/CLI/formatting.py
index 9c32318d..fb91ded6 100644
--- a/SoftLayer/CLI/formatting.py
+++ b/SoftLayer/CLI/formatting.py
@@ -259,8 +259,7 @@ def no_going_back(confirmation):
     if not confirmation:
         confirmation = 'yes'
 
-    prompt = ('This action cannot be undone! Type "%s" or press Enter '
-              'to abort' % confirmation)
+    prompt = f"This action cannot be undone! Type '{confirmation}' or press Enter to abort"
 
     ans = click.prompt(prompt, default='', show_default=False)
     if ans.lower() == str(confirmation):
diff --git a/SoftLayer/CLI/hardware/update_firmware.py b/SoftLayer/CLI/hardware/update_firmware.py
index 6b634376..96e82b9b 100644
--- a/SoftLayer/CLI/hardware/update_firmware.py
+++ b/SoftLayer/CLI/hardware/update_firmware.py
@@ -12,15 +12,23 @@
 
 @click.command(cls=SoftLayer.CLI.command.SLCommand, )
 @click.argument('identifier')
+@click.option('-i', '--ipmi', is_flag=True, help="Update IPMI firmware")
+@click.option('-r', '--raid', is_flag=True, help="Update RAID firmware")
+@click.option('-b', '--bios', is_flag=True, help="Update BIOS firmware")
+@click.option('-d', '--harddrive', is_flag=True, help="Update Hard Drives firmware")
+@click.option('-n', '--network', is_flag=True, help="Update Network Card firmware")
 @environment.pass_env
-def cli(env, identifier):
-    """Update server firmware."""
+def cli(env, identifier, ipmi, raid, bios, harddrive, network):
+    """Update server firmware. By default will update all available server components."""
 
     mgr = SoftLayer.HardwareManager(env.client)
     hw_id = helpers.resolve_id(mgr.resolve_ids, identifier, 'hardware')
-    if not (env.skip_confirmations or
-            formatting.confirm('This will power off the server with id %s and '
-                               'update device firmware. Continue?' % hw_id)):
+    confirm_message = f"This will power off the server with id {hw_id} and update device firmware. Continue?"
+    if not (env.skip_confirmations or formatting.confirm(confirm_message)):
         raise exceptions.CLIAbort('Aborted.')
 
-    mgr.update_firmware(hw_id)
+    # If no options were specified, set them all to enabled.
+    if not any([ipmi, raid, bios, harddrive, network]):
+        ipmi = raid = bios = harddrive = network = 1
+    mgr.update_firmware(hw_id, ipmi, raid, bios, harddrive, network)
+    env.fout(f"[green]Firmware update for {identifier} started")
diff --git a/SoftLayer/managers/hardware.py b/SoftLayer/managers/hardware.py
index a59e7244..69add1cf 100644
--- a/SoftLayer/managers/hardware.py
+++ b/SoftLayer/managers/hardware.py
@@ -723,22 +723,23 @@ def edit(self, hardware_id, userdata=None, hostname=None, domain=None,
 
         return self.hardware.editObject(obj, id=hardware_id)
 
-    def update_firmware(self,
-                        hardware_id,
-                        ipmi=True,
-                        raid_controller=True,
-                        bios=True,
-                        hard_drive=True):
+    def update_firmware(self, hardware_id: int,
+                        ipmi: bool = True,
+                        raid_controller: bool = True,
+                        bios: bool = True,
+                        hard_drive: bool = True,
+                        network: bool = True):
         """Update hardware firmware.
 
         This will cause the server to be unavailable for ~20 minutes.
+        https://sldn.softlayer.com/reference/services/SoftLayer_Hardware_Server/createFirmwareUpdateTransaction/
 
-        :param int hardware_id: The ID of the hardware to have its firmware
-                                updated.
+        :param int hardware_id: The ID of the hardware to have its firmware updated.
         :param bool ipmi: Update the ipmi firmware.
         :param bool raid_controller: Update the raid controller firmware.
         :param bool bios: Update the bios firmware.
         :param bool hard_drive: Update the hard drive firmware.
+        :param bool network: Update the network card firmware
 
         Example::
 
@@ -746,21 +747,22 @@ def update_firmware(self,
             result = mgr.update_firmware(hardware_id=1234)
         """
 
-        return self.hardware.createFirmwareUpdateTransaction(
-            bool(ipmi), bool(raid_controller), bool(bios), bool(hard_drive), id=hardware_id)
+        return self.client.call(
+            'SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
+            bool(ipmi), bool(raid_controller), bool(bios), bool(hard_drive), bool(network), id=hardware_id
+        )
 
-    def reflash_firmware(self,
-                         hardware_id,
-                         ipmi=True,
-                         raid_controller=True,
-                         bios=True):
+    def reflash_firmware(self, hardware_id: int,
+                         ipmi: bool = True,
+                         raid_controller: bool = True,
+                         bios: bool = True,):
         """Reflash hardware firmware.
 
         This will cause the server to be unavailable for ~60 minutes.
         The firmware will not be upgraded but rather reflashed to the version installed.
+        https://sldn.softlayer.com/reference/services/SoftLayer_Hardware_Server/createFirmwareReflashTransaction/
 
-        :param int hardware_id: The ID of the hardware to have its firmware
-                                reflashed.
+        :param int hardware_id: The ID of the hardware to have its firmware reflashed.
         :param bool ipmi: Reflash the ipmi firmware.
         :param bool raid_controller: Reflash the raid controller firmware.
         :param bool bios: Reflash the bios firmware.


Test patch:
diff --git a/tests/CLI/modules/hardware/hardware_basic_tests.py b/tests/CLI/modules/hardware/hardware_basic_tests.py
index 61135fa4..d7c2ca9b 100644
--- a/tests/CLI/modules/hardware/hardware_basic_tests.py
+++ b/tests/CLI/modules/hardware/hardware_basic_tests.py
@@ -498,26 +498,6 @@ def test_edit_server_userfile(self):
             self.assert_called_with('SoftLayer_Hardware_Server', 'setUserMetadata',
                                     args=(['some data'],), identifier=1000)
 
-    @mock.patch('SoftLayer.CLI.formatting.confirm')
-    def test_update_firmware(self, confirm_mock):
-        confirm_mock.return_value = True
-        result = self.run_command(['server', 'update-firmware', '1000'])
-
-        self.assert_no_fail(result)
-        self.assertEqual(result.output, "")
-        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
-                                args=((1, 1, 1, 1)), identifier=1000)
-
-    @mock.patch('SoftLayer.CLI.formatting.confirm')
-    def test_reflash_firmware(self, confirm_mock):
-        confirm_mock.return_value = True
-        result = self.run_command(['server', 'reflash-firmware', '1000'])
-
-        self.assert_no_fail(result)
-        self.assertEqual(result.output, 'Successfully device firmware reflashed\n')
-        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareReflashTransaction',
-                                args=((1, 1, 1)), identifier=1000)
-
     def test_edit(self):
         result = self.run_command(['server', 'edit',
                                    '--domain=example.com',
diff --git a/tests/CLI/modules/hardware/hardware_firmware_tests.py b/tests/CLI/modules/hardware/hardware_firmware_tests.py
new file mode 100644
index 00000000..cc71c488
--- /dev/null
+++ b/tests/CLI/modules/hardware/hardware_firmware_tests.py
@@ -0,0 +1,109 @@
+"""
+    SoftLayer.tests.CLI.modules.hardware.hardware_firmware_tests
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    This suite is for the firmware related tests.
+
+    :license: MIT, see LICENSE for more details.
+"""
+from SoftLayer.CLI import exceptions
+from SoftLayer import testing
+from unittest import mock as mock
+
+
+class HardwareFirmwareTests(testing.TestCase):
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_update_firmware(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['server', 'update-firmware', '1000'])
+        self.assert_no_fail(result)
+        self.assertIn("Firmware update for 1000 started", result.output)
+        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
+                                args=((1, 1, 1, 1, 1)), identifier=1000)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_update_firmware_just_ipmi(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['server', 'update-firmware', '1000', '-i'])
+
+        self.assert_no_fail(result)
+        self.assertIn("Firmware update for 1000 started", result.output)
+        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
+                                args=((1, 0, 0, 0, 0)), identifier=1000)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_update_firmware_just_raid(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['server', 'update-firmware', '1000', '-r'])
+
+        self.assert_no_fail(result)
+        self.assertIn("Firmware update for 1000 started", result.output)
+        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
+                                args=((0, 1, 0, 0, 0)), identifier=1000)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_update_firmware_just_bios(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['server', 'update-firmware', '1000', '-b'])
+
+        self.assert_no_fail(result)
+        self.assertIn("Firmware update for 1000 started", result.output)
+        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
+                                args=((0, 0, 1, 0, 0)), identifier=1000)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_update_firmware_just_disk(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['server', 'update-firmware', '1000', '-d'])
+
+        self.assert_no_fail(result)
+        self.assertIn("Firmware update for 1000 started", result.output)
+        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
+                                args=((0, 0, 0, 1, 0)), identifier=1000)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_update_firmware_just_nic(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['server', 'update-firmware', '1000', '-n'])
+
+        self.assert_no_fail(result)
+        self.assertIn("Firmware update for 1000 started", result.output)
+        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
+                                args=((0, 0, 0, 0, 1)), identifier=1000)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_update_firmware_just_all(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['server', 'update-firmware', '1000', '-i', '-r', '-b', '-d', '-n'])
+
+        self.assert_no_fail(result)
+        self.assertIn("Firmware update for 1000 started", result.output)
+        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
+                                args=((1, 1, 1, 1, 1)), identifier=1000)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_update_firmware_no_confirm(self, confirm_mock):
+        confirm_mock.return_value = False
+
+        result = self.run_command(['server', 'update-firmware', '1000'])
+        self.assertEqual(result.exit_code, 2)
+        self.assertIsInstance(result.exception, exceptions.CLIAbort)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_reflash_firmware(self, confirm_mock):
+        confirm_mock.return_value = True
+        result = self.run_command(['server', 'reflash-firmware', '1000'])
+
+        self.assert_no_fail(result)
+        self.assertEqual(result.output, 'Successfully device firmware reflashed\n')
+        self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareReflashTransaction',
+                                args=((1, 1, 1)), identifier=1000)
+
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
+    def test_reflash_firmware_no_confirm(self, confirm_mock):
+        confirm_mock.return_value = False
+
+        result = self.run_command(['server', 'reflash-firmware', '1000'])
+        self.assertEqual(result.exit_code, 2)
+        self.assertIsInstance(result.exception, exceptions.CLIAbort)
diff --git a/tests/managers/hardware_tests.py b/tests/managers/hardware_tests.py
index c6175d07..c067dd08 100644
--- a/tests/managers/hardware_tests.py
+++ b/tests/managers/hardware_tests.py
@@ -543,14 +543,14 @@ def test_update_firmware(self):
 
         self.assertEqual(result, True)
         self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
-                                identifier=100, args=(1, 1, 1, 1))
+                                identifier=100, args=(1, 1, 1, 1, 1))
 
     def test_update_firmware_selective(self):
         result = self.hardware.update_firmware(100, ipmi=False, hard_drive=False)
 
         self.assertEqual(result, True)
         self.assert_called_with('SoftLayer_Hardware_Server', 'createFirmwareUpdateTransaction',
-                                identifier=100, args=(0, 1, 1, 0))
+                                identifier=100, args=(0, 1, 1, 0, 1))
 
     def test_reflash_firmware(self):
         result = self.hardware.reflash_firmware(100)