Skip to content

Commit a050d64

Browse files
rajshekarp87rajshekarp87
authored
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
1 parent b883c13 commit a050d64

File tree

2 files changed

+29
-80
lines changed

2 files changed

+29
-80
lines changed

‎plugins/modules/omevv_firmware.py

+13-29
Original file line numberDiff line numberDiff line change
@@ -658,21 +658,20 @@ def execute(self):
658658

659659
self.is_job_name_existing(vcenter_uuid, self.module.params.get('job_name'))
660660

661-
firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict = self.is_firmware_update_needed(
661+
firmware_update_needed, before_dict, after_dict = self.is_firmware_update_needed(
662662
vcenter_uuid, cluster_group_id, new_host_id, parameters['targets'], host_service_tags)
663663

664664
if self.module.check_mode:
665-
self.handle_check_mode(firmware_update_needed, before_no_change_dict,
666-
after_no_change_dict, before_dict, after_dict)
665+
self.handle_check_mode(firmware_update_needed, before_dict, after_dict)
667666

668667
if firmware_update_needed:
669668
self.handle_firmware_update(vcenter_uuid, cluster_group_id, payload, parameters,
670669
before_dict, after_dict)
671670

672671
else:
673672
self.module.exit_json(msg=CHANGES_NOT_FOUND_MSG,
674-
diff={"before": before_no_change_dict,
675-
"after": after_no_change_dict}, changed=False)
673+
diff={"before": before_dict,
674+
"after": after_dict}, changed=False)
676675

677676
def process_cluster_target(self, target):
678677
"""
@@ -741,15 +740,12 @@ def get_host_from_parameters(self, vcenter_uuid, parameters, host_ids=None,
741740
host_ids, host_service_tags = self.get_host_id_either_host_or_service_tag(vcenter_uuid, target)
742741
return host_ids, host_service_tags
743742

744-
def handle_check_mode(self, firmware_update_needed, before_no_change_dict,
745-
after_no_change_dict, before_dict, after_dict):
743+
def handle_check_mode(self, firmware_update_needed, before_dict, after_dict):
746744
"""
747745
Handles the check mode for firmware update.
748746
749747
Args:
750748
firmware_update_needed (bool): Indicates if firmware update is needed.
751-
before_no_change_dict (dict): The dictionary representing the state before no changes.
752-
after_no_change_dict (dict): The dictionary representing the state after no changes.
753749
before_dict (dict): The dictionary representing the state before the update.
754750
after_dict (dict): The dictionary representing the state after the update.
755751
@@ -765,8 +761,8 @@ def handle_check_mode(self, firmware_update_needed, before_no_change_dict,
765761
else:
766762
if self.module._diff:
767763
self.module.exit_json(msg=CHANGES_NOT_FOUND_MSG, changed=False,
768-
diff={"before": before_no_change_dict,
769-
"after": after_no_change_dict})
764+
diff={"before": before_dict,
765+
"after": after_dict})
770766
else:
771767
self.module.exit_json(msg=CHANGES_NOT_FOUND_MSG, changed=False)
772768

