omevv_firmware - Defect fix (#791)

* Initial commit

* Single host update

* Handeled exception for 'existing update job check'

* Fixed sonar issues - code smells

* Added check_mode, diff mode and idempotency support

* Incorporated review meeting comments

* Incorporated few more review comments

* Cluster level firmware update

* Fixed defects - ECS02A-106, ECS02A-107, ECS02A-108, ECS02A-110 and ECS02A-111

* Fixed defect - ECS02A-112

* Fixed sonar issues

* Fixed sonar issue

* Incorporated review comments

* Added diff mode ouput for success case

* Fixed defects - ECS02A-115

* Fixed defects - ECS02A-116

* Fixed sonar issues

* Added UT

* Updated unit test and added idrac_certificate integration test

* Fixed sonar issues

* Fixed few more sonar issues

* Updated unit issue

* Addressed review comments and fixed sonar issue

* Fixed sonar issue and addressed review comments

* Fixed sonar and updated UT

* Fixed sonar issues

* Updated UT

* Fixed code smells

* Added doc rst file

* Addressed review comments

* Updated UT

* Udated unit test

* Updated version UT file

* Fixed defects - ECS02A-134

* Reverted ECS02A-132 defect fix code

plugins/modules/omevv_firmware.py

@@ -658,21 +658,20 @@ def execute(self):
 
         self.is_job_name_existing(vcenter_uuid, self.module.params.get('job_name'))
 
-        firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict = self.is_firmware_update_needed(
+        firmware_update_needed, before_dict, after_dict = self.is_firmware_update_needed(
             vcenter_uuid, cluster_group_id, new_host_id, parameters['targets'], host_service_tags)
 
         if self.module.check_mode:
-            self.handle_check_mode(firmware_update_needed, before_no_change_dict,
-                                   after_no_change_dict, before_dict, after_dict)
+            self.handle_check_mode(firmware_update_needed, before_dict, after_dict)
 
         if firmware_update_needed:
             self.handle_firmware_update(vcenter_uuid, cluster_group_id, payload, parameters,
                                         before_dict, after_dict)
 
         else:
             self.module.exit_json(msg=CHANGES_NOT_FOUND_MSG,
-                                  diff={"before": before_no_change_dict,
-                                        "after": after_no_change_dict}, changed=False)
+                                  diff={"before": before_dict,
+                                        "after": after_dict}, changed=False)
 
     def process_cluster_target(self, target):
         """
@@ -741,15 +740,12 @@ def get_host_from_parameters(self, vcenter_uuid, parameters, host_ids=None,
             host_ids, host_service_tags = self.get_host_id_either_host_or_service_tag(vcenter_uuid, target)
             return host_ids, host_service_tags
 
-    def handle_check_mode(self, firmware_update_needed, before_no_change_dict,
-                          after_no_change_dict, before_dict, after_dict):
+    def handle_check_mode(self, firmware_update_needed, before_dict, after_dict):
         """
         Handles the check mode for firmware update.
 
         Args:
             firmware_update_needed (bool): Indicates if firmware update is needed.
-            before_no_change_dict (dict): The dictionary representing the state before no changes.
-            after_no_change_dict (dict): The dictionary representing the state after no changes.
             before_dict (dict): The dictionary representing the state before the update.
             after_dict (dict): The dictionary representing the state after the update.
 
@@ -765,8 +761,8 @@ def handle_check_mode(self, firmware_update_needed, before_no_change_dict,
         else:
             if self.module._diff:
                 self.module.exit_json(msg=CHANGES_NOT_FOUND_MSG, changed=False,
-                                      diff={"before": before_no_change_dict,
-                                            "after": after_no_change_dict})
+                                      diff={"before": before_dict,
+                                            "after": after_dict})
             else:
                 self.module.exit_json(msg=CHANGES_NOT_FOUND_MSG, changed=False)
 
@@ -873,19 +869,15 @@ def is_firmware_update_needed(self, vcenter_uuid, cluster_group_id, host_ids,
 
         main_before_dict = {}
         main_after_dict = {}
-        main_before_no_change_dict = {}
-        main_after_no_change_dict = {}
         for idx, one_host_id in enumerate(host_ids):
-            update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, current_host_st = self.check_firmware_update(
+            update_needed, before_dict, after_dict, current_host_st = self.check_firmware_update(
                 vcenter_uuid, cluster_group_id, one_host_id, target)
             main_before_dict[current_host_st] = before_dict
             main_after_dict[current_host_st] = after_dict
-            main_before_no_change_dict[current_host_st] = before_no_change_dict
-            main_after_no_change_dict[current_host_st] = after_no_change_dict
 
             firmware_update_needed = firmware_update_needed or update_needed
 
-        return firmware_update_needed, main_before_no_change_dict, main_after_no_change_dict, main_before_dict, main_after_dict
+        return firmware_update_needed, main_before_dict, main_after_dict
 
     def check_firmware_update(self, vcenter_uuid, cluster_group_id, host_id, target):
         """
@@ -900,14 +892,12 @@ def check_firmware_update(self, vcenter_uuid, cluster_group_id, host_id, target)
         Returns:
             tuple: A tuple containing the following:
                 - firmware_update_needed (bool): A boolean indicating if firmware update is needed.
-                - before_no_change_dict (dict): A dictionary containing the firmware version before the update.
-                - after_no_change_dict (dict): A dictionary containing the firmware version after the update.
                 - before_dict (dict): A dictionary containing the firmware version before the update.
                 - after_dict (dict): A dictionary containing the firmware version after the update.
                 - current_host_st (str): The service tag of the current host.
         """
         current_host_st = None
-        before_dict, after_dict, before_no_change_dict, after_no_change_dict = {}, {}, {}, {}
+        before_dict, after_dict = {}, {}
         if 'cluster' in target[0] and target[0]['cluster']:
             cluster_firmware_drift_info = self.omevv_info_obj.get_firmware_drift_info_for_single_host(
                 vcenter_uuid, cluster_group_id, host_id)
@@ -923,23 +913,17 @@ def check_firmware_update(self, vcenter_uuid, cluster_group_id, host_id, target)
                 'hostComplianceReports', [])[0].get('componentCompliances', [])
 
         required_firmware_components = target[0]['firmware_components']
-        firmware_update_needed = False
 
         for component in required_firmware_components:
             for current_component in current_firmware_components:
-                if current_component['sourceName'] == component and current_component['driftStatus'] == 'NonCompliant':
+                if current_component['sourceName'] == component:
                     before_dict[current_component["sourceName"]] = {
                         "firmwareversion": current_component["currentValue"]}
                     after_dict[current_component["sourceName"]] = {
                         "firmwareversion": current_component["baselineValue"]}
-                    firmware_update_needed = True
-                elif current_component['sourceName'] == component and current_component['driftStatus'] == 'Compliant':
-                    before_no_change_dict[current_component["sourceName"]] = {
-                        "firmwareversion": current_component["currentValue"]}
-                    after_no_change_dict[current_component["sourceName"]] = {
-                        "firmwareversion": current_component["baselineValue"]}
+        firmware_update_needed = (before_dict != after_dict)
 
-        return firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, current_host_st
+        return firmware_update_needed, before_dict, after_dict, current_host_st
 
     def is_update_job_allowed(self, vcenter_uuid, cluster_group_id, cluster_name):
         """

tests/unit/plugins/modules/test_omevv_firmware.py

@@ -361,7 +361,7 @@ def test_execute_check_mode_false(self, mocker, omevv_connection_firmware,
         value = (1034, {}, 1001)
         mocker.patch(MODULE_PATH + 'UpdateCluster.process_cluster_target', return_value=value)
         mocker.patch(MODULE_PATH + 'UpdateCluster.is_firmware_update_needed',
-                     return_value=(False, 2, 3, 4, 5))
+                     return_value=(False, 2, 3))
         mocker.patch(MODULE_PATH + 'UpdateCluster.is_update_job_allowed', return_value=True)
         mocker.patch(MODULE_PATH + 'UpdateCluster.is_job_name_existing', return_value=None)
         mocker.patch(MODULE_PATH + 'UpdateCluster.handle_check_mode', return_value=None)
@@ -494,16 +494,12 @@ def test_handle_check_mode_firmware_update_needed_change(self,
 
         # Setup the parameters for the test
         firmware_update_needed = True
-        before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
-        after_no_change_dict = {'component1': {'firmwareversion': '1.0.1'}}
         before_dict = {'component2': {'firmwareversion': '2.0.0'}}
         after_dict = {'component2': {'firmwareversion': '2.0.1'}}
 
         # Execute the method with change
         with pytest.raises(AnsibleFailJSonException) as excinfo:
             omevv_obj.handle_check_mode(firmware_update_needed,
-                                        before_no_change_dict,
-                                        after_no_change_dict,
                                         before_dict, after_dict)
 
         assert excinfo.value.args[0] == CHANGES_FOUND_MSG
@@ -517,16 +513,12 @@ def test_handle_check_mode_firmware_update_needed_no_diff(self,
 
         # Setup the parameters for the test
         firmware_update_needed = True
-        before_no_change_dict = {}
-        after_no_change_dict = {}
         before_dict = {}
         after_dict = {}
 
         # Execute the method without diff
         with pytest.raises(AnsibleFailJSonException) as excinfo:
             omevv_obj.handle_check_mode(firmware_update_needed,
-                                        before_no_change_dict,
-                                        after_no_change_dict,
                                         before_dict, after_dict)
 
         assert excinfo.value.args[0] == CHANGES_FOUND_MSG
@@ -540,16 +532,12 @@ def test_handle_check_mode_no_firmware_update_needed_with_diff(self,
 
         # Setup the parameters for the test
         firmware_update_needed = False
-        before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
-        after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
         before_dict = {}
         after_dict = {}
 
         # Execute the method with no change
         with pytest.raises(AnsibleFailJSonException) as excinfo:
             omevv_obj.handle_check_mode(firmware_update_needed,
-                                        before_no_change_dict,
-                                        after_no_change_dict,
                                         before_dict, after_dict)
 
         assert excinfo.value.args[0] == CHANGES_NOT_FOUND_MSG
@@ -563,16 +551,12 @@ def test_handle_check_mode_no_firmware_update_needed_no_diff(self,
 
         # Setup the parameters for the test
         firmware_update_needed = False
-        before_no_change_dict = {}
-        after_no_change_dict = {}
         before_dict = {}
         after_dict = {}
 
         # Execute the method without diff and no change
         with pytest.raises(AnsibleFailJSonException) as excinfo:
             omevv_obj.handle_check_mode(firmware_update_needed,
-                                        before_no_change_dict,
-                                        after_no_change_dict,
                                         before_dict, after_dict)
 
         assert excinfo.value.args[0] == CHANGES_NOT_FOUND_MSG
@@ -584,8 +568,6 @@ def test_handle_check_mode_changes_needed_with_diff(self, mocker, omevv_connecti
         omevv_obj = UpdateCluster(f_module, omevv_connection_firmware)
 
         firmware_update_needed = True
-        before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
-        after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
         before_dict = {'component2': {'firmwareversion': '2.0.0'}}
         after_dict = {'component2': {'firmwareversion': '3.0.0'}}
 
@@ -595,8 +577,6 @@ def test_handle_check_mode_changes_needed_with_diff(self, mocker, omevv_connecti
 
         with pytest.raises(AnsibleFailJSonException) as excinfo:
             omevv_obj.handle_check_mode(firmware_update_needed,
-                                        before_no_change_dict,
-                                        after_no_change_dict,
                                         before_dict, after_dict)
 
         # Verify the exit message for changes found with diff
@@ -609,8 +589,6 @@ def test_handle_check_mode_changes_needed_without_diff(self, mocker, omevv_conne
         omevv_obj = UpdateCluster(f_module, omevv_connection_firmware)
 
         firmware_update_needed = True
-        before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
-        after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
         before_dict = {'component2': {'firmwareversion': '2.0.0'}}
         after_dict = {'component2': {'firmwareversion': '3.0.0'}}
 
@@ -619,8 +597,7 @@ def test_handle_check_mode_changes_needed_without_diff(self, mocker, omevv_conne
                      side_effect=AnsibleFailJSonException)
 
         with pytest.raises(AnsibleFailJSonException) as excinfo:
-            omevv_obj.handle_check_mode(firmware_update_needed, before_no_change_dict,
-                                        after_no_change_dict, before_dict, after_dict)
+            omevv_obj.handle_check_mode(firmware_update_needed, before_dict, after_dict)
 
         # Verify the exit message for changes found without diff
         assert excinfo.value.args[0] == CHANGES_FOUND_MSG
@@ -632,8 +609,6 @@ def test_handle_check_mode_no_changes_needed_with_diff(self, mocker, omevv_conne
         omevv_obj = UpdateCluster(f_module, omevv_connection_firmware)
 
         firmware_update_needed = False
-        before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
-        after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
         before_dict = {'component2': {'firmwareversion': '2.0.0'}}
         after_dict = {'component2': {'firmwareversion': '3.0.0'}}
 
@@ -642,8 +617,7 @@ def test_handle_check_mode_no_changes_needed_with_diff(self, mocker, omevv_conne
                      side_effect=AnsibleFailJSonException)
 
         with pytest.raises(AnsibleFailJSonException) as excinfo:
-            omevv_obj.handle_check_mode(firmware_update_needed, before_no_change_dict,
-                                        after_no_change_dict, before_dict, after_dict)
+            omevv_obj.handle_check_mode(firmware_update_needed, before_dict, after_dict)
 
         # Verify the exit message for no changes found with diff
         assert excinfo.value.args[0] == CHANGES_NOT_FOUND_MSG
@@ -656,8 +630,6 @@ def test_handle_check_mode_no_changes_needed_without_diff(self, mocker,
         omevv_obj = UpdateCluster(f_module, omevv_connection_firmware)
 
         firmware_update_needed = False
-        before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
-        after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
         before_dict = {'component2': {'firmwareversion': '2.0.0'}}
         after_dict = {'component2': {'firmwareversion': '3.0.0'}}
 
@@ -666,8 +638,7 @@ def test_handle_check_mode_no_changes_needed_without_diff(self, mocker,
                      side_effect=AnsibleFailJSonException)
 
         with pytest.raises(AnsibleFailJSonException) as excinfo:
-            omevv_obj.handle_check_mode(firmware_update_needed, before_no_change_dict,
-                                        after_no_change_dict, before_dict, after_dict)
+            omevv_obj.handle_check_mode(firmware_update_needed, before_dict, after_dict)
 
         # Verify the exit message for no changes found without diff
         assert excinfo.value.args[0] == CHANGES_NOT_FOUND_MSG
@@ -1017,10 +988,10 @@ def test_is_firmware_update_needed_update_needed(self, mocker,
         host_service_tags = ['SVCTAG1']
 
         # Mock the `check_firmware_update` method to return that an update is needed
-        mocker.patch(MODULE_PATH + 'UpdateCluster.check_firmware_update', return_value=(True, {}, {}, {}, {}, 'SVCTAG1'))
+        mocker.patch(MODULE_PATH + 'UpdateCluster.check_firmware_update', return_value=(True, {}, {}, 'SVCTAG1'))
 
         # Execute the method
-        firmware_update_needed, dict_0, dict_1, main_before_dict, main_after_dict = omevv_obj.is_firmware_update_needed(
+        firmware_update_needed, main_before_dict, main_after_dict = omevv_obj.is_firmware_update_needed(
             vcenter_uuid, cluster_group_id, host_ids, target, host_service_tags)
 
         # Verify the result
@@ -1042,12 +1013,12 @@ def test_is_firmware_update_needed_multiple_hosts(self, mocker, omevv_connection
 
         # Mock the `check_firmware_update` method to return different results for multiple hosts
         mocker.patch(MODULE_PATH + 'UpdateCluster.check_firmware_update', side_effect=[
-            (True, {}, {}, {}, {}, 'SVCTAG1'),
-            (False, {}, {}, {}, {}, 'SVCTAG2')
+            (True, {}, {}, 'SVCTAG1'),
+            (False, {}, {}, 'SVCTAG2')
         ])
 
         # Execute the method
-        firmware_update_needed, dict_1, dict_2, main_before_dict, main_after_dict = omevv_obj.is_firmware_update_needed(
+        firmware_update_needed, main_before_dict, main_after_dict = omevv_obj.is_firmware_update_needed(
             vcenter_uuid, cluster_group_id, host_ids, target, host_service_tags)
 
         # Verify the result
@@ -1082,14 +1053,12 @@ def test_check_firmware_update_compliant(self, mocker, omevv_connection_firmware
         mocker.patch(MODULE_PATH + OMEVV_INFO_FIRMWARE_DRIFT_INFO,
                      return_value=firmware_drift_info)
 
-        firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
+        firmware_update_needed, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
             vcenter_uuid, cluster_group_id, host_id, target)
 
         assert not firmware_update_needed
-        assert before_no_change_dict == {'component1': {'firmwareversion': '1.0.0'}}
-        assert after_no_change_dict == {'component1': {'firmwareversion': '1.0.0'}}
-        assert before_dict == {}
-        assert after_dict == {}
+        assert before_dict == {'component1': {'firmwareversion': '1.0.0'}}
+        assert after_dict == {'component1': {'firmwareversion': '1.0.0'}}
 
     def test_check_firmware_update_non_compliant(self, mocker, omevv_connection_firmware,
                                                  omevv_default_args):
@@ -1118,12 +1087,10 @@ def test_check_firmware_update_non_compliant(self, mocker, omevv_connection_firm
         mocker.patch(MODULE_PATH + OMEVV_INFO_FIRMWARE_DRIFT_INFO,
                      return_value=firmware_drift_info)
 
-        firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
+        firmware_update_needed, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
             vcenter_uuid, cluster_group_id, host_id, target)
 
         assert firmware_update_needed
-        assert before_no_change_dict == {}
-        assert after_no_change_dict == {}
         assert before_dict == {'component1': {'firmwareversion': '1.0.0'}}
         assert after_dict == {'component1': {'firmwareversion': '2.0.0'}}
 
@@ -1160,14 +1127,12 @@ def test_check_firmware_update_mixed_status(self, mocker, omevv_connection_firmw
         mocker.patch(MODULE_PATH + OMEVV_INFO_FIRMWARE_DRIFT_INFO,
                      return_value=firmware_drift_info)
 
-        firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
+        firmware_update_needed, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
             vcenter_uuid, cluster_group_id, host_id, target)
 
         assert firmware_update_needed
-        assert before_no_change_dict == {'component2': {'firmwareversion': '3.0.0'}}
-        assert after_no_change_dict == {'component2': {'firmwareversion': '3.0.0'}}
-        assert before_dict == {'component1': {'firmwareversion': '1.0.0'}}
-        assert after_dict == {'component1': {'firmwareversion': '2.0.0'}}
+        assert before_dict == {'component1': {'firmwareversion': '1.0.0'}, 'component2': {'firmwareversion': '3.0.0'}}
+        assert after_dict == {'component1': {'firmwareversion': '2.0.0'}, 'component2': {'firmwareversion': '3.0.0'}}
 
     def test_is_update_job_allowed_false(self, mocker,
                                          omevv_connection_firmware, omevv_default_args):