From 4de99b8354c60bb0e6de4d487e03e69d646bef4a Mon Sep 17 00:00:00 2001 From: Felipe Garcia Bulsoni Date: Tue, 12 Sep 2017 13:54:48 -0300 Subject: [PATCH 1/2] Add extra condition to check if osCustomAttributes resolves to None --- CHANGELOG.md | 1 + library/oneview_server_profile.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90219b23..44b4e115 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - [#172](https://github.com/HewlettPackard/oneview-ansible/issues/172) Allow credentials to be defined inside the playbooks - [#282](https://github.com/HewlettPackard/oneview-ansible/issues/282) Updating a Server Profile causes Server Hardware reboots for operations which do not require it - [#285](https://github.com/HewlettPackard/oneview-ansible/issues/285) Modules cannot unassign a Server Hardware or create SP with unassigned SH +- [#288](https://github.com/HewlettPackard/oneview-ansible/issues/288) Adding osCustomAttributes to a SP which had none before results in failure # v4.0.0 #### Notes diff --git a/library/oneview_server_profile.py b/library/oneview_server_profile.py index ea297c95..6ea35ba3 100644 --- a/library/oneview_server_profile.py +++ b/library/oneview_server_profile.py @@ -332,7 +332,7 @@ def __present(self, data, resource): # Removes .mac entries from resource os_custom_attributes if no .mac passed into data params. # Swaps True values for 'true' string, and False values for 'false' string to avoid common user errors. def __validations_for_os_custom_attributes(self, data, merged_data, resource): - if data.get('osDeploymentSettings') is None: + if data.get('osDeploymentSettings') is None or resource.get('osDeploymentSettings') is None: return False elif data.get('osDeploymentSettings').get('osCustomAttributes') is None: return False From 91546677f470b70ca4d20ac154a9886a4109ad46 Mon Sep 17 00:00:00 2001 From: Felipe Garcia Bulsoni Date: Tue, 12 Sep 2017 18:00:41 -0300 Subject: [PATCH 2/2] Bumped up test coverage --- library/oneview_server_profile.py | 9 +++++--- test/test_oneview_server_profile.py | 32 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/library/oneview_server_profile.py b/library/oneview_server_profile.py index 6ea35ba3..9d2ce1bd 100644 --- a/library/oneview_server_profile.py +++ b/library/oneview_server_profile.py @@ -321,6 +321,7 @@ def __present(self, data, resource): self.__validations_for_os_custom_attributes(data, merged_data, resource) if not ResourceComparator.compare(resource, merged_data): + resource = self.__update_server_profile(merged_data, resource) changed = True msg = self.MSG_UPDATED @@ -333,9 +334,11 @@ def __present(self, data, resource): # Swaps True values for 'true' string, and False values for 'false' string to avoid common user errors. def __validations_for_os_custom_attributes(self, data, merged_data, resource): if data.get('osDeploymentSettings') is None or resource.get('osDeploymentSettings') is None: - return False - elif data.get('osDeploymentSettings').get('osCustomAttributes') is None: - return False + return + elif data.get('osDeploymentSettings', {}).get('osCustomAttributes') is None: + return + elif resource.get('osDeploymentSettings', {}).get('osCustomAttributes') is None: + return attributes_merged = merged_data.get('osDeploymentSettings', {}).get('osCustomAttributes', None) attributes_resource = resource.get('osDeploymentSettings', {}).get('osCustomAttributes', None) dp_uri = resource.get('osDeploymentSettings', {}).get('osDeploymentPlanUri', None) diff --git a/test/test_oneview_server_profile.py b/test/test_oneview_server_profile.py index 99dc9bb5..ea8b4034 100644 --- a/test/test_oneview_server_profile.py +++ b/test/test_oneview_server_profile.py @@ -1904,6 +1904,38 @@ def test_should_unassign_server_hardware(self): ansible_facts=mock_facts ) + def test_validating_os_custom_attr_should_return_if_no_attributes_found_on_resource(self): + sp_get_value = deepcopy(CREATED_BASIC_PROFILE) + sp_get_value['osDeploymentSettings'] = {} + sp_exit_value = deepcopy(CREATED_BASIC_PROFILE) + sp_exit_value.update(deepcopy(PARAMS_FOR_UPDATE['data'])) + + self.mock_ov_client.server_profiles.get_by_name.return_value = sp_get_value + self.mock_ov_client.server_profiles.update.return_value = deepcopy(CREATED_BASIC_PROFILE) + self.mock_ansible_module.params = deepcopy(PARAMS_FOR_UPDATE) + + ServerProfileModule().run() + + self.mock_ov_client.server_profiles.update.assert_called_once_with(sp_exit_value, sp_exit_value['uri']) + + def test_validating_os_custom_attr_should_return_if_no_attributes_found_on_data(self): + sp_get_value = deepcopy(CREATED_BASIC_PROFILE) + sp_get_value['osDeploymentSettings'] = {} + sp_exit_value = deepcopy(CREATED_BASIC_PROFILE) + sp_exit_value.update(deepcopy(PARAMS_FOR_UPDATE['data'])) + sp_exit_value['osDeploymentSettings']['osCustomAttributes'] = None + + params_for_update = deepcopy(PARAMS_FOR_UPDATE) + params_for_update['data']['osDeploymentSettings']['osCustomAttributes'] = None + + self.mock_ov_client.server_profiles.get_by_name.return_value = sp_get_value + self.mock_ov_client.server_profiles.update.return_value = deepcopy(CREATED_BASIC_PROFILE) + self.mock_ansible_module.params = params_for_update + + ServerProfileModule().run() + + self.mock_ov_client.server_profiles.update.assert_called_once_with(sp_exit_value, sp_exit_value['uri']) + if __name__ == '__main__': unittest.main()