@@ -873,19 +869,15 @@ def is_firmware_update_needed(self, vcenter_uuid, cluster_group_id, host_ids,
873869

874870
main_before_dict = {}
875871
main_after_dict = {}
876-
main_before_no_change_dict = {}
877-
main_after_no_change_dict = {}
878872
for idx, one_host_id in enumerate(host_ids):
879-
update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, current_host_st = self.check_firmware_update(
873+
update_needed, before_dict, after_dict, current_host_st = self.check_firmware_update(
880874
vcenter_uuid, cluster_group_id, one_host_id, target)
881875
main_before_dict[current_host_st] = before_dict
882876
main_after_dict[current_host_st] = after_dict
883-
main_before_no_change_dict[current_host_st] = before_no_change_dict
884-
main_after_no_change_dict[current_host_st] = after_no_change_dict
885877

886878
firmware_update_needed = firmware_update_needed or update_needed
887879

888-
return firmware_update_needed, main_before_no_change_dict, main_after_no_change_dict, main_before_dict, main_after_dict
880+
return firmware_update_needed, main_before_dict, main_after_dict
889881

890882
def check_firmware_update(self, vcenter_uuid, cluster_group_id, host_id, target):
891883
"""
@@ -900,14 +892,12 @@ def check_firmware_update(self, vcenter_uuid, cluster_group_id, host_id, target)
900892
Returns:
901893
tuple: A tuple containing the following:
902894
- firmware_update_needed (bool): A boolean indicating if firmware update is needed.
903-
- before_no_change_dict (dict): A dictionary containing the firmware version before the update.
904-
- after_no_change_dict (dict): A dictionary containing the firmware version after the update.
905895
- before_dict (dict): A dictionary containing the firmware version before the update.
906896
- after_dict (dict): A dictionary containing the firmware version after the update.
907897
- current_host_st (str): The service tag of the current host.
908898
"""
909899
current_host_st = None
910-
before_dict, after_dict, before_no_change_dict, after_no_change_dict = {}, {}, {}, {}
900+
before_dict, after_dict = {}, {}
911901
if 'cluster' in target[0] and target[0]['cluster']:
912902
cluster_firmware_drift_info = self.omevv_info_obj.get_firmware_drift_info_for_single_host(
913903
vcenter_uuid, cluster_group_id, host_id)
@@ -923,23 +913,17 @@ def check_firmware_update(self, vcenter_uuid, cluster_group_id, host_id, target)
923913
'hostComplianceReports', [])[0].get('componentCompliances', [])
924914

925915
required_firmware_components = target[0]['firmware_components']
926-
firmware_update_needed = False
927916

928917
for component in required_firmware_components:
929918
for current_component in current_firmware_components:
930-
if current_component['sourceName'] == component and current_component['driftStatus'] == 'NonCompliant':
919+
if current_component['sourceName'] == component:
931920
before_dict[current_component["sourceName"]] = {
932921
"firmwareversion": current_component["currentValue"]}
933922
after_dict[current_component["sourceName"]] = {
934923
"firmwareversion": current_component["baselineValue"]}
935-
firmware_update_needed = True
936-
elif current_component['sourceName'] == component and current_component['driftStatus'] == 'Compliant':
937-
before_no_change_dict[current_component["sourceName"]] = {
938-
"firmwareversion": current_component["currentValue"]}
939-
after_no_change_dict[current_component["sourceName"]] = {
940-
"firmwareversion": current_component["baselineValue"]}
924+
firmware_update_needed = (before_dict != after_dict)
941925

942-
return firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, current_host_st
926+
return firmware_update_needed, before_dict, after_dict, current_host_st
943927

944928
def is_update_job_allowed(self, vcenter_uuid, cluster_group_id, cluster_name):
945929
"""

‎tests/unit/plugins/modules/test_omevv_firmware.py

+16-51
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ def test_execute_check_mode_false(self, mocker, omevv_connection_firmware,
361361
value = (1034, {}, 1001)
362362
mocker.patch(MODULE_PATH + 'UpdateCluster.process_cluster_target', return_value=value)
363363
mocker.patch(MODULE_PATH + 'UpdateCluster.is_firmware_update_needed',
364-
return_value=(False, 2, 3, 4, 5))
364+
return_value=(False, 2, 3))
365365
mocker.patch(MODULE_PATH + 'UpdateCluster.is_update_job_allowed', return_value=True)
366366
mocker.patch(MODULE_PATH + 'UpdateCluster.is_job_name_existing', return_value=None)
367367
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,
494494

495495
# Setup the parameters for the test
496496
firmware_update_needed = True
497-
before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
498-
after_no_change_dict = {'component1': {'firmwareversion': '1.0.1'}}
499497
before_dict = {'component2': {'firmwareversion': '2.0.0'}}
500498
after_dict = {'component2': {'firmwareversion': '2.0.1'}}
501499

502500
# Execute the method with change
503501
with pytest.raises(AnsibleFailJSonException) as excinfo:
504502
omevv_obj.handle_check_mode(firmware_update_needed,
505-
before_no_change_dict,
506-
after_no_change_dict,
507503
before_dict, after_dict)
508504

509505
assert excinfo.value.args[0] == CHANGES_FOUND_MSG
@@ -517,16 +513,12 @@ def test_handle_check_mode_firmware_update_needed_no_diff(self,
517513

518514
# Setup the parameters for the test
519515
firmware_update_needed = True
520-
before_no_change_dict = {}
521-
after_no_change_dict = {}
522516
before_dict = {}
523517
after_dict = {}
524518

525519
# Execute the method without diff
526520
with pytest.raises(AnsibleFailJSonException) as excinfo:
527521
omevv_obj.handle_check_mode(firmware_update_needed,
528-
before_no_change_dict,
529-
after_no_change_dict,
530522
before_dict, after_dict)
531523

532524
assert excinfo.value.args[0] == CHANGES_FOUND_MSG
@@ -540,16 +532,12 @@ def test_handle_check_mode_no_firmware_update_needed_with_diff(self,
540532

541533
# Setup the parameters for the test
542534
firmware_update_needed = False
543-
before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
544-
after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
545535
before_dict = {}
546536
after_dict = {}
547537

548538
# Execute the method with no change
549539
with pytest.raises(AnsibleFailJSonException) as excinfo:
550540
omevv_obj.handle_check_mode(firmware_update_needed,
551-
before_no_change_dict,
552-
after_no_change_dict,
553541
before_dict, after_dict)
554542

555543
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,
563551

564552
# Setup the parameters for the test
565553
firmware_update_needed = False
566-
before_no_change_dict = {}
567-
after_no_change_dict = {}
568554
before_dict = {}
569555
after_dict = {}
570556

571557
# Execute the method without diff and no change
572558
with pytest.raises(AnsibleFailJSonException) as excinfo:
573559
omevv_obj.handle_check_mode(firmware_update_needed,
574-
before_no_change_dict,
575-
after_no_change_dict,
576560
before_dict, after_dict)
577561

578562
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
584568
omevv_obj = UpdateCluster(f_module, omevv_connection_firmware)
585569

586570
firmware_update_needed = True
587-
before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
588-
after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
589571
before_dict = {'component2': {'firmwareversion': '2.0.0'}}
590572
after_dict = {'component2': {'firmwareversion': '3.0.0'}}
591573

@@ -595,8 +577,6 @@ def test_handle_check_mode_changes_needed_with_diff(self, mocker, omevv_connecti
595577

596578
with pytest.raises(AnsibleFailJSonException) as excinfo:
597579
omevv_obj.handle_check_mode(firmware_update_needed,
598-
before_no_change_dict,
599-
after_no_change_dict,
600580
before_dict, after_dict)
601581

602582
# 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
609589
omevv_obj = UpdateCluster(f_module, omevv_connection_firmware)
610590

611591
firmware_update_needed = True
612-
before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
613-
after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
614592
before_dict = {'component2': {'firmwareversion': '2.0.0'}}
615593
after_dict = {'component2': {'firmwareversion': '3.0.0'}}
616594

@@ -619,8 +597,7 @@ def test_handle_check_mode_changes_needed_without_diff(self, mocker, omevv_conne
619597
side_effect=AnsibleFailJSonException)
620598

621599
with pytest.raises(AnsibleFailJSonException) as excinfo:
622-
omevv_obj.handle_check_mode(firmware_update_needed, before_no_change_dict,
623-
after_no_change_dict, before_dict, after_dict)
600+
omevv_obj.handle_check_mode(firmware_update_needed, before_dict, after_dict)
624601

625602
# Verify the exit message for changes found without diff
626603
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
632609
omevv_obj = UpdateCluster(f_module, omevv_connection_firmware)
633610

634611
firmware_update_needed = False
635-
before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
636-
after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
637612
before_dict = {'component2': {'firmwareversion': '2.0.0'}}
638613
after_dict = {'component2': {'firmwareversion': '3.0.0'}}
639614

@@ -642,8 +617,7 @@ def test_handle_check_mode_no_changes_needed_with_diff(self, mocker, omevv_conne
642617
side_effect=AnsibleFailJSonException)
643618

644619
with pytest.raises(AnsibleFailJSonException) as excinfo:
645-
omevv_obj.handle_check_mode(firmware_update_needed, before_no_change_dict,
646-
after_no_change_dict, before_dict, after_dict)
620+
omevv_obj.handle_check_mode(firmware_update_needed, before_dict, after_dict)
647621

648622
# Verify the exit message for no changes found with diff
649623
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,
656630
omevv_obj = UpdateCluster(f_module, omevv_connection_firmware)
657631

658632
firmware_update_needed = False
659-
before_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
660-
after_no_change_dict = {'component1': {'firmwareversion': '1.0.0'}}
661633
before_dict = {'component2': {'firmwareversion': '2.0.0'}}
662634
after_dict = {'component2': {'firmwareversion': '3.0.0'}}
663635

@@ -666,8 +638,7 @@ def test_handle_check_mode_no_changes_needed_without_diff(self, mocker,
666638
side_effect=AnsibleFailJSonException)
667639

668640
with pytest.raises(AnsibleFailJSonException) as excinfo:
669-
omevv_obj.handle_check_mode(firmware_update_needed, before_no_change_dict,
670-
after_no_change_dict, before_dict, after_dict)
641+
omevv_obj.handle_check_mode(firmware_update_needed, before_dict, after_dict)
671642

672643
# Verify the exit message for no changes found without diff
673644
assert excinfo.value.args[0] == CHANGES_NOT_FOUND_MSG
@@ -1017,10 +988,10 @@ def test_is_firmware_update_needed_update_needed(self, mocker,
1017988
host_service_tags = ['SVCTAG1']
1018989

1019990
# Mock the `check_firmware_update` method to return that an update is needed
1020-
mocker.patch(MODULE_PATH + 'UpdateCluster.check_firmware_update', return_value=(True, {}, {}, {}, {}, 'SVCTAG1'))
991+
mocker.patch(MODULE_PATH + 'UpdateCluster.check_firmware_update', return_value=(True, {}, {}, 'SVCTAG1'))
1021992

1022993
# Execute the method
1023-
firmware_update_needed, dict_0, dict_1, main_before_dict, main_after_dict = omevv_obj.is_firmware_update_needed(
994+
firmware_update_needed, main_before_dict, main_after_dict = omevv_obj.is_firmware_update_needed(
1024995
vcenter_uuid, cluster_group_id, host_ids, target, host_service_tags)
1025996

1026997
# Verify the result
@@ -1042,12 +1013,12 @@ def test_is_firmware_update_needed_multiple_hosts(self, mocker, omevv_connection
10421013

10431014
# Mock the `check_firmware_update` method to return different results for multiple hosts
10441015
mocker.patch(MODULE_PATH + 'UpdateCluster.check_firmware_update', side_effect=[
1045-
(True, {}, {}, {}, {}, 'SVCTAG1'),
1046-
(False, {}, {}, {}, {}, 'SVCTAG2')
1016+
(True, {}, {}, 'SVCTAG1'),
1017+
(False, {}, {}, 'SVCTAG2')
10471018
])
10481019

10491020
# Execute the method
1050-
firmware_update_needed, dict_1, dict_2, main_before_dict, main_after_dict = omevv_obj.is_firmware_update_needed(
1021+
firmware_update_needed, main_before_dict, main_after_dict = omevv_obj.is_firmware_update_needed(
10511022
vcenter_uuid, cluster_group_id, host_ids, target, host_service_tags)
10521023

10531024
# Verify the result
@@ -1082,14 +1053,12 @@ def test_check_firmware_update_compliant(self, mocker, omevv_connection_firmware
10821053
mocker.patch(MODULE_PATH + OMEVV_INFO_FIRMWARE_DRIFT_INFO,
10831054
return_value=firmware_drift_info)
10841055

1085-
firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
1056+
firmware_update_needed, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
10861057
vcenter_uuid, cluster_group_id, host_id, target)
10871058

10881059
assert not firmware_update_needed
1089-
assert before_no_change_dict == {'component1': {'firmwareversion': '1.0.0'}}
1090-
assert after_no_change_dict == {'component1': {'firmwareversion': '1.0.0'}}
1091-
assert before_dict == {}
1092-
assert after_dict == {}
1060+
assert before_dict == {'component1': {'firmwareversion': '1.0.0'}}
1061+
assert after_dict == {'component1': {'firmwareversion': '1.0.0'}}
10931062

10941063
def test_check_firmware_update_non_compliant(self, mocker, omevv_connection_firmware,
10951064
omevv_default_args):
@@ -1118,12 +1087,10 @@ def test_check_firmware_update_non_compliant(self, mocker, omevv_connection_firm
11181087
mocker.patch(MODULE_PATH + OMEVV_INFO_FIRMWARE_DRIFT_INFO,
11191088
return_value=firmware_drift_info)
11201089

1121-
firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
1090+
firmware_update_needed, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
11221091
vcenter_uuid, cluster_group_id, host_id, target)
11231092

11241093
assert firmware_update_needed
1125-
assert before_no_change_dict == {}
1126-
assert after_no_change_dict == {}
11271094
assert before_dict == {'component1': {'firmwareversion': '1.0.0'}}
11281095
assert after_dict == {'component1': {'firmwareversion': '2.0.0'}}
11291096

@@ -1160,14 +1127,12 @@ def test_check_firmware_update_mixed_status(self, mocker, omevv_connection_firmw
11601127
mocker.patch(MODULE_PATH + OMEVV_INFO_FIRMWARE_DRIFT_INFO,
11611128
return_value=firmware_drift_info)
11621129

1163-
firmware_update_needed, before_no_change_dict, after_no_change_dict, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
1130+
firmware_update_needed, before_dict, after_dict, st_1 = omevv_obj.check_firmware_update(
11641131
vcenter_uuid, cluster_group_id, host_id, target)
11651132

11661133
assert firmware_update_needed
1167-
assert before_no_change_dict == {'component2': {'firmwareversion': '3.0.0'}}
1168-
assert after_no_change_dict == {'component2': {'firmwareversion': '3.0.0'}}
1169-
assert before_dict == {'component1': {'firmwareversion': '1.0.0'}}
1170-
assert after_dict == {'component1': {'firmwareversion': '2.0.0'}}
1134+
assert before_dict == {'component1': {'firmwareversion': '1.0.0'}, 'component2': {'firmwareversion': '3.0.0'}}
1135+
assert after_dict == {'component1': {'firmwareversion': '2.0.0'}, 'component2': {'firmwareversion': '3.0.0'}}
11711136

11721137
def test_is_update_job_allowed_false(self, mocker,
11731138
omevv_connection_firmware, omevv_default_args):

0 commit comments

Comments
 (0)