From 3c6dab1571f91174437eaf8641ce87d38b9f9cc3 Mon Sep 17 00:00:00 2001 From: Andrey Kurilin Date: Sun, 24 Jul 2016 14:49:25 +0300 Subject: [PATCH] Fix ExistingCloud properties JSONSchema was written with mistakes and allows additional properties. It means that users were able to put what ever they want and expect that Rally will work. This patch fixes config schema and adds a migration to fix known "wrong" configurations. Another part of this patch fixes usage of very important property - endpoint_type. Different OpenStack clients have different names for endpoint_type variable. KeystoneClient uses "interface", but Rally transmits "endpoint_type" which is silently ignored by kc. Actually, endpoint_type is an optional parameter(all clients have default value for it) and should not behardcoded to "public" by default (keystoneclient fails in gates with such type), so this patch removes the default value of endpoint_type and transmits it to kc only when it is specified. Possibly, this patch can break some deployments, since behaviour of discovering endpoints will changed, but it should not affect most of Rally users. Co-Authored-By: Kevin Esensoy Co-Authored-By: Illia Khudoshyn Change-Id: Ifafff0ea8ccec7402e9882b983e5a55c1817c8cc --- rally/cli/envutils.py | 17 ++- .../54e844ebfbc3_update_deployment_configs.py | 98 ++++++++++++++ rally/common/objects/credential.py | 5 +- rally/deployment/engines/existing.py | 45 +++---- rally/osclients.py | 41 +++--- tests/unit/cli/commands/test_deployment.py | 36 ++++- tests/unit/cli/test_envutils.py | 32 ++++- tests/unit/common/db/test_migrations.py | 124 +++++++++++++++++- tests/unit/common/db/test_migrations_base.py | 12 +- .../{test_endpoint.py => test_credential.py} | 9 +- tests/unit/test_osclients.py | 60 +++++---- 11 files changed, 389 insertions(+), 90 deletions(-) create mode 100644 rally/common/db/sqlalchemy/migrations/versions/54e844ebfbc3_update_deployment_configs.py rename tests/unit/common/objects/{test_endpoint.py => test_credential.py} (93%) diff --git a/rally/cli/envutils.py b/rally/cli/envutils.py index 3fd5670b1e..ccae2648e8 100644 --- a/rally/cli/envutils.py +++ b/rally/cli/envutils.py @@ -96,10 +96,7 @@ def get_creds_from_env_vars(): "admin": { "username": os.environ["OS_USERNAME"], "password": os.environ["OS_PASSWORD"], - "tenant_name": get_project_name_from_env(), - "user_domain_name": os.environ.get("OS_USER_DOMAIN_NAME", ""), - "project_domain_name": os.environ.get("OS_PROJECT_DOMAIN_NAME", - ""), + "tenant_name": get_project_name_from_env() }, "endpoint_type": get_endpoint_type_from_env(), "endpoint": os.environ.get("OS_ENDPOINT"), @@ -109,6 +106,14 @@ def get_creds_from_env_vars(): os.environ.get("OS_INSECURE")) } + user_domain_name = os.environ.get("OS_USER_DOMAIN_NAME") + project_domain_name = os.environ.get("OS_PROJECT_DOMAIN_NAME") + if user_domain_name or project_domain_name: + # it is Keystone v3 and it has another config schem + creds["admin"]["project_name"] = creds["admin"].pop("tenant_name") + creds["admin"]["user_domain_name"] = user_domain_name or "" + creds["admin"]["project_domain_name"] = project_domain_name or "" + return creds @@ -125,8 +130,8 @@ def get_project_name_from_env(): def get_endpoint_type_from_env(): endpoint_type = os.environ.get("OS_ENDPOINT_TYPE", - os.environ.get("OS_INTERFACE", "public")) - if "URL" in endpoint_type: + os.environ.get("OS_INTERFACE")) + if endpoint_type and "URL" in endpoint_type: endpoint_type = endpoint_type.replace("URL", "") return endpoint_type diff --git a/rally/common/db/sqlalchemy/migrations/versions/54e844ebfbc3_update_deployment_configs.py b/rally/common/db/sqlalchemy/migrations/versions/54e844ebfbc3_update_deployment_configs.py new file mode 100644 index 0000000000..3b0f2db355 --- /dev/null +++ b/rally/common/db/sqlalchemy/migrations/versions/54e844ebfbc3_update_deployment_configs.py @@ -0,0 +1,98 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Update_deployment_configs + +Previously we had bad deployment config validation + +Revision ID: 54e844ebfbc3 +Revises: 3177d36ea270 +Create Date: 2016-07-24 14:53:39.323105 + +""" + +# revision identifiers, used by Alembic. +revision = "54e844ebfbc3" +down_revision = "3177d36ea270" +branch_labels = None +depends_on = None + +from alembic import op # noqa +import sqlalchemy as sa # noqa + +from rally.common.db.sqlalchemy import types as sa_types +from rally import exceptions + + +deployments_helper = sa.Table( + "deployments", + sa.MetaData(), + sa.Column("id", sa.Integer, primary_key=True, autoincrement=True), + sa.Column( + "config", + sa_types.MutableJSONEncodedDict, + default={}, + nullable=False, + ) +) + + +def _check_user_entry(user): + """Fixes wrong format of users.""" + if "tenant_name" in user: + keys = set(user.keys()) + if keys == {"username", "password", "tenant_name", + "project_domain_name", "user_domain_name"}: + if (user["user_domain_name"] == "" and + user["project_domain_name"] == ""): + # it is credentials of keystone v2 and they were created + # --fromenv + del user["user_domain_name"] + del user["project_domain_name"] + return True + else: + # it looks like keystone v3 credentials + user["project_name"] = user.pop("tenant_name") + return True + + +def upgrade(): + connection = op.get_bind() + for deployment in connection.execute(deployments_helper.select()): + conf = deployment.config + if conf["type"] != "ExistingCloud": + continue + + should_update = False + + if _check_user_entry(conf["admin"]): + should_update = True + if "users" in conf: + for user in conf["users"]: + if _check_user_entry(user): + should_update = True + + if conf.get("endpoint_type") == "public": + del conf["endpoint_type"] + should_update = True + + if should_update: + connection.execute( + deployments_helper.update().where( + deployments_helper.c.id == deployment.id).values( + config=conf)) + + +def downgrade(): + raise exceptions.DowngradeNotSupported() diff --git a/rally/common/objects/credential.py b/rally/common/objects/credential.py index b6cd61a372..5110337f4f 100644 --- a/rally/common/objects/credential.py +++ b/rally/common/objects/credential.py @@ -19,8 +19,9 @@ from rally import consts class Credential(object): def __init__(self, auth_url, username, password, tenant_name=None, + project_name=None, permission=consts.EndpointPermission.USER, - region_name=None, endpoint_type=consts.EndpointType.PUBLIC, + region_name=None, endpoint_type=None, domain_name=None, endpoint=None, user_domain_name=None, admin_domain_name="Default", project_domain_name=None, @@ -28,7 +29,7 @@ class Credential(object): self.auth_url = auth_url self.username = username self.password = password - self.tenant_name = tenant_name + self.tenant_name = tenant_name or project_name self.permission = permission self.region_name = region_name self.endpoint_type = endpoint_type diff --git a/rally/deployment/engines/existing.py b/rally/deployment/engines/existing.py index 203cb716bf..d2d71990af 100644 --- a/rally/deployment/engines/existing.py +++ b/rally/deployment/engines/existing.py @@ -67,28 +67,31 @@ class ExistingCloud(engine.Engine): "definitions": { "user": { "type": "object", - "properties": { - "username": {"type": "string"}, - "password": {"type": "string"}, - }, "oneOf": [ { # v2.0 authentication "properties": { + "username": {"type": "string"}, + "password": {"type": "string"}, "tenant_name": {"type": "string"}, }, "required": ["username", "password", "tenant_name"], + "additionalProperties": False }, { # Authentication in project scope "properties": { + "username": {"type": "string"}, + "password": {"type": "string"}, "user_domain_name": {"type": "string"}, + "admin_domain_name": {"type": "string"}, "project_name": {"type": "string"}, "project_domain_name": {"type": "string"}, }, "required": ["username", "password", "project_name"], + "additionalProperties": False } - ] + ], } }, @@ -96,28 +99,21 @@ class ExistingCloud(engine.Engine): "type": {"type": "string"}, "auth_url": {"type": "string"}, "region_name": {"type": "string"}, - "endpoint_type": {"type": "string", - "enum": [consts.EndpointType.ADMIN, + "endpoint": {"type": ["string", "null"]}, + "endpoint_type": {"enum": [consts.EndpointType.ADMIN, consts.EndpointType.INTERNAL, - consts.EndpointType.PUBLIC]}, + consts.EndpointType.PUBLIC, + None]}, "https_insecure": {"type": "boolean"}, "https_cacert": {"type": "string"}, - }, - "anyOf": [ - { - "properties": { - "admin": {"$ref": "#/definitions/user"} - }, - "required": ["type", "auth_url", "admin"] - }, - { - "users": { - "type": "array", - "items": {"$ref": "#/definitions/user"} - }, - "required": ["type", "auth_url", "users"] + "admin": {"$ref": "#/definitions/user"}, + "users": { + "type": "array", + "items": {"$ref": "#/definitions/user"} } - ] + }, + "required": ["type", "auth_url", "admin"], + "additionalProperties": False } def _create_credential(self, common, user, permission): @@ -126,8 +122,7 @@ class ExistingCloud(engine.Engine): tenant_name=user.get("project_name", user.get("tenant_name")), permission=permission, region_name=common.get("region_name"), - endpoint_type=common.get("endpoint_type", - consts.EndpointType.PUBLIC), + endpoint_type=common.get("endpoint_type"), endpoint=common.get("endpoint"), domain_name=user.get("domain_name"), user_domain_name=user.get("user_domain_name", None), diff --git a/rally/osclients.py b/rally/osclients.py index 8459f056f2..367f0c1732 100644 --- a/rally/osclients.py +++ b/rally/osclients.py @@ -190,10 +190,11 @@ class OSClient(plugin.Plugin): def _get_endpoint(self, service_type=None): kc = self.keystone() - api_url = kc.service_catalog.url_for( - service_type=self.choose_service_type(service_type), - endpoint_type=self.credential.endpoint_type, - region_name=self.credential.region_name) + kw = {"service_type": self.choose_service_type(service_type), + "region_name": self.credential.region_name} + if self.credential.endpoint_type: + kw["endpoint_type"] = self.credential.endpoint_type + api_url = kc.service_catalog.url_for(**kw) return api_url def _get_auth_info(self, user_key="username", @@ -204,6 +205,7 @@ class OSClient(plugin.Plugin): user_domain_name_key="user_domain_name", project_domain_name_key="project_domain_name", cacert_key="cacert", + endpoint_type="endpoint_type", ): kw = { user_key: self.credential.username, @@ -223,7 +225,8 @@ class OSClient(plugin.Plugin): kw.update({ project_domain_name_key: self.credential.project_domain_name or "Default"}) - + if self.credential.endpoint_type: + kw[endpoint_type] = self.credential.endpoint_type return kw @abc.abstractmethod @@ -304,6 +307,8 @@ class Keystone(OSClient): # https://github.com/openstack/python-keystoneclient/commit/d9031c252848d89270a543b67109a46f9c505c86 from keystoneclient import base kw["auth_url"] = sess.get_endpoint(interface=base.AUTH_INTERFACE) + if self.credential.endpoint_type: + kw["endpoint_type"] = self.credential.endpoint_type ks = client.Client(**kw) ks.auth_ref = auth_ref return ks @@ -448,7 +453,8 @@ class Ceilometer(OSClient): token=auth_token, timeout=CONF.openstack_client_http_timeout, insecure=self.credential.insecure, - **self._get_auth_info(project_name_key="tenant_name")) + **self._get_auth_info(project_name_key="tenant_name", + endpoint_type="interface")) return client @@ -483,7 +489,10 @@ class Ironic(OSClient): ironic_url=self._get_endpoint(service_type), timeout=CONF.openstack_client_http_timeout, insecure=self.credential.insecure, - cacert=self.credential.cacert) + cacert=self.credential.cacert, + interface=self._get_auth_info().get( + "endpoint_type") + ) return client @@ -508,7 +517,6 @@ class Sahara(OSClient): client = sahara.Client( self.choose_version(version), service_type=self.choose_service_type(service_type), - endpoint_type=self.credential.endpoint_type, insecure=self.credential.insecure, **self._get_auth_info(password_key="api_key", project_name_key="project_name")) @@ -596,10 +604,10 @@ class Trove(OSClient): class Mistral(OSClient): def create_client(self, service_type=None): """Return Mistral client.""" - from mistralclient.api import client + from mistralclient.api import client as mistral kc = self.keystone() - client = client.client( + client = mistral.client( mistral_url=self._get_endpoint(service_type), service_type=self.choose_service_type(service_type), auth_token=kc.auth_token) @@ -690,7 +698,8 @@ class Senlin(OSClient): return senlin.Client( self.choose_version(version), **self._get_auth_info(project_name_key="project_name", - cacert_key="cert")) + cacert_key="cert", + endpoint_type="interface")) @configure("magnum", default_version="1", supported_versions=["1"], @@ -702,10 +711,11 @@ class Magnum(OSClient): api_url = self._get_endpoint(service_type) session = self._get_session()[0] - endpoint_type = self.credential.endpoint_type - return magnum.Client(session=session, interface=endpoint_type, - magnum_url=api_url) + return magnum.Client( + session=session, + interface=self.credential.endpoint_type, + magnum_url=api_url) @configure("watcher", default_version="1", default_service_type="infra-optim", @@ -723,7 +733,8 @@ class Watcher(OSClient): token=kc.auth_token, timeout=CONF.openstack_client_http_timeout, insecure=self.credential.insecure, - ca_file=self.credential.cacert) + ca_file=self.credential.cacert, + endpoint_type=self.credential.endpoint_type) return client diff --git a/tests/unit/cli/commands/test_deployment.py b/tests/unit/cli/commands/test_deployment.py index 1a88b33482..24fff83852 100644 --- a/tests/unit/cli/commands/test_deployment.py +++ b/tests/unit/cli/commands/test_deployment.py @@ -44,6 +44,38 @@ class DeploymentCommandsTestCase(test.TestCase): mock_deployment_create.assert_called_once_with( {"some": "json"}, "fake_deploy") + @mock.patch.dict(os.environ, {"OS_AUTH_URL": "fake_auth_url", + "OS_USERNAME": "fake_username", + "OS_PASSWORD": "fake_password", + "OS_TENANT_NAME": "fake_tenant_name", + "OS_REGION_NAME": "fake_region_name", + "OS_ENDPOINT_TYPE": "fake_endpoint_typeURL", + "OS_ENDPOINT": "fake_endpoint", + "OS_INSECURE": "True", + "OS_CACERT": "fake_cacert", + "RALLY_DEPLOYMENT": "fake_deployment_id"}) + @mock.patch("rally.cli.commands.deployment.api.Deployment.create") + @mock.patch("rally.cli.commands.deployment.DeploymentCommands.list") + def test_createfromenv_keystonev2(self, mock_list, mock_deployment_create): + self.deployment.create("from_env", True) + mock_deployment_create.assert_called_once_with( + { + "type": "ExistingCloud", + "auth_url": "fake_auth_url", + "region_name": "fake_region_name", + "endpoint_type": "fake_endpoint_type", + "endpoint": "fake_endpoint", + "admin": { + "username": "fake_username", + "password": "fake_password", + "tenant_name": "fake_tenant_name" + }, + "https_insecure": True, + "https_cacert": "fake_cacert" + }, + "from_env" + ) + @mock.patch.dict(os.environ, {"OS_AUTH_URL": "fake_auth_url", "OS_USERNAME": "fake_username", "OS_PASSWORD": "fake_password", @@ -58,7 +90,7 @@ class DeploymentCommandsTestCase(test.TestCase): "RALLY_DEPLOYMENT": "fake_deployment_id"}) @mock.patch("rally.cli.commands.deployment.api.Deployment.create") @mock.patch("rally.cli.commands.deployment.DeploymentCommands.list") - def test_createfromenv(self, mock_list, mock_deployment_create): + def test_createfromenv_keystonev3(self, mock_list, mock_deployment_create): self.deployment.create("from_env", True) mock_deployment_create.assert_called_once_with( { @@ -72,7 +104,7 @@ class DeploymentCommandsTestCase(test.TestCase): "password": "fake_password", "user_domain_name": "fake_udn", "project_domain_name": "fake_pdn", - "tenant_name": "fake_tenant_name" + "project_name": "fake_tenant_name" }, "https_insecure": True, "https_cacert": "fake_cacert" diff --git a/tests/unit/cli/test_envutils.py b/tests/unit/cli/test_envutils.py index 955ec31253..3f9cf41d37 100644 --- a/tests/unit/cli/test_envutils.py +++ b/tests/unit/cli/test_envutils.py @@ -104,6 +104,32 @@ class EnvUtilsTestCase(test.TestCase): envutils.clear_env() self.assertEqual(os.environ, {}) + @mock.patch.dict(os.environ, {"OS_AUTH_URL": "fake_auth_url", + "OS_USERNAME": "fake_username", + "OS_PASSWORD": "fake_password", + "OS_TENANT_NAME": "fake_tenant_name", + "OS_REGION_NAME": "fake_region_name", + "OS_ENDPOINT_TYPE": "fake_endpoint_typeURL", + "OS_ENDPOINT": "fake_endpoint", + "OS_INSECURE": "True", + "OS_CACERT": "fake_cacert"}) + def test_get_creds_from_env_vars_keystone_v2(self): + expected_creds = { + "auth_url": "fake_auth_url", + "admin": { + "username": "fake_username", + "password": "fake_password", + "tenant_name": "fake_tenant_name" + }, + "endpoint_type": "fake_endpoint_type", + "endpoint": "fake_endpoint", + "region_name": "fake_region_name", + "https_cacert": "fake_cacert", + "https_insecure": True + } + creds = envutils.get_creds_from_env_vars() + self.assertEqual(expected_creds, creds) + @mock.patch.dict(os.environ, {"OS_AUTH_URL": "fake_auth_url", "OS_USERNAME": "fake_username", "OS_PASSWORD": "fake_password", @@ -115,7 +141,7 @@ class EnvUtilsTestCase(test.TestCase): "OS_PROJECT_DOMAIN_NAME": "fake_pdn", "OS_USER_DOMAIN_NAME": "fake_udn", "OS_CACERT": "fake_cacert"}) - def test_get_creds_from_env_vars(self): + def test_get_creds_from_env_vars_keystone_v3(self): expected_creds = { "auth_url": "fake_auth_url", "admin": { @@ -123,7 +149,7 @@ class EnvUtilsTestCase(test.TestCase): "password": "fake_password", "user_domain_name": "fake_udn", "project_domain_name": "fake_pdn", - "tenant_name": "fake_tenant_name" + "project_name": "fake_tenant_name" }, "endpoint_type": "fake_endpoint_type", "endpoint": "fake_endpoint", @@ -190,4 +216,4 @@ class EnvUtilsTestCase(test.TestCase): @mock.patch.dict(os.environ, values={}, clear=True) def test_get_endpoint_type_from_env_when_neither(self): endpoint_type = envutils.get_endpoint_type_from_env() - self.assertEqual("public", endpoint_type) + self.assertIsNone(endpoint_type) diff --git a/tests/unit/common/db/test_migrations.py b/tests/unit/common/db/test_migrations.py index 848c9e776a..c4b54f6735 100644 --- a/tests/unit/common/db/test_migrations.py +++ b/tests/unit/common/db/test_migrations.py @@ -15,7 +15,7 @@ """Tests for DB migration.""" - +import json import pprint import alembic @@ -29,6 +29,8 @@ import rally from rally.common import db from rally.common.db.sqlalchemy import api from rally.common.db.sqlalchemy import models +from rally import consts +from rally.deployment.engines import existing from tests.unit.common.db import test_migrations_base from tests.unit import test as rtest @@ -261,3 +263,123 @@ class MigrationWalkTestCase(rtest.DBTestCase, self.assertColumnExists(engine, "deployments", "credentials") self.assertColumnNotExists(engine, "deployments", "admin") self.assertColumnNotExists(engine, "deployments", "users") + + def _pre_upgrade_54e844ebfbc3(self, engine): + self._54e844ebfbc3_deployments = { + # right config which should not be changed after migration + "should-not-be-changed-1": { + "admin": {"username": "admin", + "password": "passwd", + "project_name": "admin"}, + "auth_url": "http://example.com:5000/v3", + "region_name": "RegionOne", + "type": "ExistingCloud"}, + # right config which should not be changed after migration + "should-not-be-changed-2": { + "admin": {"username": "admin", + "password": "passwd", + "tenant_name": "admin"}, + "users": [{"username": "admin", + "password": "passwd", + "tenant_name": "admin"}], + "auth_url": "http://example.com:5000/v2.0", + "region_name": "RegionOne", + "type": "ExistingCloud"}, + # not ExistingCloud config which should not be changed + "should-not-be-changed-3": { + "url": "example.com", + "type": "Something"}, + # normal config created with "fromenv" feature + "from-env": { + "admin": {"username": "admin", + "password": "passwd", + "tenant_name": "admin", + "project_domain_name": "", + "user_domain_name": ""}, + "auth_url": "http://example.com:5000/v2.0", + "region_name": "RegionOne", + "type": "ExistingCloud"}, + # public endpoint + keystone v3 config with tenant_name + "ksv3_public": { + "admin": {"username": "admin", + "password": "passwd", + "tenant_name": "admin", + "user_domain_name": "bla", + "project_domain_name": "foo"}, + "auth_url": "http://example.com:5000/v3", + "region_name": "RegionOne", + "type": "ExistingCloud", + "endpoint_type": "public"}, + # internal endpoint + existing_users + "existing_internal": { + "admin": {"username": "admin", + "password": "passwd", + "tenant_name": "admin"}, + "users": [{"username": "admin", + "password": "passwd", + "tenant_name": "admin", + "project_domain_name": "", + "user_domain_name": ""}], + "auth_url": "http://example.com:5000/v2.0", + "region_name": "RegionOne", + "type": "ExistingCloud", + "endpoint_type": "internal"} + } + deployment_table = db_utils.get_table(engine, "deployments") + + deployment_status = consts.DeployStatus.DEPLOY_FINISHED + with engine.connect() as conn: + for deployment in self._54e844ebfbc3_deployments: + conf = json.dumps(self._54e844ebfbc3_deployments[deployment]) + conn.execute( + deployment_table.insert(), + [{"uuid": deployment, "name": deployment, + "config": conf, + "enum_deployments_status": deployment_status, + "credentials": six.b(json.dumps([])), + "users": six.b(json.dumps([])) + }]) + + def _check_54e844ebfbc3(self, engine, data): + self.assertEqual("54e844ebfbc3", + api.get_backend().schema_revision(engine=engine)) + + original_deployments = self._54e844ebfbc3_deployments + + deployment_table = db_utils.get_table(engine, "deployments") + + with engine.connect() as conn: + deployments_found = conn.execute( + deployment_table.select()).fetchall() + for deployment in deployments_found: + # check deployment + self.assertIn(deployment.uuid, original_deployments) + self.assertIn(deployment.name, original_deployments) + + config = json.loads(deployment.config) + if config != original_deployments[deployment.uuid]: + if deployment.uuid.startswith("should-not-be-changed"): + self.fail("Config of deployment '%s' is changes, but " + "should not." % deployment.uuid) + + endpoint_type = (original_deployments[ + deployment.uuid].get("endpoint_type")) + if endpoint_type in (None, "public"): + self.assertNotIn("endpoint_type", config) + else: + self.assertIn("endpoint_type", config) + self.assertEqual(endpoint_type, + config["endpoint_type"]) + + existing.ExistingCloud({"config": config}).validate() + else: + if not deployment.uuid.startswith("should-not-be-changed"): + self.fail("Config of deployment '%s' is not changes, " + "but should." % deployment.uuid) + + # this deployment created at _pre_upgrade step is not needed + # anymore and we can remove it + conn.execute( + deployment_table.delete().where( + deployment_table.c.uuid == deployment.uuid) + ) diff --git a/tests/unit/common/db/test_migrations_base.py b/tests/unit/common/db/test_migrations_base.py index 873da7b261..9c25b09e3e 100644 --- a/tests/unit/common/db/test_migrations_base.py +++ b/tests/unit/common/db/test_migrations_base.py @@ -30,7 +30,7 @@ from alembic import migration from alembic import script as alembic_script from oslo_config import cfg -import rally.common.db.sqlalchemy.api +from rally.common.db.sqlalchemy import api as s_api from rally.common.i18n import _LE from rally.common import logging @@ -41,8 +41,7 @@ CONF = cfg.CONF class BaseWalkMigrationMixin(object): ALEMBIC_CONFIG = alembic_config.Config( - os.path.join(os.path.dirname(rally.common.db.sqlalchemy.api.__file__), - "alembic.ini") + os.path.join(os.path.dirname(s_api.__file__), "alembic.ini") ) ALEMBIC_CONFIG.rally_config = CONF @@ -84,8 +83,7 @@ class BaseWalkMigrationMixin(object): env = alembic_script.ScriptDirectory.from_config(self.ALEMBIC_CONFIG) versions = [] for rev in env.walk_revisions(): - if (rev.revision == - rally.common.db.sqlalchemy.api.INITIAL_REVISION_UUID): + if rev.revision == s_api.INITIAL_REVISION_UUID: # NOTE(rpromyshlennikov): we skip initial migration here continue versions.append((rev.revision, rev.down_revision or "-1")) @@ -103,6 +101,10 @@ class BaseWalkMigrationMixin(object): """ self._configure(engine) + # NOTE(ikhudoshyn): Now DB contains certain schema + # so we can not execute all migrations starting from + # init. So we cleanup the DB. + s_api.get_backend().schema_cleanup() up_and_down_versions = self._up_and_down_versions() for ver_up, ver_down in up_and_down_versions: self._migrate_up(engine, ver_up, with_data=True) diff --git a/tests/unit/common/objects/test_endpoint.py b/tests/unit/common/objects/test_credential.py similarity index 93% rename from tests/unit/common/objects/test_endpoint.py rename to tests/unit/common/objects/test_credential.py index 4e34f7f5ca..b51babe592 100644 --- a/tests/unit/common/objects/test_endpoint.py +++ b/tests/unit/common/objects/test_credential.py @@ -18,7 +18,7 @@ from rally import consts from tests.unit import test -class EndpointTestCase(test.TestCase): +class CredentialTestCase(test.TestCase): def test_to_dict(self): credential = objects.Credential( @@ -33,7 +33,7 @@ class EndpointTestCase(test.TestCase): "region_name": None, "domain_name": None, "endpoint": None, - "endpoint_type": consts.EndpointType.PUBLIC, + "endpoint_type": None, "https_insecure": False, "https_cacert": None, "project_domain_name": None, @@ -54,7 +54,7 @@ class EndpointTestCase(test.TestCase): "domain_name": None, "endpoint": None, "permission": consts.EndpointPermission.ADMIN, - "endpoint_type": consts.EndpointType.PUBLIC, + "endpoint_type": None, "https_insecure": False, "https_cacert": None, "project_domain_name": None, @@ -66,7 +66,8 @@ class EndpointTestCase(test.TestCase): "foo_url", "foo_user", "foo_password", tenant_name="foo_tenant", permission=consts.EndpointPermission.ADMIN, - endpoint="foo_endpoint") + endpoint="foo_endpoint", + endpoint_type=consts.EndpointType.PUBLIC) self.assertEqual(credential.to_dict(), {"auth_url": "foo_url", "username": "foo_user", diff --git a/tests/unit/test_osclients.py b/tests/unit/test_osclients.py index 1ece38fe3f..dc099abbf9 100644 --- a/tests/unit/test_osclients.py +++ b/tests/unit/test_osclients.py @@ -72,11 +72,6 @@ class OSClientTestCaseUtils(object): @ddt.ddt class OSClientTestCase(test.TestCase, OSClientTestCaseUtils): - def setUp(self): - super(OSClientTestCase, self).setUp() - self.credential = objects.Credential("http://auth_url/v2.0", "user", - "pass", "tenant") - def test_choose_service_type(self): default_service_type = "default_service_type" @@ -99,10 +94,11 @@ class OSClientTestCase(test.TestCase, OSClientTestCaseUtils): ) @ddt.unpack def test__get_session(self, auth_url): + credential = objects.Credential("http://auth_url/v2.0", "user", + "pass", "tenant") mock_keystoneauth1 = mock.MagicMock() self.set_up_keystone_mocks() - osclient = osclients.OSClient( - self.credential, {}, mock.MagicMock()) + osclient = osclients.OSClient(credential, {}, mock.MagicMock()) with mock.patch.dict( "sys.modules", { "keystoneauth1": mock_keystoneauth1}): @@ -130,6 +126,30 @@ class OSClientTestCase(test.TestCase, OSClientTestCaseUtils): mock.call(auth=self.ksc_identity_plugin, timeout=180.0, verify=True)]) + @ddt.data( + {"endpoint_type": None, "service_type": None, "region_name": None}, + {"endpoint_type": "et", "service_type": "st", "region_name": "rn"} + ) + @ddt.unpack + def test__get_endpoint(self, endpoint_type, service_type, region_name): + credential = objects.Credential("http://auth_url/v2.0", "user", "pass", + endpoint_type=endpoint_type, + region_name=region_name) + mock_choose_service_type = mock.MagicMock() + osclient = osclients.OSClient(credential, {}, mock.MagicMock()) + osclient.choose_service_type = mock_choose_service_type + with mock.patch.object(osclient, "keystone") as ks: + mock_url_for = ks.return_value.service_catalog.url_for + self.assertEqual(mock_url_for.return_value, + osclient._get_endpoint(service_type)) + call_args = { + "service_type": mock_choose_service_type.return_value, + "region_name": region_name} + if endpoint_type: + call_args["endpoint_type"] = endpoint_type + mock_url_for.assert_called_once_with(**call_args) + mock_choose_service_type.assert_called_once_with(service_type) + class CachedTestCase(test.TestCase): @@ -305,7 +325,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_nova, client) self.service_catalog.url_for.assert_called_once_with( service_type="compute", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) mock_nova.client.Client.assert_called_once_with( "2", @@ -343,7 +362,6 @@ class OSClientsTestCase(test.TestCase): } self.service_catalog.url_for.assert_called_once_with( service_type="network", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) mock_neutron.client.Client.assert_called_once_with("2.0", **kw) self.assertEqual(fake_neutron, self.clients.cache["neutron"]) @@ -362,7 +380,6 @@ class OSClientsTestCase(test.TestCase): "insecure": False, "cacert": None} self.service_catalog.url_for.assert_called_once_with( service_type="image", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) mock_glance.Client.assert_called_once_with("1", **kw) self.assertEqual(fake_glance, self.clients.cache["glance"]) @@ -377,7 +394,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_cinder, client) self.service_catalog.url_for.assert_called_once_with( service_type="volumev2", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) mock_cinder.client.Client.assert_called_once_with( "2", @@ -402,7 +418,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(mock_manila.client.Client.return_value, client) self.service_catalog.url_for.assert_called_once_with( service_type="share", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) mock_manila.client.Client.assert_called_once_with( "1", @@ -436,7 +451,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_ceilometer, client) self.service_catalog.url_for.assert_called_once_with( service_type="metering", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) kw = {"os_endpoint": self.service_catalog.url_for.return_value, "token": self.fake_keystone.auth_token, @@ -485,7 +499,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_monasca, client) self.service_catalog.url_for.assert_called_once_with( service_type="monitoring", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) os_endpoint = self.service_catalog.url_for.return_value kw = {"token": self.fake_keystone.auth_token, @@ -513,14 +526,14 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_ironic, client) self.service_catalog.url_for.assert_called_once_with( service_type="baremetal", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) kw = { "os_auth_token": self.fake_keystone.auth_token, "ironic_url": self.service_catalog.url_for.return_value, "timeout": cfg.CONF.openstack_client_http_timeout, "insecure": self.credential.insecure, - "cacert": self.credential.cacert + "cacert": self.credential.cacert, + "interface": self.credential.endpoint_type } mock_ironic.client.get_client.assert_called_once_with("1", **kw) self.assertEqual(fake_ironic, self.clients.cache["ironic"]) @@ -535,7 +548,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_sahara, client) kw = { "service_type": "data-processing", - "endpoint_type": self.credential.endpoint_type, "insecure": False, "username": self.credential.username, "api_key": self.credential.password, @@ -558,7 +570,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_zaqar, client) self.service_catalog.url_for.assert_called_once_with( service_type="messaging", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) fake_zaqar_url = self.service_catalog.url_for.return_value conf = {"auth_opts": {"backend": "keystone", "options": { @@ -607,7 +618,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_mistral, client) self.service_catalog.url_for.assert_called_once_with( service_type="workflowv2", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name ) fake_mistral_url = self.service_catalog.url_for.return_value @@ -628,7 +638,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(client, fake_swift) self.service_catalog.url_for.assert_called_once_with( service_type="object-store", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) kw = {"retries": 1, "preauthurl": self.service_catalog.url_for.return_value, @@ -691,7 +700,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_murano, client) self.service_catalog.url_for.assert_called_once_with( service_type="application-catalog", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name ) kw = {"endpoint": self.service_catalog.url_for.return_value, @@ -725,7 +733,6 @@ class OSClientsTestCase(test.TestCase): self.assertEqual(fake_designate, client) self.service_catalog.url_for.assert_called_once_with( service_type="dns", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name ) @@ -766,7 +773,7 @@ class OSClientsTestCase(test.TestCase): client = self.clients.cue() self.assertEqual(fake_cue, client) mock_cue.client.Client.assert_called_once_with( - interface=consts.EndpointType.PUBLIC, + interface=self.credential.endpoint_type, session="fake_session") self.assertEqual(fake_cue, self.clients.cache["cue"]) @@ -805,11 +812,10 @@ class OSClientsTestCase(test.TestCase): self.service_catalog.url_for.assert_called_once_with( service_type="container-infra", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) mock_magnum.client.Client.assert_called_once_with( - interface=consts.EndpointType.PUBLIC, + interface=self.credential.endpoint_type, session=self.fake_keystone.session, magnum_url="http://fakeurl/") @@ -827,7 +833,6 @@ class OSClientsTestCase(test.TestCase): self.service_catalog.url_for.assert_called_once_with( service_type="infra-optim", - endpoint_type=consts.EndpointType.PUBLIC, region_name=self.credential.region_name) mock_watcher.client.Client.assert_called_once_with( @@ -836,6 +841,7 @@ class OSClientsTestCase(test.TestCase): token=self.fake_keystone.auth_token, ca_file=None, insecure=False, - timeout=180.0) + timeout=180.0, + endpoint_type=self.credential.endpoint_type) self.assertEqual(fake_watcher, self.clients.cache["watcher"])