openstack/tempest
Code Issues Proposed changes

Stop passing TestResources to Managers

Browse Source 
Dynamic credential providers generate TestResource objects, which
behave like Credentials but also include network resources.
We used to pass those directly into client managers, but that has
several issues: the interface of the client manager is confusing,
because it accepts two different type of objects in one parameter.
Having the client manager depending on the dynamic credential provider
is a problem because in turn that depends on the client manager,
which may lead to circular dependencies when trying to make those
stable interfaces for tempest.lib.
Finally TestResources seem to confused developers, since a number of tests
ignored the possibility of getting Credential fields from TestResources
directly, and went through the inner credentials atttribute.

I think the best approach for now is to pass auth.Credentials objects
only into the client manager. The client manager does not use the
network resources anyways.

We plan to refactor the link between credential provider and network
resources anyways, so it's better to have that outside of client
managers.

Change-Id: I71d4587bf0b640d05d0f78fb84364ce4d2d021f3
Partially-implements: bp client-manager-refactor
This commit is contained in:
Andrea Frittoli (andreaf) 2016-06-09 11:09:02 +01:00
parent 11e1e88660
commit 848c4a1488
11 changed files with 38 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
tempest/api/identity/v2/test_ec2_credentials.py
View File

@ -36,16 +36,16 @@ class EC2CredentialsTest(base.BaseIdentityV2Test):
def test_create_ec2_credentials(self):
"""Create user ec2 credentials."""
resp = self.non_admin_users_client.create_user_ec2_credentials(
self.creds.credentials.user_id,
tenant_id=self.creds.credentials.tenant_id)["credential"]
self.creds.user_id,
tenant_id=self.creds.tenant_id)["credential"]
access = resp['access']
self.addCleanup(
self.non_admin_users_client.delete_user_ec2_credentials,
self.creds.credentials.user_id, access)
self.creds.user_id, access)
self.assertNotEmpty(resp['access'])
self.assertNotEmpty(resp['secret'])
self.assertEqual(self.creds.credentials.user_id, resp['user_id'])
self.assertEqual(self.creds.credentials.tenant_id, resp['tenant_id'])
self.assertEqual(self.creds.user_id, resp['user_id'])
self.assertEqual(self.creds.tenant_id, resp['tenant_id'])
@test.idempotent_id('9e2ea42f-0a4f-468c-a768-51859ce492e0')
def test_list_ec2_credentials(self):
@ -54,24 +54,24 @@ class EC2CredentialsTest(base.BaseIdentityV2Test):
fetched_creds = []
# create first ec2 credentials
creds1 = self.non_admin_users_client.create_user_ec2_credentials(
self.creds.credentials.user_id,
tenant_id=self.creds.credentials.tenant_id)["credential"]
self.creds.user_id,
tenant_id=self.creds.tenant_id)["credential"]
created_creds.append(creds1['access'])
# create second ec2 credentials
creds2 = self.non_admin_users_client.create_user_ec2_credentials(
self.creds.credentials.user_id,
tenant_id=self.creds.credentials.tenant_id)["credential"]
self.creds.user_id,
tenant_id=self.creds.tenant_id)["credential"]
created_creds.append(creds2['access'])
# add credentials to be cleaned up
self.addCleanup(
self.non_admin_users_client.delete_user_ec2_credentials,
self.creds.credentials.user_id, creds1['access'])
self.creds.user_id, creds1['access'])
self.addCleanup(
self.non_admin_users_client.delete_user_ec2_credentials,
self.creds.credentials.user_id, creds2['access'])
self.creds.user_id, creds2['access'])
# get the list of user ec2 credentials
resp = self.non_admin_users_client.list_user_ec2_credentials(
self.creds.credentials.user_id)["credentials"]
self.creds.user_id)["credentials"]
fetched_creds = [cred['access'] for cred in resp]
# created credentials should be in a fetched list
missing = [cred for cred in created_creds
@ -84,14 +84,14 @@ class EC2CredentialsTest(base.BaseIdentityV2Test):
def test_show_ec2_credentials(self):
"""Get the definite user ec2 credentials."""
resp = self.non_admin_users_client.create_user_ec2_credentials(
self.creds.credentials.user_id,
tenant_id=self.creds.credentials.tenant_id)["credential"]
self.creds.user_id,
tenant_id=self.creds.tenant_id)["credential"]
self.addCleanup(
self.non_admin_users_client.delete_user_ec2_credentials,
self.creds.credentials.user_id, resp['access'])
self.creds.user_id, resp['access'])
ec2_creds = self.non_admin_users_client.show_user_ec2_credentials(
self.creds.credentials.user_id, resp['access']
self.creds.user_id, resp['access']
)["credential"]
for key in ['access', 'secret', 'user_id', 'tenant_id']:
self.assertEqual(ec2_creds[key], resp[key])
@ -100,13 +100,13 @@ class EC2CredentialsTest(base.BaseIdentityV2Test):
def test_delete_ec2_credentials(self):
"""Delete user ec2 credentials."""
resp = self.non_admin_users_client.create_user_ec2_credentials(
self.creds.credentials.user_id,
tenant_id=self.creds.credentials.tenant_id)["credential"]
self.creds.user_id,
tenant_id=self.creds.tenant_id)["credential"]
access = resp['access']
self.non_admin_users_client.delete_user_ec2_credentials(
self.creds.credentials.user_id, access)
self.creds.user_id, access)
self.assertRaises(
lib_exc.NotFound,
self.non_admin_users_client.show_user_ec2_credentials,
self.creds.credentials.user_id,
self.creds.user_id,
access)

