From 492190aae01b6587be9e4280a819e2d5a1fdbf97 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Sun, 16 Feb 2014 16:17:20 -0800 Subject: [PATCH] Minor refactoring for Hyper-V utils and tests A separate commit in the blueprint addressed by this patch introduces the "_filter_acls" and "_create_acl" methods which can be used in "enable_port_metrics_collection" as well to reduce code duplication. This commit eliminates also some code duplication in test_hyperv_utilsv2.py. Implements: blueprint hyperv-security-groups Change-Id: I48fb5389b6049641ca2649990e81e94e4c45ef7f --- neutron/plugins/hyperv/agent/utilsv2.py | 18 +-- .../tests/unit/hyperv/test_hyperv_utilsv2.py | 107 +++++++----------- 2 files changed, 48 insertions(+), 77 deletions(-) diff --git a/neutron/plugins/hyperv/agent/utilsv2.py b/neutron/plugins/hyperv/agent/utilsv2.py index 5b280c1853e..cc38db139ab 100644 --- a/neutron/plugins/hyperv/agent/utilsv2.py +++ b/neutron/plugins/hyperv/agent/utilsv2.py @@ -204,18 +204,12 @@ class HyperVUtilsV2(utils.HyperVUtils): acls = port.associators(wmi_result_class=self._PORT_ALLOC_ACL_SET_DATA) for acl_type in [self._ACL_TYPE_IPV4, self._ACL_TYPE_IPV6]: for acl_dir in [self._ACL_DIR_IN, self._ACL_DIR_OUT]: - acls = [v for v in acls - if v.Action == self._ACL_ACTION_METER and - v.Applicability == self._ACL_APPLICABILITY_LOCAL and - v.Direction == acl_dir and - v.AclType == acl_type] - if not acls: - acl = self._get_default_setting_data( - self._PORT_ALLOC_ACL_SET_DATA) - acl.AclType = acl_type - acl.Direction = acl_dir - acl.Action = self._ACL_ACTION_METER - acl.Applicability = self._ACL_APPLICABILITY_LOCAL + _acls = self._filter_acls( + acls, self._ACL_ACTION_METER, acl_dir, acl_type) + + if not _acls: + acl = self._create_acl( + acl_dir, acl_type, self._ACL_ACTION_METER) self._add_virt_feature(port, acl) def create_security_rule(self, switch_port_name, direction, acl_type, diff --git a/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py b/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py index b30d4785ab2..565368a24e5 100644 --- a/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py +++ b/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py @@ -88,83 +88,60 @@ class TestHyperVUtilsV2(base.BaseTestCase): self._utils._modify_virt_resource.assert_called_with(mock_port) def test_add_virt_resource(self): - mock_svc = self._utils._conn.Msvm_VirtualSystemManagementService()[0] - mock_svc.AddResourceSettings.return_value = (self._FAKE_JOB_PATH, - mock.MagicMock(), - self._FAKE_RET_VAL) - mock_res_setting_data = mock.MagicMock() - mock_res_setting_data.GetText_.return_value = self._FAKE_RES_DATA - - mock_vm = mock.MagicMock() - mock_vm.path_.return_value = self._FAKE_VM_PATH - - self._utils._check_job_status = mock.MagicMock() - - self._utils._add_virt_resource(mock_vm, mock_res_setting_data) - - mock_svc.AddResourceSettings.assert_called_with(self._FAKE_VM_PATH, - [self._FAKE_RES_DATA]) + self._test_virt_method('AddResourceSettings', 3, '_add_virt_resource', + True, self._FAKE_VM_PATH, [self._FAKE_RES_DATA]) def test_add_virt_feature(self): - mock_svc = self._utils._conn.Msvm_VirtualSystemManagementService()[0] - mock_svc.AddFeatureSettings.return_value = (self._FAKE_JOB_PATH, - mock.MagicMock(), - self._FAKE_RET_VAL) - mock_res_setting_data = mock.MagicMock() - mock_res_setting_data.GetText_.return_value = self._FAKE_RES_DATA - - mock_vm = mock.MagicMock() - mock_vm.path_.return_value = self._FAKE_VM_PATH - - self._utils._check_job_status = mock.MagicMock() - - self._utils._add_virt_feature(mock_vm, mock_res_setting_data) - - mock_svc.AddFeatureSettings.assert_called_once_with( - self._FAKE_VM_PATH, [self._FAKE_RES_DATA]) + self._test_virt_method('AddFeatureSettings', 3, '_add_virt_feature', + True, self._FAKE_VM_PATH, [self._FAKE_RES_DATA]) def test_modify_virt_resource(self): - mock_svc = self._utils._conn.Msvm_VirtualSystemManagementService()[0] - mock_svc.ModifyResourceSettings.return_value = (self._FAKE_JOB_PATH, - mock.MagicMock(), - self._FAKE_RET_VAL) - mock_res_setting_data = mock.MagicMock() - mock_res_setting_data.GetText_.return_value = self._FAKE_RES_DATA - - self._utils._check_job_status = mock.MagicMock() - - self._utils._modify_virt_resource(mock_res_setting_data) - - mock_svc.ModifyResourceSettings.assert_called_with( - ResourceSettings=[self._FAKE_RES_DATA]) + self._test_virt_method('ModifyResourceSettings', 3, + '_modify_virt_resource', False, + ResourceSettings=[self._FAKE_RES_DATA]) def test_remove_virt_resource(self): + self._test_virt_method('RemoveResourceSettings', 2, + '_remove_virt_resource', False, + ResourceSettings=[self._FAKE_RES_PATH]) + + def test_remove_virt_feature(self): + self._test_virt_method('RemoveFeatureSettings', 2, + '_remove_virt_feature', False, + FeatureSettings=[self._FAKE_RES_PATH]) + + def _test_virt_method(self, vsms_method_name, return_count, + utils_method_name, with_mock_vm, *args, **kwargs): mock_svc = self._utils._conn.Msvm_VirtualSystemManagementService()[0] - mock_svc.RemoveResourceSettings.return_value = (self._FAKE_JOB_PATH, - self._FAKE_RET_VAL) + vsms_method = getattr(mock_svc, vsms_method_name) + mock_rsd = self._mock_vsms_method(vsms_method, return_count) + if with_mock_vm: + mock_vm = mock.MagicMock() + mock_vm.path_.return_value = self._FAKE_VM_PATH + getattr(self._utils, utils_method_name)(mock_vm, mock_rsd) + else: + getattr(self._utils, utils_method_name)(mock_rsd) + + if args: + vsms_method.assert_called_once_with(*args) + else: + vsms_method.assert_called_once_with(**kwargs) + + def _mock_vsms_method(self, vsms_method, return_count): + args = None + if return_count == 3: + args = (self._FAKE_JOB_PATH, mock.MagicMock(), self._FAKE_RET_VAL) + else: + args = (self._FAKE_JOB_PATH, self._FAKE_RET_VAL) + + vsms_method.return_value = args mock_res_setting_data = mock.MagicMock() + mock_res_setting_data.GetText_.return_value = self._FAKE_RES_DATA mock_res_setting_data.path_.return_value = self._FAKE_RES_PATH self._utils._check_job_status = mock.MagicMock() - self._utils._remove_virt_resource(mock_res_setting_data) - - mock_svc.RemoveResourceSettings.assert_called_with( - ResourceSettings=[self._FAKE_RES_PATH]) - - @mock.patch('neutron.plugins.hyperv.agent.utilsv2.HyperVUtilsV2' - '._check_job_status') - def test_remove_virt_feature(self, mock_check_job_status): - mock_svc = self._utils._conn.Msvm_VirtualSystemManagementService()[0] - mock_svc.RemoveFeatureSettings.return_value = (self._FAKE_JOB_PATH, - self._FAKE_RET_VAL) - mock_res_setting_data = mock.MagicMock() - mock_res_setting_data.path_.return_value = self._FAKE_RES_PATH - - self._utils._remove_virt_feature(mock_res_setting_data) - - mock_svc.RemoveFeatureSettings.assert_called_with( - FeatureSettings=[self._FAKE_RES_PATH]) + return mock_res_setting_data def test_disconnect_switch_port_delete_port(self): self._test_disconnect_switch_port(True)