openstack/rally
Code Issues Proposed changes

Don't hard set keystone endpoint

Browse Source 
Keystone client utilizes the auth_url provided to it in order to
discover the appropriate end point for the action. It will either use
the publicURL or the adminURL it gets in the service catalog for the
identity service. UNLESS caller to the keystone client hard sets the
endpoint key, in which case keystone will blindly use it instead of
looking in the catalog. Because rally was setting it, rally also had to
set it differently when doing admin level stuff, but rally doesn't have
to do that at all. Rally can rely on the service catalog and let
keystone sort it out.

This change removes manually setting endpoint and just passes along the
auth_url to keystone. This obviates the need to define an admin_port in
a deployment configuration, and reduces the number of tests that need to
be ran.

A warning will be issued if a deployment defines an admin_port, however
the admin_port itself will be ignored. This provides some backwards
compatibility with existing deployments.

Change-Id: I917412e954e57ca5f03df0bb739b7806d38a12a5
Related-Bug: 1398375
This commit is contained in:
Jesse Keating 2014-12-29 13:16:07 -08:00
parent c60c653a3e
commit 8e8dcf9360
12 changed files with 12 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
doc/samples/deployments/existing-keystone-v3.json
View File

@ -3,7 +3,6 @@
"auth_url": "http://example.net:5000/v3/",
"region_name": "RegionOne",
"endpoint_type": "public",
"admin_port": 35357,
"admin": {
"username": "admin",
"password": "myadminpass",

1
doc/samples/deployments/existing.json
View File

@ -3,7 +3,6 @@
"auth_url": "http://example.net:5000/v2.0/",
"region_name": "RegionOne",
"endpoint_type": "public",
"admin_port": 35357,
"admin": {
"username": "admin",
"password": "myadminpass",

1
doc/source/deploy_engines.rst
View File

@ -74,7 +74,6 @@ Or using keystone v3 API endpoint:
"type": "ExistingCloud",
"auth_url": "http://localhost:5000/v3/",
"endpoint_type": "public",
"admin_port": 35357,
"admin": {
"username": "engineer1",
"user_domain_name": "qa",

2
rally/cmd/commands/deployment.py
View File

@ -210,7 +210,7 @@ class DeploymentCommands(object):
"""
headers = ['auth_url', 'username', 'password', 'tenant_name',
'region_name', 'endpoint_type', 'admin_port']
'region_name', 'endpoint_type']
table_rows = []
deployment = db.deployment_get(deployment)

8
rally/deploy/engines/existing.py
View File

@ -28,7 +28,6 @@ class ExistingCloud(engine.EngineFactory):
"auth_url": "http://localhost:5000/v2.0/",
"region_name": "RegionOne",
"endpoint_type": "public",
"admin_port": 35357,
"admin": {
"username": "admin",
"password": "password",
@ -43,7 +42,6 @@ class ExistingCloud(engine.EngineFactory):
"auth_url": "http://localhost:5000/v3/",
"region_name": "RegionOne",
"endpoint_type": "public",
"admin_port": 35357,
"admin": {
"username": "admin",
"password": "admin",
@ -93,11 +91,6 @@ class ExistingCloud(engine.EngineFactory):
"enum": [consts.EndpointType.ADMIN,
consts.EndpointType.INTERNAL,
consts.EndpointType.PUBLIC]},
"admin_port": {
"type": "integer",
"minimum": 2,
"maximum": 65535
}
},
"anyOf": [
{
@ -124,7 +117,6 @@ class ExistingCloud(engine.EngineFactory):
region_name=common.get("region_name"),
endpoint_type=common.get("endpoint_type",
consts.EndpointType.PUBLIC),
admin_port=common.get("admin_port", 35357),
domain_name=user.get("domain_name"),
user_domain_name=user.get("user_domain_name", "Default"),
project_domain_name=user.get("project_domain_name", "Default")

11
rally/objects/endpoint.py
View File

@ -21,9 +21,8 @@ class Endpoint(object):
def __init__(self, auth_url, username, password, tenant_name=None,
permission=consts.EndpointPermission.USER,
region_name=None, endpoint_type=consts.EndpointType.PUBLIC,
admin_port=35357, domain_name=None,
user_domain_name='Default',
project_domain_name='Default'):
admin_port=None, domain_name=None,
user_domain_name='Default', project_domain_name='Default'):
self.auth_url = auth_url
self.username = username
self.password = password
@ -31,17 +30,19 @@ class Endpoint(object):
self.permission = permission
self.region_name = region_name
self.endpoint_type = endpoint_type
self.admin_port = admin_port
self.domain_name = domain_name
self.user_domain_name = user_domain_name
self.project_domain_name = project_domain_name
if admin_port:
import warnings
warnings.warn("'admin_port' argument is deprecated and will "
"be ignored.")
def to_dict(self, include_permission=False):
dct = {"auth_url": self.auth_url, "username": self.username,
"password": self.password, "tenant_name": self.tenant_name,
"region_name": self.region_name,
"endpoint_type": self.endpoint_type,
"admin_port": self.admin_port,
"domain_name": self.domain_name,
"user_domain_name": self.user_domain_name,
"project_domain_name": self.project_domain_name}

13
rally/osclients.py
View File

@ -27,7 +27,6 @@ from neutronclient.neutron import client as neutron
from novaclient import client as nova
from oslo.config import cfg
from saharaclient import client as sahara
from six.moves.urllib import parse
from troveclient import client as trove
from zaqarclient.queues import client as zaqar
@ -99,18 +98,6 @@ class Clients(object):
}
kw = self.endpoint.to_dict()
kw.update(new_kw)
if kw["endpoint_type"] == consts.EndpointType.PUBLIC:
mgmt_url = parse.urlparse(kw["auth_url"])
if (mgmt_url.port != kw["admin_port"] and
mgmt_url.scheme != "https"):
kw["endpoint"] = "{0}://{1}:{2}{3}".format(
mgmt_url.scheme,
mgmt_url.hostname,
kw["admin_port"],
mgmt_url.path
)
else:
kw["endpoint"] = kw["auth_url"]
client = create_keystone_client(kw)
if client.auth_ref is None:
client.authenticate()

8
tests/unit/cmd/commands/test_deployment.py
View File

@ -183,8 +183,7 @@ class DeploymentCommandsTestCase(test.TestCase):
"password": "p",
"tenant_name": "t",
"region_name": "r",
"endpoint_type": consts.EndpointType.INTERNAL,
"admin_port": "ap"
"endpoint_type": consts.EndpointType.INTERNAL
},
"users": []
}
@ -193,9 +192,8 @@ class DeploymentCommandsTestCase(test.TestCase):
mock_deployment.assert_called_once_with(deployment_id)
headers = ["auth_url", "username", "password", "tenant_name",
"region_name", "endpoint_type", "admin_port"]
fake_data = ["url", "u", "p", "t", "r", consts.EndpointType.INTERNAL,
"ap"]
"region_name", "endpoint_type"]
fake_data = ["url", "u", "p", "t", "r", consts.EndpointType.INTERNAL]
mock_struct.assert_called_once_with(**dict(zip(headers, fake_data)))
mock_print_list.assert_called_once_with([mock_struct()], headers)

1
tests/unit/deploy/engines/test_existing.py
View File

@ -32,7 +32,6 @@ class TestExistingCloud(test.TestCase):
"auth_url": "http://example.net:5000/v2.0/",
"region_name": "RegionOne",
"endpoint_type": consts.EndpointType.INTERNAL,
"admin_port": 35357,
"admin": {
"username": "admin",
"password": "myadminpass",

3
tests/unit/objects/test_endpoint.py
View File

@ -29,5 +29,4 @@ class EndpointTestCase(test.TestCase):
"domain_name": None,
"endpoint_type": consts.EndpointType.PUBLIC,
"project_domain_name": "Default",
"user_domain_name": "Default",
'admin_port': 35357})
"user_domain_name": "Default"})

1
tests/unit/test_api.py
View File

@ -40,7 +40,6 @@ FAKE_DEPLOY_CONFIG = {
},
"region_name": "RegionOne",
"endpoint_type": consts.EndpointType.INTERNAL,
"admin_port": 35357
}

25
tests/unit/test_osclients.py
View File

@ -17,7 +17,6 @@
from keystoneclient import exceptions as keystone_exceptions
import mock
from oslo.config import cfg
from six.moves.urllib import parse
from rally import consts
from rally import exceptions
@ -33,10 +32,7 @@ class OSClientsTestCase(test.TestCase):
super(OSClientsTestCase, self).setUp()
self.endpoint = objects.Endpoint("http://auth_url", "use", "pass",
"tenant")
self.endpoint_https = objects.Endpoint("https://auth_url/v2.0/admin",
"use", "pass", "tenant")
self.clients = osclients.Clients(self.endpoint)
self.clients_https = osclients.Clients(self.endpoint_https)
self.fake_keystone = fakes.FakeKeystoneClient()
self.fake_keystone.auth_token = mock.MagicMock()
@ -55,32 +51,13 @@ class OSClientsTestCase(test.TestCase):
self.assertNotIn("keystone", self.clients.cache)
client = self.clients.keystone()
self.assertEqual(client, self.fake_keystone)
mgmt_url = parse.urlparse(self.endpoint.auth_url)
auth_url = "{0}://{1}:{2}{3}".format(mgmt_url.scheme,
mgmt_url.hostname,
self.endpoint.admin_port,
mgmt_url.path)
endpoint = {"timeout": cfg.CONF.openstack_client_http_timeout,
"insecure": False, "cacert": None,
"endpoint": auth_url}
"insecure": False, "cacert": None}
kwargs = self.endpoint.to_dict()
kwargs.update(endpoint.items())
self.mock_create_keystone_client.assert_called_once_with(kwargs)
self.assertEqual(self.clients.cache["keystone"], self.fake_keystone)
def test_keystone_with_https_auth_url(self):
self.assertNotIn("keystone", self.clients_https.cache)
client = self.clients_https.keystone()
self.assertEqual(client, self.fake_keystone)
endpoint = {"timeout": cfg.CONF.openstack_client_http_timeout,
"insecure": False, "cacert": None,
"endpoint": self.endpoint_https.auth_url}
kwargs = self.endpoint_https.to_dict()
kwargs.update(endpoint.items())
self.mock_create_keystone_client.assert_called_once_with(kwargs)
self.assertEqual(self.clients_https.cache["keystone"],
self.fake_keystone)
@mock.patch("rally.osclients.Clients.keystone")
def test_verified_keystone_user_not_admin(self, mock_keystone):
mock_keystone.return_value = fakes.FakeKeystoneClient()