2
tempest/api/identity/v2/test_tenants.py
View File

@ -24,7 +24,7 @@ class IdentityTenantsTest(base.BaseIdentityV2Test):
@test.idempotent_id('ecae2459-243d-4ba1-ad02-65f15dc82b78')
def test_list_tenants_returns_only_authorized_tenants(self):
alt_tenant_name = self.alt_manager.credentials.credentials.tenant_name
alt_tenant_name = self.alt_manager.credentials.tenant_name
resp = self.non_admin_tenants_client.list_tenants()
# check that user can see only that tenants that he presents in so user

4
tempest/api/identity/v2/test_tokens.py
View File

@ -43,8 +43,8 @@ class TokensTest(base.BaseIdentityV2Test):
self.assertGreater(expires_at, now)
self.assertEqual(body['token']['tenant']['id'],
creds.credentials.tenant_id)
creds.tenant_id)
self.assertEqual(body['token']['tenant']['name'],
tenant_name)
self.assertEqual(body['user']['id'], creds.credentials.user_id)
self.assertEqual(body['user']['id'], creds.user_id)

4
tempest/api/identity/v2/test_users.py
View File

@ -46,9 +46,9 @@ class IdentityUsersTest(base.BaseIdentityV2Test):
client.auth_provider.clear_auth()
client.auth_provider.set_auth()
old_pass = self.creds.credentials.password
old_pass = self.creds.password
new_pass = data_utils.rand_password()
user_id = self.creds.credentials.user_id
user_id = self.creds.user_id
# to change password back. important for allow_tenant_isolation = false
self.addCleanup(_restore_password, self.non_admin_users_client,
user_id, old_pass=old_pass, new_pass=new_pass)

2
tempest/api/identity/v3/test_projects.py
View File

@ -25,7 +25,7 @@ class IdentityV3ProjectsTest(base.BaseIdentityV3Test):
@test.idempotent_id('86128d46-e170-4644-866a-cc487f699e1d')
def test_list_projects_returns_only_authorized_projects(self):
alt_project_name =\
self.alt_manager.credentials.credentials.project_name
self.alt_manager.credentials.project_name
resp = self.non_admin_users_client.list_user_projects(
self.os.credentials.user_id)

4
tempest/api/identity/v3/test_users.py
View File

