From 5b8cca04dc56736540bd76c3c65f8cb9e24bcf92 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Tue, 26 Aug 2014 13:38:30 +0200 Subject: [PATCH] Decoupling client creation from actual config calls Client creation should be decoupled from the config calls and always passed as parameter. Adding deprecation for now it will be deleted later. Unification of client creation across all config functions. Change-Id: I26b1d42fffcd678ca5b40174b3f3ac7fc356c5a4 --- os_cloud_config/cmd/register_nodes.py | 9 +++- os_cloud_config/cmd/setup_endpoints.py | 10 ++-- os_cloud_config/cmd/setup_neutron.py | 8 +++- .../cmd/tests/test_register_nodes.py | 48 ++++++++++++++++++- .../cmd/tests/test_setup_endpoints.py | 11 +++-- .../cmd/tests/test_setup_neutron.py | 15 +++++- os_cloud_config/keystone.py | 9 ++-- os_cloud_config/neutron.py | 30 +++++++----- os_cloud_config/nodes.py | 6 +++ 9 files changed, 116 insertions(+), 30 deletions(-) diff --git a/os_cloud_config/cmd/register_nodes.py b/os_cloud_config/cmd/register_nodes.py index 8df8b7a..cc025e2 100644 --- a/os_cloud_config/cmd/register_nodes.py +++ b/os_cloud_config/cmd/register_nodes.py @@ -18,6 +18,7 @@ import json import logging import textwrap +from os_cloud_config.cmd.utils import _clients from os_cloud_config.cmd.utils import environment from os_cloud_config import nodes @@ -59,7 +60,13 @@ def main(): environment._ensure() # TODO(StevenK): Filter out registered nodes. - nodes.register_all_nodes(args.service_host, nodes_list) + keystone_client = _clients.get_keystone_client() + if nodes.using_ironic(keystone=keystone_client): + client = _clients.get_ironic_client() + else: + client = _clients.get_nova_bm_client() + + nodes.register_all_nodes(args.service_host, nodes_list, client=client) except Exception: logging.exception("Unexpected error during command execution") return 1 diff --git a/os_cloud_config/cmd/setup_endpoints.py b/os_cloud_config/cmd/setup_endpoints.py index b970e58..89a8f01 100644 --- a/os_cloud_config/cmd/setup_endpoints.py +++ b/os_cloud_config/cmd/setup_endpoints.py @@ -17,6 +17,8 @@ import simplejson import sys import textwrap + +from os_cloud_config.cmd.utils import _clients from os_cloud_config.cmd.utils import environment from os_cloud_config import keystone @@ -69,11 +71,11 @@ def main(stdout=None): # we assume it's just JSON string services = simplejson.loads(args.services) + client = _clients.get_keystone_client() + keystone.setup_endpoints( services, public_host=args.public_host, region=args.region, - os_username=os.environ["OS_USERNAME"], - os_password=os.environ["OS_PASSWORD"], - os_tenant_name=os.environ["OS_TENANT_NAME"], - os_auth_url=os.environ["OS_AUTH_URL"]) + os_auth_url=os.environ["OS_AUTH_URL"], + client=client) diff --git a/os_cloud_config/cmd/setup_neutron.py b/os_cloud_config/cmd/setup_neutron.py index 7813b6f..35a2a88 100644 --- a/os_cloud_config/cmd/setup_neutron.py +++ b/os_cloud_config/cmd/setup_neutron.py @@ -18,6 +18,7 @@ import json import logging import textwrap +from os_cloud_config.cmd.utils import _clients from os_cloud_config.cmd.utils import environment from os_cloud_config import neutron @@ -68,7 +69,12 @@ def main(): environment._ensure() with open(args.json, 'r') as jsonfile: network_desc = json.load(jsonfile) - neutron.initialize_neutron(network_desc) + + neutron_client = _clients.get_neutron_client() + keystone_client = _clients.get_keystone_client() + neutron.initialize_neutron(network_desc, + neutron_client=neutron_client, + keystone_client=keystone_client) except Exception: logging.exception("Unexpected error during command execution") return 1 diff --git a/os_cloud_config/cmd/tests/test_register_nodes.py b/os_cloud_config/cmd/tests/test_register_nodes.py index 4e2dbb0..59acc7c 100644 --- a/os_cloud_config/cmd/tests/test_register_nodes.py +++ b/os_cloud_config/cmd/tests/test_register_nodes.py @@ -24,18 +24,62 @@ from os_cloud_config.tests import base class RegisterNodesTest(base.TestCase): + @mock.patch('os_cloud_config.cmd.utils._clients.get_nova_bm_client', + return_value='nova_bm_client_mock') + @mock.patch('os_cloud_config.cmd.utils._clients.get_keystone_client', + return_value='keystone_client_mock') + @mock.patch('os_cloud_config.nodes.using_ironic', return_value=False) @mock.patch('os_cloud_config.nodes.register_all_nodes') @mock.patch.dict('os.environ', {'OS_USERNAME': 'a', 'OS_PASSWORD': 'a', 'OS_TENANT_NAME': 'a', 'OS_AUTH_URL': 'a'}) @mock.patch.object(sys, 'argv', ['register-nodes', '--service-host', 'seed', '--nodes']) - def test_with_arguments(self, register_mock): + def test_with_arguments_nova_baremetal(self, register_mock, + using_ironic_mock, + get_keystone_client_mock, + get_nova_bm_client_mock): with tempfile.NamedTemporaryFile() as f: f.write(u'{}\n'.encode('utf-8')) f.flush() sys.argv.append(f.name) return_code = register_nodes.main() - register_mock.has_calls([mock.call("seed", "{}")]) + + register_mock.assert_called_once_with( + "seed", {}, client='nova_bm_client_mock') + using_ironic_mock.assert_called_once_with( + keystone='keystone_client_mock') + get_keystone_client_mock.assert_called_once_with() + get_nova_bm_client_mock.assert_called_once_with() + + self.assertEqual(0, return_code) + + @mock.patch('os_cloud_config.cmd.utils._clients.get_ironic_client', + return_value='ironic_client_mock') + @mock.patch('os_cloud_config.cmd.utils._clients.get_keystone_client', + return_value='keystone_client_mock') + @mock.patch('os_cloud_config.nodes.using_ironic', return_value=True) + @mock.patch('os_cloud_config.nodes.register_all_nodes') + @mock.patch.dict('os.environ', {'OS_USERNAME': 'a', 'OS_PASSWORD': 'a', + 'OS_TENANT_NAME': 'a', 'OS_AUTH_URL': 'a'}) + @mock.patch.object(sys, 'argv', ['register-nodes', '--service-host', + 'seed', '--nodes']) + def test_with_arguments_ironic(self, register_mock, + using_ironic_mock, + get_keystone_client_mock, + get_ironic_client_mock): + with tempfile.NamedTemporaryFile() as f: + f.write(u'{}\n'.encode('utf-8')) + f.flush() + sys.argv.append(f.name) + return_code = register_nodes.main() + + register_mock.assert_called_once_with( + "seed", {}, client='ironic_client_mock') + using_ironic_mock.assert_called_once_with( + keystone='keystone_client_mock') + get_keystone_client_mock.assert_called_once_with() + get_ironic_client_mock.assert_called_once_with() + self.assertEqual(0, return_code) @mock.patch('os_cloud_config.nodes.register_all_nodes') diff --git a/os_cloud_config/cmd/tests/test_setup_endpoints.py b/os_cloud_config/cmd/tests/test_setup_endpoints.py index 594e499..bde81bc 100644 --- a/os_cloud_config/cmd/tests/test_setup_endpoints.py +++ b/os_cloud_config/cmd/tests/test_setup_endpoints.py @@ -20,6 +20,8 @@ from os_cloud_config.tests import base class SetupEndpointsTest(base.TestCase): + @mock.patch('os_cloud_config.cmd.utils._clients.get_keystone_client', + return_value='keystone_client_mock') @mock.patch('os_cloud_config.keystone.setup_endpoints') @mock.patch.object( sys, 'argv', @@ -29,13 +31,12 @@ class SetupEndpointsTest(base.TestCase): 'OS_PASSWORD': 'password', 'OS_TENANT_NAME': 'admin', 'OS_AUTH_URL': 'http://localhost:5000'}) - def test_script(self, setup_endpoints_mock): + def test_script(self, setup_endpoints_mock, get_keystone_client_mock): setup_endpoints.main() + get_keystone_client_mock.assert_called_once_with() setup_endpoints_mock.assert_called_once_with( {'nova': {'password': '123'}}, public_host='192.0.2.28', region='EC', - os_username="admin", - os_password="password", - os_tenant_name="admin", - os_auth_url="http://localhost:5000") + os_auth_url="http://localhost:5000", + client="keystone_client_mock") diff --git a/os_cloud_config/cmd/tests/test_setup_neutron.py b/os_cloud_config/cmd/tests/test_setup_neutron.py index 4fa17b3..8691f88 100644 --- a/os_cloud_config/cmd/tests/test_setup_neutron.py +++ b/os_cloud_config/cmd/tests/test_setup_neutron.py @@ -25,18 +25,29 @@ from os_cloud_config.tests import base class SetupNeutronTest(base.TestCase): + @mock.patch('os_cloud_config.cmd.utils._clients.get_keystone_client', + return_value='keystone_client_mock') + @mock.patch('os_cloud_config.cmd.utils._clients.get_neutron_client', + return_value='neutron_client_mock') @mock.patch('os_cloud_config.neutron.initialize_neutron') @mock.patch.dict('os.environ', {'OS_USERNAME': 'a', 'OS_PASSWORD': 'a', 'OS_TENANT_NAME': 'a', 'OS_AUTH_URL': 'a'}) @mock.patch.object(sys, 'argv', ['setup-neutron', '--network-json']) - def test_with_arguments(self, initialize_mock): + def test_with_arguments(self, initialize_mock, get_neutron_client_mock, + get_keystone_client_mock): network_desc = {'physical': {'metadata_server': 'foo.bar'}} with tempfile.NamedTemporaryFile() as f: f.write(json.dumps(network_desc).encode('utf-8')) f.flush() sys.argv.append(f.name) return_code = setup_neutron.main() - initialize_mock.assert_called_once_with(mock.ANY) + + get_keystone_client_mock.assert_called_once_with() + get_neutron_client_mock.assert_called_once_with() + initialize_mock.assert_called_once_with( + network_desc, + neutron_client='neutron_client_mock', + keystone_client='keystone_client_mock') self.assertEqual(0, return_code) @mock.patch('os_cloud_config.neutron.initialize_neutron') diff --git a/os_cloud_config/keystone.py b/os_cloud_config/keystone.py index ed1e79f..4979e58 100644 --- a/os_cloud_config/keystone.py +++ b/os_cloud_config/keystone.py @@ -19,6 +19,7 @@ import time from keystoneclient.openstack.common.apiclient import exceptions import keystoneclient.v2_0.client as ksclient +from os_cloud_config.utils import clients LOG = logging.getLogger(__name__) @@ -272,10 +273,10 @@ def setup_endpoints(endpoints, public_host=None, region=None, client=None, } if not client: - client = ksclient.Client(username=os_username, - password=os_password, - tenant_name=os_tenant_name, - auth_url=os_auth_url) + LOG.warn('Creating client inline is deprecated, please pass ' + 'the client as parameter.') + client = clients.get_keystone_client( + os_username, os_password, os_tenant_name, os_auth_url) # Setup roles first _setup_roles(client) diff --git a/os_cloud_config/neutron.py b/os_cloud_config/neutron.py index 55b0253..feb6b8e 100644 --- a/os_cloud_config/neutron.py +++ b/os_cloud_config/neutron.py @@ -20,10 +20,18 @@ from os_cloud_config.cmd.utils import _clients as clients LOG = logging.getLogger(__name__) -def initialize_neutron(network_desc): - neutron = clients.get_neutron_client() - keystone = clients.get_keystone_client() - admin_tenant = _get_admin_tenant_id(keystone) +def initialize_neutron(network_desc, neutron_client=None, + keystone_client=None): + if not neutron_client: + LOG.warn('Creating neutron client inline is deprecated, please pass ' + 'the client as parameter.') + neutron_client = clients.get_neutron_client() + if not keystone_client: + LOG.warn('Creating keystone client inline is deprecated, please pass ' + 'the client as parameter.') + keystone_client = clients.get_keystone_client() + + admin_tenant = _get_admin_tenant_id(keystone_client) if 'physical' in network_desc: network_type = 'physical' if not admin_tenant: @@ -35,15 +43,15 @@ def initialize_neutron(network_desc): network_type = 'float' else: raise ValueError("No float or physical network defined.") - net = _create_net(neutron, network_desc, network_type, admin_tenant) - subnet = _create_subnet(neutron, net, network_desc, network_type, + net = _create_net(neutron_client, network_desc, network_type, admin_tenant) + subnet = _create_subnet(neutron_client, net, network_desc, network_type, admin_tenant) if 'external' in network_desc: - router = _create_router(neutron, subnet) - ext_net = _create_net(neutron, network_desc, 'external', None) - _create_subnet(neutron, ext_net, network_desc, 'external', None) - neutron.add_gateway_router(router['router']['id'], - {'network_id': ext_net['network']['id']}) + router = _create_router(neutron_client, subnet) + ext_net = _create_net(neutron_client, network_desc, 'external', None) + _create_subnet(neutron_client, ext_net, network_desc, 'external', None) + neutron_client.add_gateway_router( + router['router']['id'], {'network_id': ext_net['network']['id']}) LOG.debug("Neutron configured.") diff --git a/os_cloud_config/nodes.py b/os_cloud_config/nodes.py index 58c0054..5ebcb22 100644 --- a/os_cloud_config/nodes.py +++ b/os_cloud_config/nodes.py @@ -105,10 +105,14 @@ def register_all_nodes(service_host, nodes_list, client=None): LOG.debug('Registering all nodes.') if using_ironic(keystone=None): if client is None: + LOG.warn('Creating ironic client inline is deprecated, please ' + 'pass the client as parameter.') client = clients.get_ironic_client() register_func = register_ironic_node else: if client is None: + LOG.warn('Creating nova-bm client inline is deprecated, please ' + 'pass the client as parameter.') client = clients.get_nova_bm_client() register_func = register_nova_bm_node for node in nodes_list: @@ -118,5 +122,7 @@ def register_all_nodes(service_host, nodes_list, client=None): def using_ironic(keystone=None): LOG.debug('Checking for usage of ironic.') if keystone is None: + LOG.warn('Creating keystone client inline is deprecated, please pass ' + 'the client as parameter.') keystone = clients.get_keystone_client() return 'ironic' in [service.name for service in keystone.services.list()]