From 87a053a5aa10421316a530436818fdd836194ed1 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 15 May 2018 22:19:28 +0200 Subject: [PATCH] Use openstack.config for arguments parsing This change switches the CLI to use openstacksdk for populating authentication arguments and creating a session. As a side effect, using clouds.yaml is now supported and used in the CI. Change-Id: If20ddc46f10d9deb34e595310313bd87e2e7243b --- README.rst | 12 +++----- metalsmith/_cmd.py | 34 ++++++--------------- metalsmith/_os_api.py | 15 +++++++-- metalsmith/_provisioner.py | 4 +-- metalsmith/test/test_cmd.py | 56 ++++++++++++++++++++++++++-------- metalsmith/test/test_os_api.py | 38 +++++++++++++++++++++++ playbooks/integration/run.yaml | 33 +++++--------------- requirements.txt | 2 +- 8 files changed, 119 insertions(+), 75 deletions(-) create mode 100644 metalsmith/test/test_os_api.py diff --git a/README.rst b/README.rst index 6a58e5d..d29e3cd 100644 --- a/README.rst +++ b/README.rst @@ -20,24 +20,22 @@ Installation Usage ----- -Start with sourcing your OpenStack credentials, for example:: - - . ~/stackrc - Generic usage is as follows:: - metalsmith deploy --image --network \ - --ssh-public-key + metalsmith --os-cloud deploy --image \ + --network --ssh-public-key \ + This is an example suitable for TripleO (replace ``compute`` with the profile you want to deploy):: + source ~/stackrc metalsmith deploy --image overcloud-full --network ctlplane \ --capability profile=compute --ssh-public-key ~/.ssh/id_rsa.pub baremetal To remove the deployed instance:: - metalsmith undeploy + metalsmith --os-cloud undeploy For all possible options see the built-in help:: diff --git a/metalsmith/_cmd.py b/metalsmith/_cmd.py index b43ec54..6a273ad 100644 --- a/metalsmith/_cmd.py +++ b/metalsmith/_cmd.py @@ -15,11 +15,9 @@ import argparse import logging -import os import sys -from keystoneauth1.identity import generic -from keystoneauth1 import session +from openstack import config as os_config from metalsmith import _provisioner @@ -49,7 +47,7 @@ def _do_undeploy(api, args, wait=None): api.unprovision_node(args.node, wait=wait) -def _parse_args(args): +def _parse_args(args, config): parser = argparse.ArgumentParser( description='Deployment and Scheduling tool for Bare Metal') verbosity = parser.add_mutually_exclusive_group() @@ -60,19 +58,12 @@ def _parse_args(args): parser.add_argument('--dry-run', action='store_true', help='do not take any destructive actions') wait = parser.add_mutually_exclusive_group() - wait.add_argument('--timeout', type=int, default=1800, + wait.add_argument('--wait', type=int, default=1800, help='action timeout (in seconds)') wait.add_argument('--no-wait', action='store_true', help='disable waiting for action to finish') - parser.add_argument('--os-username', default=os.environ.get('OS_USERNAME')) - parser.add_argument('--os-password', default=os.environ.get('OS_PASSWORD')) - parser.add_argument('--os-project-name', - default=os.environ.get('OS_PROJECT_NAME')) - parser.add_argument('--os-auth-url', default=os.environ.get('OS_AUTH_URL')) - parser.add_argument('--os-user-domain-name', - default=os.environ.get('OS_USER_DOMAIN_NAME')) - parser.add_argument('--os-project-domain-name', - default=os.environ.get('OS_PROJECT_DOMAIN_NAME')) + + config.register_argparse_arguments(parser, sys.argv[1:]) subparsers = parser.add_subparsers() @@ -116,21 +107,16 @@ def _configure_logging(args): def main(args=sys.argv[1:]): - args = _parse_args(args) + config = os_config.OpenStackConfig() + args = _parse_args(args, config) _configure_logging(args) if args.no_wait: wait = None else: - wait = args.timeout + wait = args.wait - auth = generic.Password(auth_url=args.os_auth_url, - username=args.os_username, - project_name=args.os_project_name, - password=args.os_password, - user_domain_name=args.os_user_domain_name, - project_domain_name=args.os_project_domain_name) - sess = session.Session(auth=auth) - api = _provisioner.Provisioner(sess, dry_run=args.dry_run) + region = config.get_one(argparse=args) + api = _provisioner.Provisioner(cloud_region=region, dry_run=args.dry_run) try: args.func(api, args, wait=wait) diff --git a/metalsmith/_os_api.py b/metalsmith/_os_api.py index 3b90bb9..3346272 100644 --- a/metalsmith/_os_api.py +++ b/metalsmith/_os_api.py @@ -1,4 +1,4 @@ -# Copyright 2015 Red Hat, Inc. +# Copyright 2015-2018 Red Hat, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -42,8 +42,17 @@ class API(object): IRONIC_VERSION = '1' IRONIC_MICRO_VERSION = '1.28' - def __init__(self, session): - self.session = session + def __init__(self, session=None, cloud_region=None): + if cloud_region is None: + if session is None: + raise TypeError('Either session or cloud_region must ' + 'be provided') + self.session = session + elif session is not None: + raise TypeError('Either session or cloud_region must be provided, ' + 'but not both') + else: + self.session = cloud_region.get_session() LOG.debug('Creating service clients') self.glance = glanceclient.Client(self.GLANCE_VERSION, diff --git a/metalsmith/_provisioner.py b/metalsmith/_provisioner.py index a43ebbb..2339c8b 100644 --- a/metalsmith/_provisioner.py +++ b/metalsmith/_provisioner.py @@ -32,8 +32,8 @@ _CREATED_PORTS = 'metalsmith_created_ports' class Provisioner(object): """API to deploy/undeploy nodes with OpenStack.""" - def __init__(self, session, dry_run=False): - self._api = _os_api.API(session) + def __init__(self, session=None, cloud_region=None, dry_run=False): + self._api = _os_api.API(session=session, cloud_region=cloud_region) self._dry_run = dry_run def reserve_node(self, resource_class, capabilities=None): diff --git a/metalsmith/test/test_cmd.py b/metalsmith/test/test_cmd.py index 706fcb4..a3d5ad6 100644 --- a/metalsmith/test/test_cmd.py +++ b/metalsmith/test/test_cmd.py @@ -23,12 +23,14 @@ from metalsmith import _provisioner @mock.patch.object(_provisioner, 'Provisioner', autospec=True) -@mock.patch.object(_cmd.generic, 'Password', autospec=True) +@mock.patch.object(_cmd.os_config, 'OpenStackConfig', autospec=True) class TestMain(testtools.TestCase): - def test_args_ok(self, mock_auth, mock_pr): + def test_args_ok(self, mock_os_conf, mock_pr): args = ['deploy', '--network', 'mynet', '--image', 'myimg', 'compute'] _cmd.main(args) - mock_pr.assert_called_once_with(mock.ANY, dry_run=False) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=False) mock_pr.return_value.reserve_node.assert_called_once_with( resource_class='compute', capabilities={} @@ -42,11 +44,33 @@ class TestMain(testtools.TestCase): netboot=False, wait=1800) - def test_args_debug(self, mock_auth, mock_pr): + def test_args_dry_run(self, mock_os_conf, mock_pr): + args = ['--dry-run', 'deploy', '--network', 'mynet', + '--image', 'myimg', 'compute'] + _cmd.main(args) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=True) + mock_pr.return_value.reserve_node.assert_called_once_with( + resource_class='compute', + capabilities={} + ) + mock_pr.return_value.provision_node.assert_called_once_with( + mock_pr.return_value.reserve_node.return_value, + image_ref='myimg', + network_refs=['mynet'], + root_disk_size=None, + ssh_keys=[], + netboot=False, + wait=1800) + + def test_args_debug(self, mock_os_conf, mock_pr): args = ['--debug', 'deploy', '--network', 'mynet', '--image', 'myimg', 'compute'] _cmd.main(args) - mock_pr.assert_called_once_with(mock.ANY, dry_run=False) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=False) mock_pr.return_value.reserve_node.assert_called_once_with( resource_class='compute', capabilities={} @@ -60,11 +84,13 @@ class TestMain(testtools.TestCase): netboot=False, wait=1800) - def test_args_quiet(self, mock_auth, mock_pr): + def test_args_quiet(self, mock_os_conf, mock_pr): args = ['--quiet', 'deploy', '--network', 'mynet', '--image', 'myimg', 'compute'] _cmd.main(args) - mock_pr.assert_called_once_with(mock.ANY, dry_run=False) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=False) mock_pr.return_value.reserve_node.assert_called_once_with( resource_class='compute', capabilities={} @@ -79,7 +105,7 @@ class TestMain(testtools.TestCase): wait=1800) @mock.patch.object(_cmd.LOG, 'critical', autospec=True) - def test_reservation_failure(self, mock_log, mock_auth, mock_pr): + def test_reservation_failure(self, mock_log, mock_os_conf, mock_pr): args = ['deploy', '--network', 'mynet', '--image', 'myimg', 'compute'] failure = RuntimeError('boom') mock_pr.return_value.reserve_node.side_effect = failure @@ -87,19 +113,21 @@ class TestMain(testtools.TestCase): mock_log.assert_called_once_with('%s', failure, exc_info=False) @mock.patch.object(_cmd.LOG, 'critical', autospec=True) - def test_deploy_failure(self, mock_log, mock_auth, mock_pr): + def test_deploy_failure(self, mock_log, mock_os_conf, mock_pr): args = ['deploy', '--network', 'mynet', '--image', 'myimg', 'compute'] failure = RuntimeError('boom') mock_pr.return_value.provision_node.side_effect = failure self.assertRaises(SystemExit, _cmd.main, args) mock_log.assert_called_once_with('%s', failure, exc_info=False) - def test_args_capabilities(self, mock_auth, mock_pr): + def test_args_capabilities(self, mock_os_conf, mock_pr): args = ['deploy', '--network', 'mynet', '--image', 'myimg', '--capability', 'foo=bar', '--capability', 'answer=42', 'compute'] _cmd.main(args) - mock_pr.assert_called_once_with(mock.ANY, dry_run=False) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=False) mock_pr.return_value.reserve_node.assert_called_once_with( resource_class='compute', capabilities={'foo': 'bar', 'answer': '42'} @@ -113,7 +141,7 @@ class TestMain(testtools.TestCase): netboot=False, wait=1800) - def test_args_configdrive(self, mock_auth, mock_pr): + def test_args_configdrive(self, mock_os_conf, mock_pr): with tempfile.NamedTemporaryFile() as fp: fp.write(b'foo\n') fp.flush() @@ -121,7 +149,9 @@ class TestMain(testtools.TestCase): args = ['deploy', '--network', 'mynet', '--image', 'myimg', '--ssh-public-key', fp.name, 'compute'] _cmd.main(args) - mock_pr.assert_called_once_with(mock.ANY, dry_run=False) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=False) mock_pr.return_value.reserve_node.assert_called_once_with( resource_class='compute', capabilities={} diff --git a/metalsmith/test/test_os_api.py b/metalsmith/test/test_os_api.py new file mode 100644 index 0000000..021441d --- /dev/null +++ b/metalsmith/test/test_os_api.py @@ -0,0 +1,38 @@ +# Copyright 2018 Red Hat, Inc. +# +# 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. + +import mock +import testtools + +from metalsmith import _os_api + + +class TestInit(testtools.TestCase): + def test_missing_auth(self): + self.assertRaisesRegex(TypeError, 'must be provided', _os_api.API) + + def test_both_provided(self): + self.assertRaisesRegex(TypeError, 'not both', _os_api.API, + session=mock.Mock(), cloud_region=mock.Mock()) + + def test_session_only(self): + session = mock.Mock() + api = _os_api.API(session=session) + self.assertIs(api.session, session) + + def test_cloud_region_only(self): + region = mock.Mock() + api = _os_api.API(cloud_region=region) + self.assertIs(api.session, region.get_session.return_value) diff --git a/playbooks/integration/run.yaml b/playbooks/integration/run.yaml index 503858e..0060406 100644 --- a/playbooks/integration/run.yaml +++ b/playbooks/integration/run.yaml @@ -37,22 +37,14 @@ ssh_key_file: "{{ ssh_key_result.stdout }}" - name: Deploy a node - shell: | - # FIXME(dtantsur): just use OS_CLOUD - source /opt/stack/devstack/openrc admin admin - metalsmith --debug deploy \ - --network private \ - --image {{ cirros_image }} \ - --ssh-public-key {{ ssh_key_file }} \ - --root-disk-size 9 \ - --netboot \ + command: > + metalsmith --debug deploy + --network private + --image {{ cirros_image }} + --ssh-public-key {{ ssh_key_file }} + --root-disk-size 9 + --netboot baremetal - args: - executable: /bin/bash - environment: - # FIXME(dtantsur): just use OS_CLOUD - OS_USER_DOMAIN_NAME: Default - OS_PROJECT_DOMAIN_NAME: Default - name: Find the deployed node command: openstack baremetal node list --provision-state active -f value -c UUID @@ -71,16 +63,7 @@ command: openstack baremetal node show {{ active_node }} - name: Undeploy a node - shell: | - # FIXME(dtantsur): just use OS_CLOUD - source /opt/stack/devstack/openrc admin admin - metalsmith --debug undeploy {{ active_node }} - args: - executable: /bin/bash - environment: - # FIXME(dtantsur): just use OS_CLOUD - OS_USER_DOMAIN_NAME: Default - OS_PROJECT_DOMAIN_NAME: Default + command: metalsmith --debug undeploy {{ active_node }} - name: Get the current status of the deployed node command: openstack baremetal node show {{ active_node }} -f value -c provision_state diff --git a/requirements.txt b/requirements.txt index f144fcb..c001d97 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. pbr!=2.1.0,>=2.0.0 # Apache-2.0 -keystoneauth1>=3.1.0 # Apache-2.0 +openstacksdk>=0.11.0 # Apache-2.0 oslo.utils>=3.20.0 # Apache-2.0 python-glanceclient>=2.8.0 # Apache-2.0 python-ironicclient>=1.14.0 # Apache-2.0