From ada57e6704faa89d97d9fe0fc4816aca552d5d54 Mon Sep 17 00:00:00 2001 From: Andrey Kurilin Date: Thu, 21 Sep 2017 17:09:23 +0300 Subject: [PATCH] [validators] Unify the order of arguments A lot of validators have arguments of validate method in a wrong order. It is not a failure, since the caller (ValidatablePluginMixin.validate) uses keyword arguments. Change-Id: I0a277925ced9775591c2698ffeffe4babb12624e --- rally/plugins/openstack/validators.py | 28 +++--- .../unit/plugins/openstack/test_validators.py | 99 ++++++++++--------- 2 files changed, 65 insertions(+), 62 deletions(-) diff --git a/rally/plugins/openstack/validators.py b/rally/plugins/openstack/validators.py index ccbf2091..3c8d83b7 100644 --- a/rally/plugins/openstack/validators.py +++ b/rally/plugins/openstack/validators.py @@ -48,7 +48,7 @@ class ImageExistsValidator(validation.Validator): self.param_name = param_name self.nullable = nullable - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): image_args = config.get("args", {}).get(self.param_name) @@ -91,7 +91,7 @@ class ExternalNetworkExistsValidator(validation.Validator): super(ExternalNetworkExistsValidator, self).__init__() self.param_name = param_name - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): ext_network = config.get("args", {}).get(self.param_name) if not ext_network: @@ -139,7 +139,7 @@ class RequiredNeutronExtensionsValidator(validation.Validator): self.req_ext = [extensions] self.req_ext.extend(args) - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): clients = credentials["openstack"]["users"][0]["credential"].clients() extensions = clients.neutron().list_extensions()["extensions"] aliases = [x["alias"] for x in extensions] @@ -190,12 +190,14 @@ class FlavorExistsValidator(validation.Validator): pass self.fail("Flavor '%s' not found" % flavor_value) - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): # flavors do not depend on user or tenant, so checking for one user # should be enough user = credentials["openstack"]["users"][0] clients = user["credential"].clients() - self._get_validated_flavor(config, clients, self.param_name) + self._get_validated_flavor(config=config, + clients=clients, + param_name=self.param_name) @validation.add("required_platform", platform="openstack", users=True) @@ -263,7 +265,7 @@ class ImageValidOnFlavorValidator(FlavorExistsValidator): except (glance_exc.HTTPNotFound, exceptions.InvalidScenarioArgument): self.fail("Image '%s' not found" % image_args) - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): flavor = None for user in credentials["openstack"]["users"]: @@ -333,7 +335,7 @@ class RequiredClientsValidator(validation.Validator): "Client for {0} is not installed. To install it run " "`pip install python-{0}client`".format(client_component)) - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): LOG.warning("The validator 'required_clients' is deprecated since " "Rally 0.10.0. If you are interested in it, please " "contact Rally team via E-mail, IRC or Gitter (see " @@ -374,7 +376,7 @@ class RequiredServicesValidator(validation.Validator): self.services = [services] self.services.extend(args) - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): creds = (credentials["openstack"].get("admin") or credentials["openstack"]["users"][0]["credential"]) @@ -423,7 +425,7 @@ class ValidateHeatTemplateValidator(validation.Validator): self.params = [params] self.params.extend(args) - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): for param_name in self.params: template_path = config.get("args", {}).get(param_name) @@ -462,7 +464,7 @@ class RequiredCinderServicesValidator(validation.Validator): super(RequiredCinderServicesValidator, self).__init__() self.services = services - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): clients = credentials["openstack"]["admin"].clients().cinder() for service in clients.services.list(): @@ -487,7 +489,7 @@ class RequiredAPIVersionsValidator(validation.Validator): self.component = component self.versions = versions - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): versions = [str(v) for v in self.versions] versions_str = ", ".join(versions) msg = ("Task was designed to be used with %(component)s " @@ -537,7 +539,7 @@ class VolumeTypeExistsValidator(validation.Validator): self.param = param_name self.nullable = nullable - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): volume_type = config.get("args", {}).get(self.param, False) if not volume_type and self.nullable: @@ -572,7 +574,7 @@ class WorkbookContainsWorkflowValidator(validators.FileExistsValidator): self.workbook = workbook_param self.workflow = workflow_param - def validate(self, config, credentials, plugin_cls, plugin_cfg): + def validate(self, credentials, config, plugin_cls, plugin_cfg): wf_name = config.get("args", {}).get(self.workflow) if wf_name: wb_path = config.get("args", {}).get(self.workbook) diff --git a/tests/unit/plugins/openstack/test_validators.py b/tests/unit/plugins/openstack/test_validators.py index bddc8efc..44a98262 100644 --- a/tests/unit/plugins/openstack/test_validators.py +++ b/tests/unit/plugins/openstack/test_validators.py @@ -80,7 +80,7 @@ class ImageExistsValidatorTestCase(test.TestCase): if err_msg: e = self.assertRaises( validators.validation.ValidationError, - validator.validate, self.config, self.credentials, None, None) + validator.validate, self.credentials, self.config, None, None) self.assertEqual(err_msg, e.message) else: result = validator.validate(self.config, self.credentials, None, @@ -93,7 +93,7 @@ class ImageExistsValidatorTestCase(test.TestCase): "images": { "image_name": "foo"}}} - self.validator.validate(config, self.credentials, None, None) + self.validator.validate(self.credentials, config, None, None) @mock.patch("%s.openstack_types.GlanceImage.transform" % PATH, return_value="image_id") @@ -107,7 +107,7 @@ class ImageExistsValidatorTestCase(test.TestCase): "openstack"]["users"][0].get.return_value.clients.return_value clients.glance().images.get = mock.Mock() - result = self.validator.validate(config, self.credentials, None, None) + result = self.validator.validate(self.credentials, config, None, None) self.assertIsNone(result) mock_glance_image_transform.assert_called_once_with( @@ -121,7 +121,7 @@ class ImageExistsValidatorTestCase(test.TestCase): e = self.assertRaises( validators.validation.ValidationError, - self.validator.validate, config, self.credentials, None, None) + self.validator.validate, self.credentials, config, None, None) self.assertEqual("Image 'fake_image' not found", e.message) @@ -163,13 +163,13 @@ class ExternalNetworkExistsValidatorTestCase(test.TestCase): if err_msg: e = self.assertRaises( validators.validation.ValidationError, - self.validator.validate, foo_conf, self.credentials, + self.validator.validate, self.credentials, foo_conf, None, None) self.assertEqual( err_msg.format(user["credential"].username, net1, net2), e.message) else: - result = self.validator.validate(foo_conf, self.credentials, + result = self.validator.validate(self.credentials, foo_conf, None, None) self.assertIsNone(result, "Unexpected result '%s'" % result) @@ -191,7 +191,7 @@ class RequiredNeutronExtensionsValidatorTestCase(test.TestCase): clients.neutron().list_extensions.return_value = { "extensions": [{"alias": "existing_extension"}]} - validator.validate({}, self.credentials, None, None) + validator.validate(self.credentials, {}, None, None) def test_validator_failed(self): err_msg = "Neutron extension absent_extension is not configured" @@ -205,7 +205,7 @@ class RequiredNeutronExtensionsValidatorTestCase(test.TestCase): e = self.assertRaises( validators.validation.ValidationError, - validator.validate, {}, self.credentials, None, None) + validator.validate, self.credentials, {}, None, None) self.assertEqual(err_msg, e.message) @@ -293,11 +293,12 @@ class FlavorExistsValidatorTestCase(test.TestCase): creds = mock.MagicMock() actual_e = self.assertRaises( validators.validation.ValidationError, - self.validator.validate, config, creds, None, None) + self.validator.validate, creds, config, None, None) self.assertEqual(expected_e, actual_e) self.validator._get_validated_flavor.assert_called_once_with( - config, creds["openstack"]["users"][0]["credential"].clients(), - self.validator.param_name) + config=config, + clients=creds["openstack"]["users"][0]["credential"].clients(), + param_name=self.validator.param_name) @ddt.ddt @@ -338,12 +339,12 @@ class ImageValidOnFlavorValidatorTestCase(test.TestCase): # case 1: no image, but it is ok, since fail_on_404_image is False validator._get_validated_image = mock.Mock( side_effect=validators.validation.ValidationError("!!!")) - validator.validate({}, self.credentials, None, None) + validator.validate(self.credentials, {}, None, None) # case 2: there is an image validator._get_validated_image = mock.Mock( return_value=fake_image) - validator.validate({}, self.credentials, None, None) + validator.validate(self.credentials, {}, None, None) # case 3: check caching of the flavor user = self.credentials["openstack"]["users"][0] @@ -351,7 +352,7 @@ class ImageValidOnFlavorValidatorTestCase(test.TestCase): validator._get_validated_image.reset_mock() validator._get_validated_flavor.reset_mock() - validator.validate({}, self.credentials, None, None) + validator.validate(self.credentials, {}, None, None) self.assertEqual(1, validator._get_validated_flavor.call_count) self.assertEqual(2, validator._get_validated_image.call_count) @@ -377,7 +378,7 @@ class ImageValidOnFlavorValidatorTestCase(test.TestCase): side_effect=expected_e) actual_e = self.assertRaises( validators.validation.ValidationError, - validator.validate, {}, self.credentials, None, None + validator.validate, self.credentials, {}, None, None ) self.assertEqual(expected_e, actual_e) @@ -386,7 +387,7 @@ class ImageValidOnFlavorValidatorTestCase(test.TestCase): validator._get_validated_flavor.side_effect = expected_e actual_e = self.assertRaises( KeyError, - validator.validate, {}, self.credentials, None, None + validator.validate, self.credentials, {}, None, None ) self.assertEqual(expected_e, actual_e) @@ -399,7 +400,7 @@ class ImageValidOnFlavorValidatorTestCase(test.TestCase): return_value=fake_image) e = self.assertRaises( validators.validation.ValidationError, - validator.validate, {}, self.credentials, None, None + validator.validate, self.credentials, {}, None, None ) self.assertEqual( "The memory size for flavor 'flavor_id' is too small for " @@ -413,7 +414,7 @@ class ImageValidOnFlavorValidatorTestCase(test.TestCase): return_value=fake_image) e = self.assertRaises( validators.validation.ValidationError, - validator.validate, {}, self.credentials, None, None + validator.validate, self.credentials, {}, None, None ) self.assertEqual( "The disk size for flavor 'flavor_id' is too small for " @@ -428,7 +429,7 @@ class ImageValidOnFlavorValidatorTestCase(test.TestCase): return_value=fake_image) e = self.assertRaises( validators.validation.ValidationError, - validator.validate, {}, self.credentials, None, None + validator.validate, self.credentials, {}, None, None ) self.assertEqual( "The minimal disk size for flavor 'flavor_id' is too small for " @@ -448,7 +449,7 @@ class ImageValidOnFlavorValidatorTestCase(test.TestCase): actual_e = self.assertRaises( KeyError, - validator.validate, {}, self.credentials, None, None + validator.validate, self.credentials, {}, None, None ) self.assertEqual(expected_e, actual_e) @@ -553,13 +554,13 @@ class RequiredClientsValidatorTestCase(test.TestCase): clients = self.credentials[ "openstack"]["users"][0]["credential"].clients.return_value - result = validator.validate(self.config, self.credentials, None, None) + result = validator.validate(self.credentials, self.config, None, None) self.assertIsNone(result) clients.nova.side_effect = ImportError e = self.assertRaises( validators.validation.ValidationError, - validator.validate, self.config, self.credentials, None, None) + validator.validate, self.credentials, self.config, None, None) self.assertEqual("Client for nova is not installed. To install it " "run `pip install python-novaclient`", e.message) @@ -569,13 +570,13 @@ class RequiredClientsValidatorTestCase(test.TestCase): admin=True) clients = self.credentials[ "openstack"]["admin"].clients.return_value - result = validator.validate(self.config, self.credentials, None, None) + result = validator.validate(self.credentials, self.config, None, None) self.assertIsNone(result) clients.keystone.side_effect = ImportError e = self.assertRaises( validators.validation.ValidationError, - validator.validate, self.config, self.credentials, None, None) + validator.validate, self.credentials, self.config, None, None) self.assertEqual("Client for keystone is not installed. To install it " "run `pip install python-keystoneclient`", e.message) @@ -603,19 +604,19 @@ class RequiredServicesValidatorTestCase(test.TestCase): consts.Service.NOVA_NET] fake_service = mock.Mock(binary="nova-network", status="enabled") clients.nova.services.list.return_value = [fake_service] - result = self.validator.validate(self.config, self.credentials, + result = self.validator.validate(self.credentials, self.config, None, None) self.assertIsNone(result) fake_service = mock.Mock(binary="keystone", status="enabled") clients.nova.services.list.return_value = [fake_service] - result = self.validator.validate(self.config, self.credentials, + result = self.validator.validate(self.credentials, self.config, None, None) self.assertIsNone(result) fake_service = mock.Mock(binary="nova-network", status="disabled") clients.nova.services.list.return_value = [fake_service] - result = self.validator.validate(self.config, self.credentials, + result = self.validator.validate(self.credentials, self.config, None, None) self.assertIsNone(result) @@ -635,7 +636,7 @@ class RequiredServicesValidatorTestCase(test.TestCase): e = self.assertRaises( validators.validation.ValidationError, - validator.validate, {}, self.credentials, None, None) + validator.validate, self.credentials, {}, None, None) expected_msg = ("'{0}' service is not available. Hint: If '{0}'" " service has non-default service_type, try to setup" " it via 'api_versions' context.").format("lol") @@ -674,7 +675,7 @@ class ValidateHeatTemplateValidatorTestCase(test.TestCase): context = {"args": {"template_path1": "fake_path1", "template_path2": "fake_path2"}} if not exception_msg: - result = self.validator.validate(context, self.credentials, None, + result = self.validator.validate(self.credentials, context, None, None) heat_validator.assert_has_calls([ @@ -689,7 +690,7 @@ class ValidateHeatTemplateValidatorTestCase(test.TestCase): else: e = self.assertRaises( validators.validation.ValidationError, - self.validator.validate, context, self.credentials, None, None) + self.validator.validate, self.credentials, context, None, None) heat_validator.assert_called_once_with( template="fake_template1") self.assertEqual( @@ -702,7 +703,7 @@ class ValidateHeatTemplateValidatorTestCase(test.TestCase): e = self.assertRaises( validators.validation.ValidationError, - validator.validate, self.config, self.credentials, None, None) + validator.validate, self.credentials, self.config, None, None) expected_msg = ("Path to heat template is not specified. Its needed " "for heat template validation. Please check the " @@ -716,7 +717,7 @@ class ValidateHeatTemplateValidatorTestCase(test.TestCase): "template_path2": "fake_path2"}} e = self.assertRaises( validators.validation.ValidationError, - self.validator.validate, context, self.credentials, None, None) + self.validator.validate, self.credentials, context, None, None) expected_msg = "No file found by the given path fake_path1" self.assertEqual(expected_msg, e.message) @@ -735,13 +736,13 @@ class RequiredCinderServicesValidatorTestCase(test.TestCase): fake_service = mock.Mock(binary="cinder_service", state="up") clients = self.credentials["openstack"]["admin"].clients() clients.cinder().services.list.return_value = [fake_service] - result = validator.validate(self.config, self.credentials, None, None) + result = validator.validate(self.credentials, self.config, None, None) self.assertIsNone(result) fake_service.state = "down" e = self.assertRaises( validators.validation.ValidationError, - validator.validate, self.config, self.credentials, None, None) + validator.validate, self.credentials, self.config, None, None) self.assertEqual("cinder_service service is not available", e.message) @@ -774,10 +775,10 @@ class RequiredAPIVersionsValidatorTestCase(test.TestCase): "credential"].clients() clients.keystone.return_value = self._get_keystone_v3_mock_client() - validator.validate(self.config, self.credentials, None, None) + validator.validate(self.credentials, self.config, None, None) clients.keystone.return_value = self._get_keystone_v2_mock_client() - validator.validate(self.config, self.credentials, None, None) + validator.validate(self.credentials, self.config, None, None) def test_validate_with_keystone_v2(self): validator = validators.RequiredAPIVersionsValidator("keystone", @@ -786,12 +787,12 @@ class RequiredAPIVersionsValidatorTestCase(test.TestCase): clients = self.credentials["openstack"]["users"][0][ "credential"].clients() clients.keystone.return_value = self._get_keystone_v2_mock_client() - validator.validate(self.config, self.credentials, None, None) + validator.validate(self.credentials, self.config, None, None) clients.keystone.return_value = self._get_keystone_v3_mock_client() e = self.assertRaises( validators.validation.ValidationError, - validator.validate, self.config, self.credentials, None, None) + validator.validate, self.credentials, self.config, None, None) self.assertEqual("Task was designed to be used with keystone V2.0, " "but V3 is selected.", e.message) @@ -802,12 +803,12 @@ class RequiredAPIVersionsValidatorTestCase(test.TestCase): clients = self.credentials["openstack"]["users"][0][ "credential"].clients() clients.keystone.return_value = self._get_keystone_v3_mock_client() - validator.validate(self.config, self.credentials, None, None) + validator.validate(self.credentials, self.config, None, None) clients.keystone.return_value = self._get_keystone_v2_mock_client() e = self.assertRaises( validators.validation.ValidationError, - validator.validate, self.config, self.credentials, None, None) + validator.validate, self.credentials, self.config, None, None) self.assertEqual("Task was designed to be used with keystone V3, " "but V2.0 is selected.", e.message) @@ -837,10 +838,10 @@ class RequiredAPIVersionsValidatorTestCase(test.TestCase): if err_msg: e = self.assertRaises( validators.validation.ValidationError, - validator.validate, config, self.credentials, None, None) + validator.validate, self.credentials, config, None, None) self.assertEqual(err_msg, e.message) else: - result = validator.validate(config, self.credentials, None, None) + result = validator.validate(self.credentials, config, None, None) self.assertIsNone(result) @ddt.unpack @@ -857,10 +858,10 @@ class RequiredAPIVersionsValidatorTestCase(test.TestCase): if err_msg: e = self.assertRaises( validators.validation.ValidationError, - validator.validate, config, self.credentials, None, None) + validator.validate, self.credentials, config, None, None) self.assertEqual(err_msg, e.message) else: - result = validator.validate(config, self.credentials, None, None) + result = validator.validate(self.credentials, config, None, None) self.assertIsNone(result) @@ -882,7 +883,7 @@ class VolumeTypeExistsValidatorTestCase(test.TestCase): clients.cinder().volume_types.list.return_value = [mock.MagicMock()] - result = validator.validate(self.config, self.credentials, None, None) + result = validator.validate(self.credentials, self.config, None, None) self.assertIsNone(result, "Unexpected result") def test_validator_without_ctx_failed(self): @@ -896,7 +897,7 @@ class VolumeTypeExistsValidatorTestCase(test.TestCase): e = self.assertRaises( validators.validation.ValidationError, - validator.validate, self.config, self.credentials, None, None) + validator.validate, self.credentials, self.config, None, None) self.assertEqual( "The parameter 'fake_param' is required and should not be empty.", e.message) @@ -907,7 +908,7 @@ class VolumeTypeExistsValidatorTestCase(test.TestCase): clients.cinder().volume_types.list.return_value = [] ctx = {"args": {"volume_type": "fake_type"}, "context": {"volume_types": ["fake_type"]}} - result = self.validator.validate(ctx, self.credentials, None, None) + result = self.validator.validate(self.credentials, ctx, None, None) self.assertIsNone(result) @@ -919,7 +920,7 @@ class VolumeTypeExistsValidatorTestCase(test.TestCase): "context": {"volume_types": ["fake_type_2"]}} e = self.assertRaises( validators.validation.ValidationError, - self.validator.validate, ctx, self.credentials, None, None) + self.validator.validate, self.credentials, ctx, None, None) err_msg = ("Specified volume type fake_type not found for user {}. " "List of available types: ['fake_type_2']") @@ -963,7 +964,7 @@ class WorkbookContainsWorkflowValidatorTestCase(test.TestCase): } } - result = validator.validate(context, None, None, None) + result = validator.validate(None, context, None, None) self.assertIsNone(result) self.assertEqual(1, mock_open.called)