@ -46,9 +46,9 @@ class IdentityV3UsersTest(base.BaseIdentityV3Test):
client.auth_provider.clear_auth()
client.auth_provider.set_auth()
old_pass = self.creds.credentials.password
old_pass = self.creds.password
new_pass = data_utils.rand_password()
user_id = self.creds.credentials.user_id
user_id = self.creds.user_id
# to change password back. important for allow_tenant_isolation = false
self.addCleanup(_restore_password, self.non_admin_users_client,
user_id, old_pass=old_pass, new_pass=new_pass)

2
tempest/cmd/verify_tempest_config.py
View File

@ -384,7 +384,7 @@ def main(opts=None):
icreds = credentials.get_credentials_provider(
'verify_tempest_config', network_resources=net_resources)
try:
os = clients.Manager(icreds.get_primary_creds())
os = clients.Manager(icreds.get_primary_creds().credentials)
services = check_service_availability(os, update)
results = {}
for service in ['nova', 'cinder', 'neutron', 'swift']:

13
tempest/manager.py
View File

@ -13,7 +13,6 @@
# License for the specific language governing permissions and limitations
# under the License.
from tempest.common import cred_provider
from tempest import config
from tempest import exceptions
from tempest.lib import auth
@ -34,7 +33,7 @@ class Manager(object):
Credentials to be used within the various client classes managed by the
Manager object must be defined.
:param credentials: type Credentials or TestResources
:param credentials: An instance of `auth.Credentials`
:param scope: default scope for tokens produced by the auth provider
"""
self.credentials = credentials
@ -42,15 +41,9 @@ class Manager(object):
if not self.credentials.is_valid():
raise exceptions.InvalidCredentials()
self.auth_version = CONF.identity.auth_version
# Tenant isolation creates TestResources, but
# PreProvisionedCredentialProvider and some tests create Credentials
if isinstance(credentials, cred_provider.TestResources):
creds = self.credentials.credentials
else:
creds = self.credentials
# Creates an auth provider for the credentials
self.auth_provider = get_auth_provider(creds, pre_auth=True,
scope=scope)
self.auth_provider = get_auth_provider(
self.credentials, pre_auth=True, scope=scope)
def get_auth_provider_class(credentials):

3
tempest/scenario/utils.py
View File

@ -107,7 +107,8 @@ class InputScenarioUtils(object):
name='InputScenarioUtils',
identity_version=CONF.identity.auth_version,
network_resources=network_resources)
os = clients.Manager(self.cred_provider.get_primary_creds())
os = clients.Manager(
self.cred_provider.get_primary_creds().credentials)
self.compute_images_client = os.compute_images_client
self.flavors_client = os.flavors_client
self.image_pattern = CONF.input_scenario.image_regex

5
tempest/test.py
View File

@ -541,7 +541,8 @@ class BaseTestCase(testtools.testcase.WithAttributes,
else:
raise exceptions.InvalidCredentials(
"Invalid credentials type %s" % credential_type)
return cls.client_manager(credentials=creds, service=cls._service)
return cls.client_manager(credentials=creds.credentials,
service=cls._service)
@classmethod
def clear_credentials(cls):
@ -640,7 +641,7 @@ class BaseTestCase(testtools.testcase.WithAttributes,
credentials.is_admin_available(
identity_version=cls.get_identity_version())):
admin_creds = cred_provider.get_admin_creds()
admin_manager = clients.Manager(admin_creds)
admin_manager = clients.Manager(admin_creds.credentials)
networks_client = admin_manager.compute_networks_client
return fixed_network.get_tenant_network(
cred_provider, networks_client, CONF.compute.fixed_network_name)

2
tempest/tests/test_base_test.py
View File

@ -66,7 +66,7 @@ class TestBaseTestCase(base.TestCase):
test.BaseTestCase.get_tenant_network()
mock_man.assert_called_once_with(
mock_prov.get_admin_creds.return_value)
mock_prov.get_admin_creds.return_value.credentials)
mock_iaa.assert_called_once_with(
identity_version=mock_giv.return_value)
mock_gcp.assert_called_once_with()