From e4e9fb594d003ea6c3ec29aab0bccf72ffab6781 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Wed, 3 Mar 2021 12:46:02 -0500 Subject: [PATCH 001/697] Add --subnet-pool to subnet list The neutron API supports filtering subnets by subnet pool id, but the CLI was missing support for it. Change-Id: Ic230c2c5cda8255d8f2c422880aeac81670b2df3 --- openstackclient/network/v2/subnet.py | 10 +++++ .../tests/unit/network/v2/test_subnet.py | 42 +++++++++++++++++++ ...st-subnet-by-pool-id-a642efc13d04fa08.yaml | 5 +++ 3 files changed, 57 insertions(+) create mode 100644 releasenotes/notes/list-subnet-by-pool-id-a642efc13d04fa08.yaml diff --git a/openstackclient/network/v2/subnet.py b/openstackclient/network/v2/subnet.py index f87f7abe1b..92a9e750c5 100644 --- a/openstackclient/network/v2/subnet.py +++ b/openstackclient/network/v2/subnet.py @@ -488,6 +488,12 @@ def get_parser(self, prog_name): "(in CIDR notation) in output " "e.g.: --subnet-range 10.10.0.0/16") ) + parser.add_argument( + '--subnet-pool', + metavar='', + help=_("List only subnets which belong to a given subnet pool " + "in output (Name or ID)") + ) _tag.add_tag_filtering_option_to_parser(parser, _('subnets')) return parser @@ -523,6 +529,10 @@ def take_action(self, parsed_args): filters['name'] = parsed_args.name if parsed_args.subnet_range: filters['cidr'] = parsed_args.subnet_range + if parsed_args.subnet_pool: + subnetpool_id = network_client.find_subnet_pool( + parsed_args.subnet_pool, ignore_missing=False).id + filters['subnetpool_id'] = subnetpool_id _tag.get_tag_filtering_args(parsed_args, filters) data = network_client.subnets(**filters) diff --git a/openstackclient/tests/unit/network/v2/test_subnet.py b/openstackclient/tests/unit/network/v2/test_subnet.py index 1b4bfdad2f..06096f4b63 100644 --- a/openstackclient/tests/unit/network/v2/test_subnet.py +++ b/openstackclient/tests/unit/network/v2/test_subnet.py @@ -899,6 +899,48 @@ def test_subnet_list_subnet_range(self): self.assertEqual(self.columns, columns) self.assertItemsEqual(self.data, list(data)) + def test_subnet_list_subnetpool_by_name(self): + subnet_pool = network_fakes.FakeSubnetPool.create_one_subnet_pool() + subnet = network_fakes.FakeSubnet.create_one_subnet( + {'subnetpool_id': subnet_pool.id}) + self.network.find_network = mock.Mock(return_value=subnet) + self.network.find_subnet_pool = mock.Mock(return_value=subnet_pool) + arglist = [ + '--subnet-pool', subnet_pool.name, + ] + verifylist = [ + ('subnet_pool', subnet_pool.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + filters = {'subnetpool_id': subnet_pool.id} + + self.network.subnets.assert_called_once_with(**filters) + self.assertEqual(self.columns, columns) + self.assertItemsEqual(self.data, list(data)) + + def test_subnet_list_subnetpool_by_id(self): + subnet_pool = network_fakes.FakeSubnetPool.create_one_subnet_pool() + subnet = network_fakes.FakeSubnet.create_one_subnet( + {'subnetpool_id': subnet_pool.id}) + self.network.find_network = mock.Mock(return_value=subnet) + self.network.find_subnet_pool = mock.Mock(return_value=subnet_pool) + arglist = [ + '--subnet-pool', subnet_pool.id, + ] + verifylist = [ + ('subnet_pool', subnet_pool.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + filters = {'subnetpool_id': subnet_pool.id} + + self.network.subnets.assert_called_once_with(**filters) + self.assertEqual(self.columns, columns) + self.assertItemsEqual(self.data, list(data)) + def test_list_with_tag_options(self): arglist = [ '--tags', 'red,blue', diff --git a/releasenotes/notes/list-subnet-by-pool-id-a642efc13d04fa08.yaml b/releasenotes/notes/list-subnet-by-pool-id-a642efc13d04fa08.yaml new file mode 100644 index 0000000000..d784a9aadb --- /dev/null +++ b/releasenotes/notes/list-subnet-by-pool-id-a642efc13d04fa08.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Add ``--subnet-pool`` option to ``subnet list`` to filter + by subnets by subnet pool. From 1169a114e7388e4ee622dca98b9fb8e2a06c8ec3 Mon Sep 17 00:00:00 2001 From: "wu.shiming" Date: Thu, 3 Jun 2021 14:49:23 +0800 Subject: [PATCH 002/697] Changed minversion in tox to 3.18.0 The patch bumps min version of tox to 3.18.0 in order to replace tox's whitelist_externals by allowlist_externals option: https://github.com/tox-dev/tox/blob/master/docs/changelog.rst#v3180-2020-07-23 Change-Id: Ibb77fa2afad3f09e95f0dba243d3a096daedd787 --- tox.ini | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tox.ini b/tox.ini index ebcdc2d5d9..5e0be38422 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -minversion = 3.2.0 +minversion = 3.18.0 envlist = py38,pep8 skipdist = True # Automatic envs (pyXX) will only use the python version appropriate to that @@ -18,7 +18,7 @@ deps = -r{toxinidir}/test-requirements.txt -r{toxinidir}/requirements.txt commands = stestr run {posargs} -whitelist_externals = stestr +allowlist_externals = stestr [testenv:fast8] # Use same environment directory as pep8 env to save space and install time @@ -71,7 +71,7 @@ commands = pythom -m pip install -q -e "git+file://{toxinidir}/../openstacksdk#egg=openstacksdk" python -m pip freeze stestr run {posargs} -whitelist_externals = stestr +allowlist_externals = stestr [testenv:functional] setenv = OS_TEST_PATH=./openstackclient/tests/functional From eca1fcd65f22ead444ec39fe20f48c83f8d7c1cf Mon Sep 17 00:00:00 2001 From: David Caro Date: Wed, 2 Jun 2021 16:33:53 +0200 Subject: [PATCH 003/697] Include hosts in aggregate list --long This makes it easier to get the total list of aggregates and the hosts belonging to each of them (specially for scripting purposes). Change-Id: I94833c15075ae655bc11e7c0fc47c0abad5846fc Signed-off-by: David Caro --- openstackclient/compute/v2/aggregate.py | 2 ++ openstackclient/tests/unit/compute/v2/test_aggregate.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/openstackclient/compute/v2/aggregate.py b/openstackclient/compute/v2/aggregate.py index e39eb2d272..37522a788a 100644 --- a/openstackclient/compute/v2/aggregate.py +++ b/openstackclient/compute/v2/aggregate.py @@ -193,12 +193,14 @@ def take_action(self, parsed_args): "Name", "Availability Zone", "Properties", + "Hosts", ) columns = ( "ID", "Name", "Availability Zone", "Metadata", + "Hosts", ) else: column_headers = columns = ( diff --git a/openstackclient/tests/unit/compute/v2/test_aggregate.py b/openstackclient/tests/unit/compute/v2/test_aggregate.py index 8563f98811..92ff607f94 100644 --- a/openstackclient/tests/unit/compute/v2/test_aggregate.py +++ b/openstackclient/tests/unit/compute/v2/test_aggregate.py @@ -234,6 +234,7 @@ class TestAggregateList(TestAggregate): "Name", "Availability Zone", "Properties", + "Hosts", ) list_data = (( @@ -251,6 +252,7 @@ class TestAggregateList(TestAggregate): for key, value in TestAggregate.fake_ag.metadata.items() if key != 'availability_zone' }), + format_columns.ListColumn(TestAggregate.fake_ag.hosts), ), ) def setUp(self): From 43639e1118a757fd1a00d9ea999db43133c40763 Mon Sep 17 00:00:00 2001 From: Cyril Roelandt Date: Tue, 26 Oct 2021 15:53:35 +0200 Subject: [PATCH 004/697] Fix typos Change-Id: Idd502c8df21da79ff3b9339870f38378f5337879 --- openstackclient/common/progressbar.py | 2 +- openstackclient/common/quota.py | 4 ++-- openstackclient/compute/v2/server.py | 4 ++-- openstackclient/identity/v3/endpoint_group.py | 2 +- openstackclient/tests/functional/base.py | 4 ++-- .../tests/functional/volume/v1/test_volume_type.py | 4 ++-- .../tests/functional/volume/v2/test_volume_type.py | 4 ++-- .../tests/functional/volume/v3/test_volume_type.py | 4 ++-- openstackclient/tests/unit/compute/v2/fakes.py | 4 ++-- openstackclient/volume/v1/volume_type.py | 2 +- openstackclient/volume/v2/volume_snapshot.py | 2 +- 11 files changed, 18 insertions(+), 18 deletions(-) diff --git a/openstackclient/common/progressbar.py b/openstackclient/common/progressbar.py index ef767a9cf8..7678aceba0 100644 --- a/openstackclient/common/progressbar.py +++ b/openstackclient/common/progressbar.py @@ -17,7 +17,7 @@ class _ProgressBarBase(object): - """A progress bar provider for a wrapped obect. + """A progress bar provider for a wrapped object. Base abstract class used by specific class wrapper to show a progress bar when the wrapped object are consumed. diff --git a/openstackclient/common/quota.py b/openstackclient/common/quota.py index 643cb4e474..00bbc51459 100644 --- a/openstackclient/common/quota.py +++ b/openstackclient/common/quota.py @@ -205,7 +205,7 @@ def get_network_quota(self, parsed_args): class ListQuota(command.Lister, BaseQuota): _description = _( "List quotas for all projects with non-default quota values or " - "list detailed quota informations for requested project") + "list detailed quota information for requested project") def _get_detailed_quotas(self, parsed_args): columns = ( @@ -230,7 +230,7 @@ def _get_detailed_quotas(self, parsed_args): result = [] for resource, values in quotas.items(): # NOTE(slaweq): there is no detailed quotas info for some resources - # and it should't be displayed here + # and it shouldn't be displayed here if type(values) is dict: result.append({ 'resource': resource, diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index c11f4b5781..b0de65a7c3 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2466,7 +2466,7 @@ def take_action(self, parsed_args): # and 'image' missing in the server response during # infrastructure failure situations. # For those servers with partial constructs we just skip the - # processing of the image and flavor informations. + # processing of the image and flavor information. if not hasattr(s, 'image') or not hasattr(s, 'flavor'): continue if 'id' in s.image: @@ -4292,7 +4292,7 @@ def _show_progress(progress): server_obj.shelve() - # if we don't hav to wait, either because it was requested explicitly + # if we don't have to wait, either because it was requested explicitly # or is required implicitly, then our job is done if not parsed_args.wait and not parsed_args.offload: return diff --git a/openstackclient/identity/v3/endpoint_group.py b/openstackclient/identity/v3/endpoint_group.py index cbe27edb4b..9bb026a9b8 100644 --- a/openstackclient/identity/v3/endpoint_group.py +++ b/openstackclient/identity/v3/endpoint_group.py @@ -268,7 +268,7 @@ def get_parser(self, prog_name): parser.add_argument( '--name', metavar='', - help=_('New enpoint group name'), + help=_('New endpoint group name'), ) parser.add_argument( '--filters', diff --git a/openstackclient/tests/functional/base.py b/openstackclient/tests/functional/base.py index 0ed7dff8c4..3a4c13944e 100644 --- a/openstackclient/tests/functional/base.py +++ b/openstackclient/tests/functional/base.py @@ -42,7 +42,7 @@ class TestCase(testtools.TestCase): def openstack(cls, cmd, cloud=ADMIN_CLOUD, fail_ok=False): """Executes openstackclient command for the given action - NOTE(dtroyer): There is a subtle distinction between pasing + NOTE(dtroyer): There is a subtle distinction between passing cloud=None and cloud='': for compatibility reasons passing cloud=None continues to include the option '--os-auth-type none' in the command while passing cloud='' omits the '--os-auth-type' @@ -61,7 +61,7 @@ def openstack(cls, cmd, cloud=ADMIN_CLOUD, fail_ok=False): fail_ok=fail_ok ) else: - # Execure command with an explicit cloud specified + # Execute command with an explicit cloud specified return execute( 'openstack --os-cloud=' + cloud + ' ' + cmd, fail_ok=fail_ok diff --git a/openstackclient/tests/functional/volume/v1/test_volume_type.py b/openstackclient/tests/functional/volume/v1/test_volume_type.py index fb8dabdb04..7434b5b36b 100644 --- a/openstackclient/tests/functional/volume/v1/test_volume_type.py +++ b/openstackclient/tests/functional/volume/v1/test_volume_type.py @@ -118,10 +118,10 @@ def test_multi_delete(self): raw_output = self.openstack(cmd) self.assertOutput('', raw_output) - # NOTE: Add some basic funtional tests with the old format to + # NOTE: Add some basic functional tests with the old format to # make sure the command works properly, need to change # these to new test format when beef up all tests for - # volume tye commands. + # volume type commands. def test_encryption_type(self): encryption_type = uuid.uuid4().hex # test create new encryption type diff --git a/openstackclient/tests/functional/volume/v2/test_volume_type.py b/openstackclient/tests/functional/volume/v2/test_volume_type.py index 3f1a6ea8a5..861c393d80 100644 --- a/openstackclient/tests/functional/volume/v2/test_volume_type.py +++ b/openstackclient/tests/functional/volume/v2/test_volume_type.py @@ -139,10 +139,10 @@ def test_multi_delete(self): raw_output = self.openstack(cmd) self.assertOutput('', raw_output) - # NOTE: Add some basic funtional tests with the old format to + # NOTE: Add some basic functional tests with the old format to # make sure the command works properly, need to change # these to new test format when beef up all tests for - # volume tye commands. + # volume type commands. def test_encryption_type(self): name = uuid.uuid4().hex encryption_type = uuid.uuid4().hex diff --git a/openstackclient/tests/functional/volume/v3/test_volume_type.py b/openstackclient/tests/functional/volume/v3/test_volume_type.py index 79d4096998..165c625c20 100644 --- a/openstackclient/tests/functional/volume/v3/test_volume_type.py +++ b/openstackclient/tests/functional/volume/v3/test_volume_type.py @@ -139,10 +139,10 @@ def test_multi_delete(self): raw_output = self.openstack(cmd) self.assertOutput('', raw_output) - # NOTE: Add some basic funtional tests with the old format to + # NOTE: Add some basic functional tests with the old format to # make sure the command works properly, need to change # these to new test format when beef up all tests for - # volume tye commands. + # volume type commands. def test_encryption_type(self): name = uuid.uuid4().hex encryption_type = uuid.uuid4().hex diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index 3142a24489..fd0a570969 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -1285,7 +1285,7 @@ def create_one_server_group(attrs=None): class FakeServerGroupV264(object): - """Fake one server group fo API >= 2.64""" + """Fake one server group for API >= 2.64""" @staticmethod def create_one_server_group(attrs=None): @@ -1400,7 +1400,7 @@ def create_one_comp_quota(attrs=None): @staticmethod def create_one_default_comp_quota(attrs=None): - """Crate one quota""" + """Create one quota""" attrs = attrs or {} diff --git a/openstackclient/volume/v1/volume_type.py b/openstackclient/volume/v1/volume_type.py index 4f015d13f6..c584943e10 100644 --- a/openstackclient/volume/v1/volume_type.py +++ b/openstackclient/volume/v1/volume_type.py @@ -411,7 +411,7 @@ def get_parser(self, prog_name): "--encryption-type", action="store_true", help=_("Remove the encryption type for this volume type " - "(admin oly)"), + "(admin only)"), ) return parser diff --git a/openstackclient/volume/v2/volume_snapshot.py b/openstackclient/volume/v2/volume_snapshot.py index 656f59d4ac..53d8d27fed 100644 --- a/openstackclient/volume/v2/volume_snapshot.py +++ b/openstackclient/volume/v2/volume_snapshot.py @@ -98,7 +98,7 @@ def get_parser(self, prog_name): "--remote-source", metavar="", action=parseractions.KeyValueAction, - help=_("The attribute(s) of the exsiting remote volume snapshot " + help=_("The attribute(s) of the existing remote volume snapshot " "(admin required) (repeat option to specify multiple " "attributes) e.g.: '--remote-source source-name=test_name " "--remote-source source-id=test_id'"), From 8cb0a28607e7f8b9eb4bb71a95b39230d43a969c Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 2 Nov 2021 09:59:27 +0000 Subject: [PATCH 005/697] compute: Don't warn if disk overcommit params unset Due to a small logic error, we were emitting a warning about a deprecated option when the user tried to live migrate an instance using microversion 2.25 even though the user hadn't actually set that option. Correct this. Change-Id: Ib61e817bd4ced9b5533e7c7f9d8f0b45fe81c211 Signed-off-by: Stephen Finucane Story: 2009657 Task: 43836 --- openstackclient/compute/v2/server.py | 8 ++++++-- .../tests/unit/compute/v2/test_server.py | 20 +++++++++---------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index c11f4b5781..d0edaf0769 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2621,7 +2621,7 @@ def get_parser(self, prog_name): disk_group.add_argument( '--disk-overcommit', action='store_true', - default=False, + default=None, help=_( 'Allow disk over-commit on the destination host' '(supported with --os-compute-api-version 2.24 or below)' @@ -2631,7 +2631,6 @@ def get_parser(self, prog_name): '--no-disk-overcommit', dest='disk_overcommit', action='store_false', - default=False, help=_( 'Do not over-commit disk on the destination host (default)' '(supported with --os-compute-api-version 2.24 or below)' @@ -2693,6 +2692,11 @@ def _show_progress(progress): if compute_client.api_version < api_versions.APIVersion('2.25'): kwargs['disk_over_commit'] = parsed_args.disk_overcommit + # We can't use an argparse default value because then we can't + # distinguish between explicit 'False' and unset for the below + # case (microversion >= 2.25) + if kwargs['disk_over_commit'] is None: + kwargs['disk_over_commit'] = False elif parsed_args.disk_overcommit is not None: # TODO(stephenfin): Raise an error here in OSC 7.0 msg = _( diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 3d8c17fdeb..285b2b411e 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -4812,7 +4812,7 @@ def test_server_migrate_no_options(self): verifylist = [ ('live_migration', False), ('block_migration', None), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -4834,7 +4834,7 @@ def test_server_migrate_with_host_2_56(self): ('live_migration', False), ('host', 'fakehost'), ('block_migration', None), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -4856,7 +4856,7 @@ def test_server_migrate_with_block_migration(self): verifylist = [ ('live_migration', False), ('block_migration', True), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -4897,7 +4897,7 @@ def test_server_migrate_with_host_pre_2_56(self): ('live_migration', False), ('host', 'fakehost'), ('block_migration', None), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -4923,7 +4923,7 @@ def test_server_live_migrate(self): ('live_migration', True), ('host', None), ('block_migration', None), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -4947,7 +4947,7 @@ def test_server_live_migrate_with_host(self): ('live_migration', True), ('host', 'fakehost'), ('block_migration', None), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -4975,7 +4975,7 @@ def test_server_live_migrate_with_host_pre_v230(self): ('live_migration', True), ('host', 'fakehost'), ('block_migration', None), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -5002,7 +5002,7 @@ def test_server_block_live_migrate(self): verifylist = [ ('live_migration', True), ('block_migration', True), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -5088,7 +5088,7 @@ def test_server_migrate_with_wait(self, mock_wait_for_status): verifylist = [ ('live_migration', False), ('block_migration', None), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -5108,7 +5108,7 @@ def test_server_migrate_with_wait_fails(self, mock_wait_for_status): verifylist = [ ('live_migration', False), ('block_migration', None), - ('disk_overcommit', False), + ('disk_overcommit', None), ('wait', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) From 442838ed158cd8fb4168877744a60de5cfca29cc Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 2 Nov 2021 09:34:32 +0000 Subject: [PATCH 006/697] compute: Use correct command class for 'show migration' We should be inheriting from 'ShowOne'. Failure to do so results in a tuple being dumped to the screen. Not what we intended. While we're here, we update the docstring of this command to clarify the command's intent. Nova does not provide an API to retrieve an individual migration record for a cold migration or completed live migration. As such, the 'server migration show' command only works for in-progress live-migrations. Change-Id: I2e2fe3da7d642b9e8e3d930603dcde178cd68cde Signed-off-by: Stephen Finucane Story: 2009658 Task: 43837 --- openstackclient/compute/v2/server.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index d0edaf0769..1dc56fc7e6 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2967,8 +2967,13 @@ def take_action(self, parsed_args): return self.print_migrations(parsed_args, compute_client, migrations) -class ShowMigration(command.Command): - """Show a migration for a given server.""" +class ShowMigration(command.ShowOne): + """Show an in-progress live migration for a given server. + + Note that it is not possible to show cold migrations or completed + live-migrations. Use 'openstack server migration list' to get details for + these. + """ def get_parser(self, prog_name): parser = super().get_parser(prog_name) From 9acbd3e1052d533c1395eb59de4274170baed67b Mon Sep 17 00:00:00 2001 From: Thrivikram Mudunuri Date: Thu, 28 Oct 2021 18:05:03 -0400 Subject: [PATCH 007/697] Switch server image create to SDK Switch the server image create command from novaclient to SDK. Use the SDK versions of test fakes to support fake Server resources. Also, fetch updated image *after* waiting. If a user requests that we wait (--wait) for a server image to become active before returning, then we should probably return the final image. If we don't then the image can appear to be in a non-active state when it fact it's active. Correct this by fetching the image after the wait call. Change-Id: I83a403c035add9ab041ed6d59b5b29e42267f143 --- openstackclient/compute/v2/server_image.py | 18 ++++++------- .../unit/compute/v2/test_server_image.py | 27 +++++++++---------- ...-server-image-to-sdk-e3d8077ffe05bb3d.yaml | 4 +++ 3 files changed, 25 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/migrate-create-server-image-to-sdk-e3d8077ffe05bb3d.yaml diff --git a/openstackclient/compute/v2/server_image.py b/openstackclient/compute/v2/server_image.py index 6c0e3b22cf..2021fae7c0 100644 --- a/openstackclient/compute/v2/server_image.py +++ b/openstackclient/compute/v2/server_image.py @@ -73,25 +73,23 @@ def _show_progress(progress): self.app.stdout.write('\rProgress: %s' % progress) self.app.stdout.flush() - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute + image_client = self.app.client_manager.image - server = utils.find_resource( - compute_client.servers, - parsed_args.server, + server = compute_client.find_server( + parsed_args.server, ignore_missing=False, ) + if parsed_args.name: image_name = parsed_args.name else: image_name = server.name - image_id = compute_client.servers.create_image( + image_id = compute_client.create_server_image( server.id, image_name, parsed_args.properties, - ) - - image_client = self.app.client_manager.image - image = image_client.find_image(image_id) + ).id if parsed_args.wait: if utils.wait_for_status( @@ -105,6 +103,8 @@ def _show_progress(progress): _('Error creating server image: %s'), parsed_args.server) raise exceptions.CommandError + image = image_client.find_image(image_id, ignore_missing=False) + if self.app.client_manager._api_version['image'] == '1': info = {} info.update(image._info) diff --git a/openstackclient/tests/unit/compute/v2/test_server_image.py b/openstackclient/tests/unit/compute/v2/test_server_image.py index 9b14428a27..b73cc763cb 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_image.py +++ b/openstackclient/tests/unit/compute/v2/test_server_image.py @@ -27,8 +27,9 @@ def setUp(self): super(TestServerImage, self).setUp() # Get a shortcut to the compute client ServerManager Mock - self.servers_mock = self.app.client_manager.compute.servers - self.servers_mock.reset_mock() + self.app.client_manager.sdk_connection = mock.Mock() + self.app.client_manager.sdk_connection.compute = mock.Mock() + self.sdk_client = self.app.client_manager.sdk_connection.compute # Get a shortcut to the image client ImageManager Mock self.images_mock = self.app.client_manager.image @@ -41,14 +42,14 @@ def setUp(self): self.methods = {} def setup_servers_mock(self, count): - servers = compute_fakes.FakeServer.create_servers( + servers = compute_fakes.FakeServer.create_sdk_servers( attrs=self.attrs, methods=self.methods, count=count, ) - # This is the return value for utils.find_resource() - self.servers_mock.get = compute_fakes.FakeServer.get_servers( + # This is the return value for compute_client.find_server() + self.sdk_client.find_server = compute_fakes.FakeServer.get_servers( servers, 0, ) @@ -104,8 +105,8 @@ def setup_images_mock(self, count, servers=None): ) self.images_mock.find_image = mock.Mock(side_effect=images) - self.servers_mock.create_image = mock.Mock( - return_value=images[0].id, + self.sdk_client.create_server_image = mock.Mock( + return_value=images[0], ) return images @@ -126,8 +127,7 @@ def test_server_image_create_defaults(self): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServerManager.create_image(server, image_name, metadata=) - self.servers_mock.create_image.assert_called_with( + self.sdk_client.create_server_image.assert_called_with( servers[0].id, servers[0].name, None, @@ -157,8 +157,7 @@ def test_server_image_create_options(self): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServerManager.create_image(server, image_name, metadata=) - self.servers_mock.create_image.assert_called_with( + self.sdk_client.create_server_image.assert_called_with( servers[0].id, 'img-nam', {'key': 'value'}, @@ -188,8 +187,7 @@ def test_server_create_image_wait_fail(self, mock_wait_for_status): parsed_args, ) - # ServerManager.create_image(server, image_name, metadata=) - self.servers_mock.create_image.assert_called_with( + self.sdk_client.create_server_image.assert_called_with( servers[0].id, servers[0].name, None, @@ -221,8 +219,7 @@ def test_server_create_image_wait_ok(self, mock_wait_for_status): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServerManager.create_image(server, image_name, metadata=) - self.servers_mock.create_image.assert_called_with( + self.sdk_client.create_server_image.assert_called_with( servers[0].id, servers[0].name, None, diff --git a/releasenotes/notes/migrate-create-server-image-to-sdk-e3d8077ffe05bb3d.yaml b/releasenotes/notes/migrate-create-server-image-to-sdk-e3d8077ffe05bb3d.yaml new file mode 100644 index 0000000000..20f8d55018 --- /dev/null +++ b/releasenotes/notes/migrate-create-server-image-to-sdk-e3d8077ffe05bb3d.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Migrate openstack server image create from novaclient to sdk. From 690e9a13a232f522162adc109d32c8eee864814e Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 17 Nov 2021 10:28:40 +0000 Subject: [PATCH 008/697] image: Remove dead test helper methods These haven't been used since we switched the image commands from glanceclient to openstacksdk. There's more cleanup to be done here but that can be done later. Change-Id: I3de1f24323886b122b3a30660fb3de18eb7014e9 Signed-off-by: Stephen Finucane --- openstackclient/tests/unit/image/v1/fakes.py | 31 ---- openstackclient/tests/unit/image/v2/fakes.py | 173 ------------------- 2 files changed, 204 deletions(-) diff --git a/openstackclient/tests/unit/image/v1/fakes.py b/openstackclient/tests/unit/image/v1/fakes.py index add3978d1d..59ae5f7a2d 100644 --- a/openstackclient/tests/unit/image/v1/fakes.py +++ b/openstackclient/tests/unit/image/v1/fakes.py @@ -11,7 +11,6 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -# from unittest import mock import uuid @@ -25,36 +24,6 @@ image_id = 'im1' image_name = 'graven' -image_owner = 'baal' -image_protected = False -image_public = True -image_properties = { - 'Alpha': 'a', - 'Beta': 'b', - 'Gamma': 'g', -} -image_properties_str = "Alpha='a', Beta='b', Gamma='g'" -image_data = 'line 1\nline 2\n' -image_size = 0 - -IMAGE = { - 'id': image_id, - 'name': image_name, - 'container_format': '', - 'disk_format': '', - 'owner': image_owner, - 'min_disk': 0, - 'min_ram': 0, - 'is_public': image_public, - 'protected': image_protected, - 'properties': image_properties, - 'size': image_size, -} - -IMAGE_columns = tuple(sorted(IMAGE)) -IMAGE_output = dict(IMAGE) -IMAGE_output['properties'] = image_properties_str -IMAGE_data = tuple((IMAGE_output[x] for x in sorted(IMAGE_output))) class FakeImagev1Client(object): diff --git a/openstackclient/tests/unit/image/v2/fakes.py b/openstackclient/tests/unit/image/v2/fakes.py index 0d83f98b95..49ce400d92 100644 --- a/openstackclient/tests/unit/image/v2/fakes.py +++ b/openstackclient/tests/unit/image/v2/fakes.py @@ -11,16 +11,13 @@ # 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 copy import random from unittest import mock import uuid from openstack.image.v2 import image from openstack.image.v2 import member -from osc_lib.cli import format_columns from openstackclient.tests.unit import fakes from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes @@ -28,121 +25,6 @@ image_id = '0f41529e-7c12-4de8-be2d-181abb825b3c' image_name = 'graven' -image_owner = 'baal' -image_protected = False -image_visibility = 'public' -image_tags = [] -image_size = 0 - -IMAGE = { - 'id': image_id, - 'name': image_name, - 'owner': image_owner, - 'protected': image_protected, - 'visibility': image_visibility, - 'tags': image_tags, - 'size': image_size -} - -IMAGE_columns = tuple(sorted(IMAGE)) -IMAGE_data = tuple((IMAGE[x] for x in sorted(IMAGE))) - -IMAGE_SHOW = copy.copy(IMAGE) -IMAGE_SHOW['tags'] = format_columns.ListColumn(IMAGE_SHOW['tags']) -IMAGE_SHOW_data = tuple((IMAGE_SHOW[x] for x in sorted(IMAGE_SHOW))) - -# Just enough v2 schema to do some testing -IMAGE_schema = { - "additionalProperties": { - "type": "string" - }, - "name": "image", - "links": [ - { - "href": "{self}", - "rel": "self" - }, - { - "href": "{file}", - "rel": "enclosure" - }, - { - "href": "{schema}", - "rel": "describedby" - } - ], - "properties": { - "id": { - "pattern": "^([0-9a-fA-F]){8}-([0-9a-fA-F]){4}-([0-9a-fA-F]){4}-([0-9a-fA-F]){4}-([0-9a-fA-F]){12}$", # noqa - "type": "string", - "description": "An identifier for the image" - }, - "name": { - "type": [ - "null", - "string" - ], - "description": "Descriptive name for the image", - "maxLength": 255 - }, - "owner": { - "type": [ - "null", - "string" - ], - "description": "Owner of the image", - "maxLength": 255 - }, - "protected": { - "type": "boolean", - "description": "If true, image will not be deletable." - }, - "self": { - "type": "string", - "description": "(READ-ONLY)" - }, - "schema": { - "type": "string", - "description": "(READ-ONLY)" - }, - "size": { - "type": [ - "null", - "integer", - "string" - ], - "description": "Size of image file in bytes (READ-ONLY)" - }, - "status": { - "enum": [ - "queued", - "saving", - "active", - "killed", - "deleted", - "pending_delete" - ], - "type": "string", - "description": "Status of the image (READ-ONLY)" - }, - "tags": { - "items": { - "type": "string", - "maxLength": 255 - }, - "type": "array", - "description": "List of strings related to the image" - }, - "visibility": { - "enum": [ - "public", - "private" - ], - "type": "string", - "description": "Scope of image accessibility" - }, - } -} class FakeImagev2Client(object): @@ -231,61 +113,6 @@ def create_images(attrs=None, count=2): return images - @staticmethod - def get_images(images=None, count=2): - """Get an iterable MagicMock object with a list of faked images. - - If images list is provided, then initialize the Mock object with the - list. Otherwise create one. - - :param List images: - A list of FakeResource objects faking images - :param Integer count: - The number of images to be faked - :return: - An iterable Mock object with side_effect set to a list of faked - images - """ - if images is None: - images = FakeImage.create_images(count) - - return mock.Mock(side_effect=images) - - @staticmethod - def get_image_columns(image=None): - """Get the image columns from a faked image object. - - :param image: - A FakeResource objects faking image - :return: - A tuple which may include the following keys: - ('id', 'name', 'owner', 'protected', 'visibility', 'tags') - """ - if image is not None: - return tuple(sorted(image)) - return IMAGE_columns - - @staticmethod - def get_image_data(image=None): - """Get the image data from a faked image object. - - :param image: - A FakeResource objects faking image - :return: - A tuple which may include the following values: - ('image-123', 'image-foo', 'admin', False, 'public', 'bar, baz') - """ - data_list = [] - if image is not None: - for x in sorted(image.keys()): - if x == 'tags': - # The 'tags' should be format_list - data_list.append( - format_columns.ListColumn(getattr(image, x))) - else: - data_list.append(getattr(image, x)) - return tuple(data_list) - @staticmethod def create_one_image_member(attrs=None): """Create a fake image member. From 2135a9ea05c79a11185ca87f6bb5ade3b71501bb Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 17 Nov 2021 10:35:51 +0000 Subject: [PATCH 009/697] image: Remove FakeImage test helper We're no longer creating fake versions of glanceclient's 'Resource' object but rather openstacksdk objects. As such, there's no point nesting things under a fake resource class. Change-Id: I39cd5302622f4542db9eebcccfad0cb90d077441 Signed-off-by: Stephen Finucane --- .../tests/unit/common/test_project_purge.py | 2 +- .../tests/unit/compute/v2/test_aggregate.py | 2 +- .../tests/unit/compute/v2/test_server.py | 28 ++-- .../unit/compute/v2/test_server_backup.py | 13 +- .../unit/compute/v2/test_server_image.py | 4 +- openstackclient/tests/unit/image/v1/fakes.py | 74 +++++----- .../tests/unit/image/v1/test_image.py | 11 +- openstackclient/tests/unit/image/v2/fakes.py | 133 ++++++++--------- .../tests/unit/image/v2/test_image.py | 134 +++++++++--------- .../tests/unit/volume/v1/test_volume.py | 4 +- .../tests/unit/volume/v2/test_volume.py | 4 +- 11 files changed, 187 insertions(+), 222 deletions(-) diff --git a/openstackclient/tests/unit/common/test_project_purge.py b/openstackclient/tests/unit/common/test_project_purge.py index adc48ce26f..5199093ce3 100644 --- a/openstackclient/tests/unit/common/test_project_purge.py +++ b/openstackclient/tests/unit/common/test_project_purge.py @@ -70,7 +70,7 @@ class TestProjectPurge(TestProjectPurgeInit): project = identity_fakes.FakeProject.create_one_project() server = compute_fakes.FakeServer.create_one_server() - image = image_fakes.FakeImage.create_one_image() + image = image_fakes.create_one_image() volume = volume_fakes.FakeVolume.create_one_volume() backup = volume_fakes.FakeBackup.create_one_backup() snapshot = volume_fakes.FakeSnapshot.create_one_snapshot() diff --git a/openstackclient/tests/unit/compute/v2/test_aggregate.py b/openstackclient/tests/unit/compute/v2/test_aggregate.py index 7c4fe5cbd3..071f2a3027 100644 --- a/openstackclient/tests/unit/compute/v2/test_aggregate.py +++ b/openstackclient/tests/unit/compute/v2/test_aggregate.py @@ -552,7 +552,7 @@ def test_aggregate_unset_no_option(self): class TestAggregateCacheImage(TestAggregate): - images = image_fakes.FakeImage.create_images(count=2) + images = image_fakes.create_images(count=2) def setUp(self): super(TestAggregateCacheImage, self).setUp() diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 9623cb0abf..f7ed4b16a4 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -1168,7 +1168,7 @@ def setUp(self): self.servers_mock.create.return_value = self.new_server - self.image = image_fakes.FakeImage.create_one_image() + self.image = image_fakes.create_one_image() self.find_image_mock.return_value = self.image self.get_image_mock.return_value = self.image @@ -2918,7 +2918,7 @@ def test_server_create_image_property(self): 'hypervisor_type': 'qemu', } - _image = image_fakes.FakeImage.create_one_image(image_info) + _image = image_fakes.create_one_image(image_info) self.images_mock.return_value = [_image] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -2974,7 +2974,7 @@ def test_server_create_image_property_multi(self): 'hypervisor_type': 'qemu', 'hw_disk_bus': 'ide', } - _image = image_fakes.FakeImage.create_one_image(image_info) + _image = image_fakes.create_one_image(image_info) self.images_mock.return_value = [_image] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -3031,7 +3031,7 @@ def test_server_create_image_property_missed(self): 'hw_disk_bus': 'ide', } - _image = image_fakes.FakeImage.create_one_image(image_info) + _image = image_fakes.create_one_image(image_info) self.images_mock.return_value = [_image] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -3063,8 +3063,8 @@ def test_server_create_image_property_with_image_list(self): } } - target_image = image_fakes.FakeImage.create_one_image(image_info) - another_image = image_fakes.FakeImage.create_one_image({}) + target_image = image_fakes.create_one_image(image_info) + another_image = image_fakes.create_one_image({}) self.images_mock.return_value = [target_image, another_image] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -4102,7 +4102,7 @@ def setUp(self): self.servers = self.setup_servers_mock(3) self.servers_mock.list.return_value = self.servers - self.image = image_fakes.FakeImage.create_one_image() + self.image = image_fakes.create_one_image() # self.images_mock.return_value = [self.image] self.find_image_mock.return_value = self.image @@ -6021,7 +6021,7 @@ def setUp(self): super(TestServerRebuild, self).setUp() # Return value for utils.find_resource for image - self.image = image_fakes.FakeImage.create_one_image() + self.image = image_fakes.create_one_image() self.get_image_mock.return_value = self.image # Fake the rebuilt new server. @@ -6051,7 +6051,7 @@ def setUp(self): def test_rebuild_with_image_name(self): image_name = 'my-custom-image' - user_image = image_fakes.FakeImage.create_one_image( + user_image = image_fakes.create_one_image( attrs={'name': image_name}) self.find_image_mock.return_value = user_image @@ -6600,7 +6600,7 @@ class TestEvacuateServer(TestServer): def setUp(self): super(TestEvacuateServer, self).setUp() # Return value for utils.find_resource for image - self.image = image_fakes.FakeImage.create_one_image() + self.image = image_fakes.create_one_image() self.images_mock.get.return_value = self.image # Fake the rebuilt new server. @@ -6794,7 +6794,7 @@ def setUp(self): super(TestServerRescue, self).setUp() # Return value for utils.find_resource for image - self.image = image_fakes.FakeImage.create_one_image() + self.image = image_fakes.create_one_image() self.get_image_mock.return_value = self.image new_server = compute_fakes.FakeServer.create_one_server() @@ -6835,7 +6835,7 @@ def test_rescue_with_current_image(self): self.server.rescue.assert_called_with(image=None, password=None) def test_rescue_with_new_image(self): - new_image = image_fakes.FakeImage.create_one_image() + new_image = image_fakes.create_one_image() self.find_image_mock.return_value = new_image arglist = [ '--image', new_image.id, @@ -7950,7 +7950,7 @@ class TestServerShow(TestServer): def setUp(self): super(TestServerShow, self).setUp() - self.image = image_fakes.FakeImage.create_one_image() + self.image = image_fakes.create_one_image() self.flavor = compute_fakes.FakeFlavor.create_one_flavor() self.topology = { 'nodes': [{'vcpu_set': [0, 1]}, {'vcpu_set': [2, 3]}], @@ -8540,7 +8540,7 @@ def test_prep_server_detail(self, find_resource): # - The first time, return server info. # - The second time, return image info. # - The third time, return flavor info. - _image = image_fakes.FakeImage.create_one_image() + _image = image_fakes.create_one_image() _flavor = compute_fakes.FakeFlavor.create_one_flavor() server_info = { 'image': {u'id': _image.id}, diff --git a/openstackclient/tests/unit/compute/v2/test_server_backup.py b/openstackclient/tests/unit/compute/v2/test_server_backup.py index 1644baae00..1a5e0a1225 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_backup.py +++ b/openstackclient/tests/unit/compute/v2/test_server_backup.py @@ -91,7 +91,7 @@ def setUp(self): def setup_images_mock(self, count, servers=None): if servers: - images = image_fakes.FakeImage.create_images( + images = image_fakes.create_images( attrs={ 'name': servers[0].name, 'status': 'active', @@ -99,7 +99,7 @@ def setup_images_mock(self, count, servers=None): count=count, ) else: - images = image_fakes.FakeImage.create_images( + images = image_fakes.create_images( attrs={ 'status': 'active', }, @@ -178,15 +178,6 @@ def test_server_backup_create_options(self): def test_server_backup_wait_fail(self, mock_wait_for_status): servers = self.setup_servers_mock(count=1) images = self.setup_images_mock(count=1, servers=servers) -# images = image_fakes.FakeImage.create_images( -# attrs={ -# 'name': servers[0].name, -# 'status': 'active', -# }, -# count=1, -# ) -# -# self.images_mock.find_image.return_value = images[0] self.images_mock.get_image = mock.Mock( side_effect=images[0], ) diff --git a/openstackclient/tests/unit/compute/v2/test_server_image.py b/openstackclient/tests/unit/compute/v2/test_server_image.py index 9b14428a27..e17401691a 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_image.py +++ b/openstackclient/tests/unit/compute/v2/test_server_image.py @@ -88,7 +88,7 @@ def setUp(self): def setup_images_mock(self, count, servers=None): if servers: - images = image_fakes.FakeImage.create_images( + images = image_fakes.create_images( attrs={ 'name': servers[0].name, 'status': 'active', @@ -96,7 +96,7 @@ def setup_images_mock(self, count, servers=None): count=count, ) else: - images = image_fakes.FakeImage.create_images( + images = image_fakes.create_images( attrs={ 'status': 'active', }, diff --git a/openstackclient/tests/unit/image/v1/fakes.py b/openstackclient/tests/unit/image/v1/fakes.py index 59ae5f7a2d..3097a42f59 100644 --- a/openstackclient/tests/unit/image/v1/fakes.py +++ b/openstackclient/tests/unit/image/v1/fakes.py @@ -22,10 +22,6 @@ from openstackclient.tests.unit.volume.v1 import fakes as volume_fakes -image_id = 'im1' -image_name = 'graven' - - class FakeImagev1Client(object): def __init__(self, **kwargs): @@ -51,40 +47,36 @@ def setUp(self): ) -class FakeImage(object): - """Fake one or more images.""" - - @staticmethod - def create_one_image(attrs=None): - """Create a fake image. - - :param Dictionary attrs: - A dictionary with all attrbutes of image - :return: - A FakeResource object with id, name, owner, protected, - visibility and tags attrs - """ - attrs = attrs or {} - - # Set default attribute - image_info = { - 'id': str(uuid.uuid4()), - 'name': 'image-name' + uuid.uuid4().hex, - 'owner': 'image-owner' + uuid.uuid4().hex, - 'container_format': '', - 'disk_format': '', - 'min_disk': 0, - 'min_ram': 0, - 'is_public': True, - 'protected': False, - 'properties': { - 'Alpha': 'a', - 'Beta': 'b', - 'Gamma': 'g'}, - 'status': 'status' + uuid.uuid4().hex - } - - # Overwrite default attributes if there are some attributes set - image_info.update(attrs) - - return image.Image(**image_info) +def create_one_image(attrs=None): + """Create a fake image. + + :param Dictionary attrs: + A dictionary with all attrbutes of image + :return: + A FakeResource object with id, name, owner, protected, + visibility and tags attrs + """ + attrs = attrs or {} + + # Set default attribute + image_info = { + 'id': str(uuid.uuid4()), + 'name': 'image-name' + uuid.uuid4().hex, + 'owner': 'image-owner' + uuid.uuid4().hex, + 'container_format': '', + 'disk_format': '', + 'min_disk': 0, + 'min_ram': 0, + 'is_public': True, + 'protected': False, + 'properties': { + 'Alpha': 'a', + 'Beta': 'b', + 'Gamma': 'g'}, + 'status': 'status' + uuid.uuid4().hex + } + + # Overwrite default attributes if there are some attributes set + image_info.update(attrs) + + return image.Image(**image_info) diff --git a/openstackclient/tests/unit/image/v1/test_image.py b/openstackclient/tests/unit/image/v1/test_image.py index 5c69bf0f87..06519800ce 100644 --- a/openstackclient/tests/unit/image/v1/test_image.py +++ b/openstackclient/tests/unit/image/v1/test_image.py @@ -34,7 +34,7 @@ def setUp(self): class TestImageCreate(TestImage): - new_image = image_fakes.FakeImage.create_one_image() + new_image = image_fakes.create_one_image() columns = ( 'container_format', 'disk_format', @@ -210,7 +210,7 @@ def test_image_create_file(self, mock_open): class TestImageDelete(TestImage): - _image = image_fakes.FakeImage.create_one_image() + _image = image_fakes.create_one_image() def setUp(self): super(TestImageDelete, self).setUp() @@ -239,7 +239,7 @@ def test_image_delete_no_options(self): class TestImageList(TestImage): - _image = image_fakes.FakeImage.create_one_image() + _image = image_fakes.create_one_image() columns = ( 'ID', @@ -443,7 +443,7 @@ def test_image_list_sort_option(self, si_mock): class TestImageSet(TestImage): - _image = image_fakes.FakeImage.create_one_image() + _image = image_fakes.create_one_image() def setUp(self): super(TestImageSet, self).setUp() @@ -682,8 +682,7 @@ def test_image_set_numeric_options_to_zero(self): class TestImageShow(TestImage): - _image = image_fakes.FakeImage.create_one_image( - attrs={'size': 2000}) + _image = image_fakes.create_one_image(attrs={'size': 2000}) columns = ( 'container_format', 'disk_format', diff --git a/openstackclient/tests/unit/image/v2/fakes.py b/openstackclient/tests/unit/image/v2/fakes.py index 49ce400d92..910bd726ff 100644 --- a/openstackclient/tests/unit/image/v2/fakes.py +++ b/openstackclient/tests/unit/image/v2/fakes.py @@ -23,9 +23,6 @@ from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes from openstackclient.tests.unit import utils -image_id = '0f41529e-7c12-4de8-be2d-181abb825b3c' -image_name = 'graven' - class FakeImagev2Client(object): @@ -63,75 +60,67 @@ def setUp(self): ) -class FakeImage(object): - """Fake one or more images. +def create_one_image(attrs=None): + """Create a fake image. + + :param attrs: A dictionary with all attributes of image + :type attrs: dict + :return: A fake Image object. + :rtype: `openstack.image.v2.image.Image` + """ + attrs = attrs or {} + + # Set default attribute + image_info = { + 'id': str(uuid.uuid4()), + 'name': 'image-name' + uuid.uuid4().hex, + 'owner_id': 'image-owner' + uuid.uuid4().hex, + 'is_protected': bool(random.choice([0, 1])), + 'visibility': random.choice(['public', 'private']), + 'tags': [uuid.uuid4().hex for r in range(2)], + } + + # Overwrite default attributes if there are some attributes set + image_info.update(attrs) + + return image.Image(**image_info) + + +def create_images(attrs=None, count=2): + """Create multiple fake images. - TODO(xiexs): Currently, only image API v2 is supported by this class. + :param attrs: A dictionary with all attributes of image + :type attrs: dict + :param count: The number of images to be faked + :type count: int + :return: A list of fake Image objects + :rtype: list """ + images = [] + for n in range(0, count): + images.append(create_one_image(attrs)) + + return images + + +def create_one_image_member(attrs=None): + """Create a fake image member. + + :param attrs: A dictionary with all attributes of image member + :type attrs: dict + :return: A fake Member object. + :rtype: `openstack.image.v2.member.Member` + """ + attrs = attrs or {} + + # Set default attribute + image_member_info = { + 'member_id': 'member-id-' + uuid.uuid4().hex, + 'image_id': 'image-id-' + uuid.uuid4().hex, + 'status': 'pending', + } + + # Overwrite default attributes if there are some attributes set + image_member_info.update(attrs) - @staticmethod - def create_one_image(attrs=None): - """Create a fake image. - - :param Dictionary attrs: - A dictionary with all attrbutes of image - :return: - A FakeResource object with id, name, owner, protected, - visibility, tags and size attrs - """ - attrs = attrs or {} - - # Set default attribute - image_info = { - 'id': str(uuid.uuid4()), - 'name': 'image-name' + uuid.uuid4().hex, - 'owner_id': 'image-owner' + uuid.uuid4().hex, - 'is_protected': bool(random.choice([0, 1])), - 'visibility': random.choice(['public', 'private']), - 'tags': [uuid.uuid4().hex for r in range(2)], - } - - # Overwrite default attributes if there are some attributes set - image_info.update(attrs) - - return image.Image(**image_info) - - @staticmethod - def create_images(attrs=None, count=2): - """Create multiple fake images. - - :param Dictionary attrs: - A dictionary with all attributes of image - :param Integer count: - The number of images to be faked - :return: - A list of FakeResource objects - """ - images = [] - for n in range(0, count): - images.append(FakeImage.create_one_image(attrs)) - - return images - - @staticmethod - def create_one_image_member(attrs=None): - """Create a fake image member. - - :param Dictionary attrs: - A dictionary with all attributes of image member - :return: - A FakeResource object with member_id, image_id and so on - """ - attrs = attrs or {} - - # Set default attribute - image_member_info = { - 'member_id': 'member-id-' + uuid.uuid4().hex, - 'image_id': 'image-id-' + uuid.uuid4().hex, - 'status': 'pending', - } - - # Overwrite default attributes if there are some attributes set - image_member_info.update(attrs) - - return member.Member(**image_member_info) + return member.Member(**image_member_info) diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py index 35af6799f6..f15463738a 100644 --- a/openstackclient/tests/unit/image/v2/test_image.py +++ b/openstackclient/tests/unit/image/v2/test_image.py @@ -11,7 +11,6 @@ # 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 copy import io @@ -52,7 +51,7 @@ def setUp(self): self.domain_mock.reset_mock() def setup_images_mock(self, count): - images = image_fakes.FakeImage.create_images(count=count) + images = image_fakes.create_images(count=count) return images @@ -65,7 +64,7 @@ class TestImageCreate(TestImage): def setUp(self): super(TestImageCreate, self).setUp() - self.new_image = image_fakes.FakeImage.create_one_image() + self.new_image = image_fakes.create_one_image() self.client.create_image.return_value = self.new_image self.project_mock.get.return_value = self.project @@ -182,7 +181,7 @@ def test_image_create_with_unexist_project(self): '--protected', '--private', '--project', 'unexist_owner', - image_fakes.image_name, + 'graven', ] verifylist = [ ('container_format', 'ovf'), @@ -194,7 +193,7 @@ def test_image_create_with_unexist_project(self): ('public', False), ('private', True), ('project', 'unexist_owner'), - ('name', image_fakes.image_name), + ('name', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -302,8 +301,8 @@ class TestAddProjectToImage(TestImage): project = identity_fakes.FakeProject.create_one_project() domain = identity_fakes.FakeDomain.create_one_domain() - _image = image_fakes.FakeImage.create_one_image() - new_member = image_fakes.FakeImage.create_one_image_member( + _image = image_fakes.create_one_image() + new_member = image_fakes.create_one_image_member( attrs={'image_id': _image.id, 'member_id': project.id} ) @@ -435,7 +434,7 @@ def test_image_delete_multi_images(self): def test_image_delete_multi_images_exception(self): - images = image_fakes.FakeImage.create_images(count=2) + images = image_fakes.create_images(count=2) arglist = [ images[0].id, images[1].id, @@ -467,7 +466,7 @@ def test_image_delete_multi_images_exception(self): class TestImageList(TestImage): - _image = image_fakes.FakeImage.create_one_image() + _image = image_fakes.create_one_image() columns = ( 'ID', @@ -786,18 +785,13 @@ def test_image_list_project_option(self): @mock.patch('osc_lib.utils.find_resource') def test_image_list_marker_option(self, fr_mock): - # tangchen: Since image_fakes.IMAGE is a dict, it cannot offer a .id - # operation. Will fix this by using FakeImage class instead - # of IMAGE dict. self.client.find_image = mock.Mock(return_value=self._image) -# fr_mock.return_value = mock.Mock() -# fr_mock.return_value.id = image_fakes.image_id arglist = [ - '--marker', image_fakes.image_name, + '--marker', 'graven', ] verifylist = [ - ('marker', image_fakes.image_name), + ('marker', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -806,7 +800,7 @@ def test_image_list_marker_option(self, fr_mock): marker=self._image.id, ) - self.client.find_image.assert_called_with(image_fakes.image_name) + self.client.find_image.assert_called_with('graven') def test_image_list_name_option(self): arglist = [ @@ -869,8 +863,8 @@ def test_image_list_tag_option(self): class TestListImageProjects(TestImage): project = identity_fakes.FakeProject.create_one_project() - _image = image_fakes.FakeImage.create_one_image() - member = image_fakes.FakeImage.create_one_image_member( + _image = image_fakes.create_one_image() + member = image_fakes.create_one_image_member( attrs={'image_id': _image.id, 'member_id': project.id} ) @@ -920,7 +914,7 @@ class TestRemoveProjectImage(TestImage): def setUp(self): super(TestRemoveProjectImage, self).setUp() - self._image = image_fakes.FakeImage.create_one_image() + self._image = image_fakes.create_one_image() # This is the return value for utils.find_resource() self.client.find_image.return_value = self._image @@ -979,7 +973,7 @@ class TestImageSet(TestImage): project = identity_fakes.FakeProject.create_one_project() domain = identity_fakes.FakeDomain.create_one_domain() - _image = image_fakes.FakeImage.create_one_image({'tags': []}) + _image = image_fakes.create_one_image({'tags': []}) def setUp(self): super(TestImageSet, self).setUp() @@ -999,10 +993,10 @@ def setUp(self): def test_image_set_no_options(self): arglist = [ - image_fakes.image_id, + '0f41529e-7c12-4de8-be2d-181abb825b3c', ] verifylist = [ - ('image', image_fakes.image_id) + ('image', '0f41529e-7c12-4de8-be2d-181abb825b3c') ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1013,8 +1007,8 @@ def test_image_set_no_options(self): self.image_members_mock.update.assert_not_called() def test_image_set_membership_option_accept(self): - membership = image_fakes.FakeImage.create_one_image_member( - attrs={'image_id': image_fakes.image_id, + membership = image_fakes.create_one_image_member( + attrs={'image_id': '0f41529e-7c12-4de8-be2d-181abb825b3c', 'member_id': self.project.id} ) self.client.update_member.return_value = membership @@ -1044,21 +1038,21 @@ def test_image_set_membership_option_accept(self): self.client.update_image.assert_called_with(self._image.id) def test_image_set_membership_option_reject(self): - membership = image_fakes.FakeImage.create_one_image_member( - attrs={'image_id': image_fakes.image_id, + membership = image_fakes.create_one_image_member( + attrs={'image_id': '0f41529e-7c12-4de8-be2d-181abb825b3c', 'member_id': self.project.id} ) self.client.update_member.return_value = membership arglist = [ '--reject', - image_fakes.image_id, + '0f41529e-7c12-4de8-be2d-181abb825b3c', ] verifylist = [ ('accept', False), ('reject', True), ('pending', False), - ('image', image_fakes.image_id) + ('image', '0f41529e-7c12-4de8-be2d-181abb825b3c') ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1075,21 +1069,21 @@ def test_image_set_membership_option_reject(self): self.client.update_image.assert_called_with(self._image.id) def test_image_set_membership_option_pending(self): - membership = image_fakes.FakeImage.create_one_image_member( - attrs={'image_id': image_fakes.image_id, + membership = image_fakes.create_one_image_member( + attrs={'image_id': '0f41529e-7c12-4de8-be2d-181abb825b3c', 'member_id': self.project.id} ) self.client.update_member.return_value = membership arglist = [ '--pending', - image_fakes.image_id, + '0f41529e-7c12-4de8-be2d-181abb825b3c', ] verifylist = [ ('accept', False), ('reject', False), ('pending', True), - ('image', image_fakes.image_id) + ('image', '0f41529e-7c12-4de8-be2d-181abb825b3c') ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1149,11 +1143,11 @@ def test_image_set_with_unexist_project(self): arglist = [ '--project', 'unexist_owner', - image_fakes.image_id, + '0f41529e-7c12-4de8-be2d-181abb825b3c', ] verifylist = [ ('project', 'unexist_owner'), - ('image', image_fakes.image_id), + ('image', '0f41529e-7c12-4de8-be2d-181abb825b3c'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1165,14 +1159,14 @@ def test_image_set_bools1(self): arglist = [ '--protected', '--private', - image_fakes.image_name, + 'graven', ] verifylist = [ ('protected', True), ('unprotected', False), ('public', False), ('private', True), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1193,14 +1187,14 @@ def test_image_set_bools2(self): arglist = [ '--unprotected', '--public', - image_fakes.image_name, + 'graven', ] verifylist = [ ('protected', False), ('unprotected', True), ('public', True), ('private', False), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1221,11 +1215,11 @@ def test_image_set_properties(self): arglist = [ '--property', 'Alpha=1', '--property', 'Beta=2', - image_fakes.image_name, + 'graven', ] verifylist = [ ('properties', {'Alpha': '1', 'Beta': '2'}), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1250,7 +1244,7 @@ def test_image_set_fake_properties(self): '--os-distro', 'cpm', '--os-version', '2.2H', '--ramdisk-id', 'xyzpdq', - image_fakes.image_name, + 'graven', ] verifylist = [ ('architecture', 'z80'), @@ -1259,7 +1253,7 @@ def test_image_set_fake_properties(self): ('os_distro', 'cpm'), ('os_version', '2.2H'), ('ramdisk_id', 'xyzpdq'), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1283,11 +1277,11 @@ def test_image_set_fake_properties(self): def test_image_set_tag(self): arglist = [ '--tag', 'test-tag', - image_fakes.image_name, + 'graven', ] verifylist = [ ('tags', ['test-tag']), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1307,11 +1301,11 @@ def test_image_set_activate(self): arglist = [ '--tag', 'test-tag', '--activate', - image_fakes.image_name, + 'graven', ] verifylist = [ ('tags', ['test-tag']), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1335,11 +1329,11 @@ def test_image_set_deactivate(self): arglist = [ '--tag', 'test-tag', '--deactivate', - image_fakes.image_name, + 'graven', ] verifylist = [ ('tags', ['test-tag']), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1365,11 +1359,11 @@ def test_image_set_tag_merge(self): self.client.find_image.return_value = old_image arglist = [ '--tag', 'test-tag', - image_fakes.image_name, + 'graven', ] verifylist = [ ('tags', ['test-tag']), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1391,11 +1385,11 @@ def test_image_set_tag_merge_dupe(self): self.client.find_image.return_value = old_image arglist = [ '--tag', 'old1', - image_fakes.image_name, + 'graven', ] verifylist = [ ('tags', ['old1']), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1415,11 +1409,11 @@ def test_image_set_dead_options(self): arglist = [ '--visibility', '1-mile', - image_fakes.image_name, + 'graven', ] verifylist = [ ('visibility', '1-mile'), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1431,12 +1425,12 @@ def test_image_set_numeric_options_to_zero(self): arglist = [ '--min-disk', '0', '--min-ram', '0', - image_fakes.image_name, + 'graven', ] verifylist = [ ('min_disk', 0), ('min_ram', 0), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1457,13 +1451,13 @@ def test_image_set_hidden(self): arglist = [ '--hidden', '--public', - image_fakes.image_name, + 'graven', ] verifylist = [ ('hidden', True), ('public', True), ('private', False), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1484,13 +1478,13 @@ def test_image_set_unhidden(self): arglist = [ '--unhidden', '--public', - image_fakes.image_name, + 'graven', ] verifylist = [ ('hidden', False), ('public', True), ('private', False), - ('image', image_fakes.image_name), + ('image', 'graven'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1510,10 +1504,10 @@ def test_image_set_unhidden(self): class TestImageShow(TestImage): - new_image = image_fakes.FakeImage.create_one_image( + new_image = image_fakes.create_one_image( attrs={'size': 1000}) - _data = image_fakes.FakeImage.create_one_image() + _data = image_fakes.create_one_image() columns = ( 'id', 'name', 'owner', 'protected', 'tags', 'visibility' @@ -1538,10 +1532,10 @@ def setUp(self): def test_image_show(self): arglist = [ - image_fakes.image_id, + '0f41529e-7c12-4de8-be2d-181abb825b3c', ] verifylist = [ - ('image', image_fakes.image_id), + ('image', '0f41529e-7c12-4de8-be2d-181abb825b3c'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1550,7 +1544,7 @@ def test_image_show(self): # data to be shown. columns, data = self.cmd.take_action(parsed_args) self.client.find_image.assert_called_with( - image_fakes.image_id, + '0f41529e-7c12-4de8-be2d-181abb825b3c', ignore_missing=False ) @@ -1592,7 +1586,7 @@ def setUp(self): attrs['hw_rng_model'] = 'virtio' attrs['prop'] = 'test' attrs['prop2'] = 'fake' - self.image = image_fakes.FakeImage.create_one_image(attrs) + self.image = image_fakes.create_one_image(attrs) self.client.find_image.return_value = self.image self.client.remove_tag.return_value = self.image @@ -1603,10 +1597,10 @@ def setUp(self): def test_image_unset_no_options(self): arglist = [ - image_fakes.image_id, + '0f41529e-7c12-4de8-be2d-181abb825b3c', ] verifylist = [ - ('image', image_fakes.image_id) + ('image', '0f41529e-7c12-4de8-be2d-181abb825b3c') ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1681,7 +1675,7 @@ def test_image_unset_mixed_option(self): class TestImageSave(TestImage): - image = image_fakes.FakeImage.create_one_image({}) + image = image_fakes.create_one_image({}) def setUp(self): super(TestImageSave, self).setUp() diff --git a/openstackclient/tests/unit/volume/v1/test_volume.py b/openstackclient/tests/unit/volume/v1/test_volume.py index 702f79ed83..584eca2a7b 100644 --- a/openstackclient/tests/unit/volume/v1/test_volume.py +++ b/openstackclient/tests/unit/volume/v1/test_volume.py @@ -317,7 +317,7 @@ def test_volume_create_properties(self): self.assertCountEqual(self.datalist, data) def test_volume_create_image_id(self): - image = image_fakes.FakeImage.create_one_image() + image = image_fakes.create_one_image() self.images_mock.get.return_value = image arglist = [ @@ -360,7 +360,7 @@ def test_volume_create_image_id(self): self.assertCountEqual(self.datalist, data) def test_volume_create_image_name(self): - image = image_fakes.FakeImage.create_one_image() + image = image_fakes.create_one_image() self.images_mock.get.return_value = image arglist = [ diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index 4aa6f906cc..ec82e6746b 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -221,7 +221,7 @@ def test_volume_create_properties(self): self.assertCountEqual(self.datalist, data) def test_volume_create_image_id(self): - image = image_fakes.FakeImage.create_one_image() + image = image_fakes.create_one_image() self.find_image_mock.return_value = image arglist = [ @@ -259,7 +259,7 @@ def test_volume_create_image_id(self): self.assertCountEqual(self.datalist, data) def test_volume_create_image_name(self): - image = image_fakes.FakeImage.create_one_image() + image = image_fakes.create_one_image() self.find_image_mock.return_value = image arglist = [ From 1feb676469f7ccd6a022027bf2e1ecee9cf6d548 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 17 Nov 2021 10:55:33 +0000 Subject: [PATCH 010/697] tests: Update fake image client in tests These clients are intended to fake out the old glanceclient client which we no longer use. They were only "working" because we weren't actually using any of the glancelclient-based stuff and were instead overriding everything within the tests. Move these overrides back to the main fake client and remove the crud. Change-Id: I92ee74a1df72a6dd23f9d2dc04342aab0cbd3210 Signed-off-by: Stephen Finucane --- openstackclient/tests/unit/image/v1/fakes.py | 8 +++--- .../tests/unit/image/v1/test_image.py | 6 +---- openstackclient/tests/unit/image/v2/fakes.py | 27 +++++++++++-------- .../tests/unit/image/v2/test_image.py | 23 +++++----------- 4 files changed, 28 insertions(+), 36 deletions(-) diff --git a/openstackclient/tests/unit/image/v1/fakes.py b/openstackclient/tests/unit/image/v1/fakes.py index 3097a42f59..164050c00e 100644 --- a/openstackclient/tests/unit/image/v1/fakes.py +++ b/openstackclient/tests/unit/image/v1/fakes.py @@ -22,11 +22,11 @@ from openstackclient.tests.unit.volume.v1 import fakes as volume_fakes -class FakeImagev1Client(object): +class FakeImagev1Client: def __init__(self, **kwargs): self.images = mock.Mock() - self.images.resource_class = fakes.FakeResource(None, {}) + self.auth_token = kwargs['token'] self.management_url = kwargs['endpoint'] self.version = 1.0 @@ -35,7 +35,7 @@ def __init__(self, **kwargs): class TestImagev1(utils.TestCommand): def setUp(self): - super(TestImagev1, self).setUp() + super().setUp() self.app.client_manager.image = FakeImagev1Client( endpoint=fakes.AUTH_URL, @@ -46,6 +46,8 @@ def setUp(self): token=fakes.AUTH_TOKEN, ) + self.client = self.app.client_manager.image + def create_one_image(attrs=None): """Create a fake image. diff --git a/openstackclient/tests/unit/image/v1/test_image.py b/openstackclient/tests/unit/image/v1/test_image.py index 06519800ce..6c65f9a395 100644 --- a/openstackclient/tests/unit/image/v1/test_image.py +++ b/openstackclient/tests/unit/image/v1/test_image.py @@ -25,11 +25,7 @@ class TestImage(image_fakes.TestImagev1): - def setUp(self): - super(TestImage, self).setUp() - - self.app.client_manager.image = mock.Mock() - self.client = self.app.client_manager.image + pass class TestImageCreate(TestImage): diff --git a/openstackclient/tests/unit/image/v2/fakes.py b/openstackclient/tests/unit/image/v2/fakes.py index 910bd726ff..a0eda6d23a 100644 --- a/openstackclient/tests/unit/image/v2/fakes.py +++ b/openstackclient/tests/unit/image/v2/fakes.py @@ -24,21 +24,26 @@ from openstackclient.tests.unit import utils -class FakeImagev2Client(object): +class FakeImagev2Client: def __init__(self, **kwargs): self.images = mock.Mock() - self.images.resource_class = fakes.FakeResource(None, {}) - self.image_members = mock.Mock() - self.image_members.resource_class = fakes.FakeResource(None, {}) - self.image_tags = mock.Mock() - self.image_tags.resource_class = fakes.FakeResource(None, {}) - + self.create_image = mock.Mock() + self.delete_image = mock.Mock() + self.update_image = mock.Mock() self.find_image = mock.Mock() - self.find_image.resource_class = fakes.FakeResource(None, {}) - self.get_image = mock.Mock() - self.get_image.resource_class = fakes.FakeResource(None, {}) + self.download_image = mock.Mock() + self.reactivate_image = mock.Mock() + self.deactivate_image = mock.Mock() + + self.members = mock.Mock() + self.add_member = mock.Mock() + self.remove_member = mock.Mock() + self.update_member = mock.Mock() + + self.remove_tag = mock.Mock() + self.auth_token = kwargs['token'] self.management_url = kwargs['endpoint'] self.version = 2.0 @@ -47,7 +52,7 @@ def __init__(self, **kwargs): class TestImagev2(utils.TestCommand): def setUp(self): - super(TestImagev2, self).setUp() + super().setUp() self.app.client_manager.image = FakeImagev2Client( endpoint=fakes.AUTH_URL, diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py index f15463738a..510976f7e8 100644 --- a/openstackclient/tests/unit/image/v2/test_image.py +++ b/openstackclient/tests/unit/image/v2/test_image.py @@ -32,18 +32,9 @@ class TestImage(image_fakes.TestImagev2): def setUp(self): super(TestImage, self).setUp() - # Get shortcuts to the Mocks in image client - # SDK proxy mock - self.app.client_manager.image = mock.Mock() + # Get shortcuts to mocked image client self.client = self.app.client_manager.image - self.client.remove_member = mock.Mock() - - self.client.create_image = mock.Mock() - self.client.update_image = mock.Mock() - self.image_members_mock = self.app.client_manager.image.image_members - self.image_tags_mock = self.app.client_manager.image.image_tags - # Get shortcut to the Mocks in identity client self.project_mock = self.app.client_manager.identity.projects self.project_mock.reset_mock() @@ -483,11 +474,7 @@ class TestImageList(TestImage): def setUp(self): super(TestImageList, self).setUp() - self.api_mock = mock.Mock() - self.api_mock.side_effect = [ - [self._image], [], - ] - self.client.images = self.api_mock + self.client.images.side_effect = [[self._image], []] # Get the command object to test self.cmd = image.ListImage(self.app, None) @@ -1003,8 +990,10 @@ def test_image_set_no_options(self): result = self.cmd.take_action(parsed_args) self.assertIsNone(result) - - self.image_members_mock.update.assert_not_called() + # we'll have called this but not set anything + self.app.client_manager.image.update_image.called_once_with( + self._image.id, + ) def test_image_set_membership_option_accept(self): membership = image_fakes.create_one_image_member( From 61fac5b79e2e4a4120046b2f830e4745bb383fb3 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 17 Nov 2021 11:31:55 +0000 Subject: [PATCH 011/697] image: Sanity check the 'SetImage' command This was a very difficult command to grok, due to the layering on of additional features over the years. Make this a little easier to follow by grouping related logic and making use of argparse features. Change-Id: I4e1a0aed09ea5d6a8c26ec3e888c9c7b6cefc25a Signed-off-by: Stephen Finucane --- openstackclient/image/v2/image.py | 90 ++++++++++--------- .../tests/unit/image/v2/test_image.py | 12 +-- 2 files changed, 53 insertions(+), 49 deletions(-) diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py index becb54f456..407c129294 100644 --- a/openstackclient/image/v2/image.py +++ b/openstackclient/image/v2/image.py @@ -1012,17 +1012,26 @@ def get_parser(self, prog_name): membership_group = parser.add_mutually_exclusive_group() membership_group.add_argument( "--accept", - action="store_true", + action="store_const", + const="accepted", + dest="membership", + default=None, help=_("Accept the image membership"), ) membership_group.add_argument( "--reject", - action="store_true", + action="store_const", + const="rejected", + dest="membership", + default=None, help=_("Reject the image membership"), ) membership_group.add_argument( "--pending", - action="store_true", + action="store_const", + const="pending", + dest="membership", + default=None, help=_("Reset the image membership to 'pending'"), ) @@ -1053,6 +1062,43 @@ def take_action(self, parsed_args): _("ERROR: --%s was given, which is an Image v1 option" " that is no longer supported in Image v2") % deadopt) + image = image_client.find_image( + parsed_args.image, ignore_missing=False, + ) + project_id = None + if parsed_args.project: + project_id = common.find_project( + identity_client, + parsed_args.project, + parsed_args.project_domain, + ).id + + # handle activation status changes + + activation_status = None + if parsed_args.deactivate or parsed_args.activate: + if parsed_args.deactivate: + image_client.deactivate_image(image.id) + activation_status = "deactivated" + if parsed_args.activate: + image_client.reactivate_image(image.id) + activation_status = "activated" + + # handle membership changes + + if parsed_args.membership: + # If a specific project is not passed, assume we want to update + # our own membership + if not project_id: + project_id = self.app.client_manager.auth_ref.project_id + image_client.update_member( + image=image.id, + member=project_id, + status=parsed_args.membership, + ) + + # handle everything else + kwargs = {} copy_attrs = ('architecture', 'container_format', 'disk_format', 'file', 'instance_id', 'kernel_id', 'locations', @@ -1089,48 +1135,12 @@ def take_action(self, parsed_args): kwargs['visibility'] = 'community' if parsed_args.shared: kwargs['visibility'] = 'shared' - project_id = None if parsed_args.project: - project_id = common.find_project( - identity_client, - parsed_args.project, - parsed_args.project_domain, - ).id + # We already did the project lookup above kwargs['owner_id'] = project_id - - image = image_client.find_image(parsed_args.image, - ignore_missing=False) - - # image = utils.find_resource( - # image_client.images, parsed_args.image) - - activation_status = None - if parsed_args.deactivate: - image_client.deactivate_image(image.id) - activation_status = "deactivated" - if parsed_args.activate: - image_client.reactivate_image(image.id) - activation_status = "activated" - - membership_group_args = ('accept', 'reject', 'pending') - membership_status = [status for status in membership_group_args - if getattr(parsed_args, status)] - if membership_status: - # If a specific project is not passed, assume we want to update - # our own membership - if not project_id: - project_id = self.app.client_manager.auth_ref.project_id - # The mutually exclusive group of the arg parser ensure we have at - # most one item in the membership_status list. - if membership_status[0] != 'pending': - membership_status[0] += 'ed' # Glance expects the past form - image_client.update_member( - image=image.id, member=project_id, status=membership_status[0]) - if parsed_args.tags: # Tags should be extended, but duplicates removed kwargs['tags'] = list(set(image.tags).union(set(parsed_args.tags))) - if parsed_args.hidden is not None: kwargs['is_hidden'] = parsed_args.hidden diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py index 510976f7e8..7ccc9f0fb4 100644 --- a/openstackclient/tests/unit/image/v2/test_image.py +++ b/openstackclient/tests/unit/image/v2/test_image.py @@ -1007,9 +1007,7 @@ def test_image_set_membership_option_accept(self): self._image.id, ] verifylist = [ - ('accept', True), - ('reject', False), - ('pending', False), + ('membership', 'accepted'), ('image', self._image.id) ] @@ -1038,9 +1036,7 @@ def test_image_set_membership_option_reject(self): '0f41529e-7c12-4de8-be2d-181abb825b3c', ] verifylist = [ - ('accept', False), - ('reject', True), - ('pending', False), + ('membership', 'rejected'), ('image', '0f41529e-7c12-4de8-be2d-181abb825b3c') ] @@ -1069,9 +1065,7 @@ def test_image_set_membership_option_pending(self): '0f41529e-7c12-4de8-be2d-181abb825b3c', ] verifylist = [ - ('accept', False), - ('reject', False), - ('pending', True), + ('membership', 'pending'), ('image', '0f41529e-7c12-4de8-be2d-181abb825b3c') ] From b3d09ffc37f6b577ce479a5203c6c882062fee74 Mon Sep 17 00:00:00 2001 From: JieonLee Date: Sun, 21 Nov 2021 05:05:25 +0000 Subject: [PATCH 012/697] Add missing command mapping in nova nova command: instance-action openstack command: server event show Change-Id: I8e5dad90cfd28b1f0d65be688651918869f679e4 --- doc/source/cli/data/nova.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/cli/data/nova.csv b/doc/source/cli/data/nova.csv index dd8992f68c..0ab2b2fee4 100644 --- a/doc/source/cli/data/nova.csv +++ b/doc/source/cli/data/nova.csv @@ -46,7 +46,7 @@ hypervisor-show,hypervisor show,Display the details of the specified hypervisor. hypervisor-stats,hypervisor stats show,Get hypervisor statistics over all compute nodes. hypervisor-uptime,,Display the uptime of the specified hypervisor. image-create,server image create,Create a new image by taking a snapshot of a running server. -instance-action,,Show an action. +instance-action,server event show,Show an action. instance-action-list,,List actions on a server. instance-usage-audit-log,,List/Get server usage audits. interface-attach,server add port / server add floating ip / server add fixed ip,Attach a network interface to a server. From 3078a0a121743c387d83d7f27ce3d8fd8fbb4ccf Mon Sep 17 00:00:00 2001 From: Diwei Zhu Date: Thu, 28 Oct 2021 23:25:52 +0000 Subject: [PATCH 013/697] Switch command server add volume to sdk. File tests.unit.volume.v2.fakes is modified to provide sdk volume fakes. File tests.unit.compute.v2.fakes is modified to provide sdk volume attachment fakes. For test, setup_sdk_volumes_mock() method is created so that volumes are created in similar way as servers are created. Change-Id: I290ba83b6ba27a1377ab73fd0ae06ecced25efd1 --- openstackclient/compute/v2/server.py | 38 ++- .../tests/unit/compute/v2/fakes.py | 56 ++++ .../tests/unit/compute/v2/test_server.py | 258 +++++++++--------- openstackclient/tests/unit/volume/v2/fakes.py | 110 ++++++-- ...er-add-volume-to-sdk-685e036a88839651.yaml | 4 + 5 files changed, 296 insertions(+), 170 deletions(-) create mode 100644 releasenotes/notes/migrate-server-add-volume-to-sdk-685e036a88839651.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 08627f9bc5..18c1197cc9 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -548,24 +548,25 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - volume_client = self.app.client_manager.volume + compute_client = self.app.client_manager.sdk_connection.compute + volume_client = self.app.client_manager.sdk_connection.volume - server = utils.find_resource( - compute_client.servers, + server = compute_client.find_server( parsed_args.server, + ignore_missing=False, ) - volume = utils.find_resource( - volume_client.volumes, + volume = volume_client.find_volume( parsed_args.volume, + ignore_missing=False, ) kwargs = { + "volumeId": volume.id, "device": parsed_args.device } if parsed_args.tag: - if compute_client.api_version < api_versions.APIVersion('2.49'): + if not sdk_utils.supports_microversion(compute_client, '2.49'): msg = _( '--os-compute-api-version 2.49 or greater is required to ' 'support the --tag option' @@ -575,7 +576,7 @@ def take_action(self, parsed_args): kwargs['tag'] = parsed_args.tag if parsed_args.enable_delete_on_termination: - if compute_client.api_version < api_versions.APIVersion('2.79'): + if not sdk_utils.supports_microversion(compute_client, '2.79'): msg = _( '--os-compute-api-version 2.79 or greater is required to ' 'support the --enable-delete-on-termination option.' @@ -585,7 +586,7 @@ def take_action(self, parsed_args): kwargs['delete_on_termination'] = True if parsed_args.disable_delete_on_termination: - if compute_client.api_version < api_versions.APIVersion('2.79'): + if not sdk_utils.supports_microversion(compute_client, '2.79'): msg = _( '--os-compute-api-version 2.79 or greater is required to ' 'support the --disable-delete-on-termination option.' @@ -594,28 +595,23 @@ def take_action(self, parsed_args): kwargs['delete_on_termination'] = False - volume_attachment = compute_client.volumes.create_server_volume( - server.id, - volume.id, - **kwargs + volume_attachment = compute_client.create_volume_attachment( + server, + **kwargs, ) - columns = ('id', 'serverId', 'volumeId', 'device') + columns = ('id', 'server id', 'volume id', 'device') column_headers = ('ID', 'Server ID', 'Volume ID', 'Device') - if compute_client.api_version >= api_versions.APIVersion('2.49'): + if sdk_utils.supports_microversion(compute_client, '2.49'): columns += ('tag',) column_headers += ('Tag',) - if compute_client.api_version >= api_versions.APIVersion('2.79'): + if sdk_utils.supports_microversion(compute_client, '2.79'): columns += ('delete_on_termination',) column_headers += ('Delete On Termination',) return ( column_headers, - utils.get_item_properties( - volume_attachment, - columns, - mixed_case_fields=('serverId', 'volumeId'), - ) + utils.get_item_properties(volume_attachment, columns,) ) diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index 23468ebc4d..7618c229c5 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -21,6 +21,7 @@ from novaclient import api_versions from openstack.compute.v2 import flavor as _flavor from openstack.compute.v2 import server +from openstack.compute.v2 import volume_attachment from openstackclient.api import compute_v2 from openstackclient.tests.unit import fakes @@ -1803,3 +1804,58 @@ def create_volume_attachments(attrs=None, methods=None, count=2): attrs, methods)) return volume_attachments + + @staticmethod + def create_one_sdk_volume_attachment(attrs=None, methods=None): + """Create a fake sdk VolumeAttachment. + + :param dict attrs: + A dictionary with all attributes + :param dict methods: + A dictionary with all methods + :return: + A fake VolumeAttachment object, with id, device, and so on + """ + attrs = attrs or {} + methods = methods or {} + + # Set default attributes. + volume_attachment_info = { + "id": uuid.uuid4().hex, + "device": "/dev/sdb", + "server_id": uuid.uuid4().hex, + "volume_id": uuid.uuid4().hex, + # introduced in API microversion 2.70 + "tag": "foo", + # introduced in API microversion 2.79 + "delete_on_termination": True, + # introduced in API microversion 2.89 + "attachment_id": uuid.uuid4().hex, + "bdm_uuid": uuid.uuid4().hex + } + + # Overwrite default attributes. + volume_attachment_info.update(attrs) + + return volume_attachment.VolumeAttachment(**volume_attachment_info) + + @staticmethod + def create_sdk_volume_attachments(attrs=None, methods=None, count=2): + """Create multiple fake VolumeAttachment objects (BDMs). + + :param dict attrs: + A dictionary with all attributes + :param dict methods: + A dictionary with all methods + :param int count: + The number of volume attachments to fake + :return: + A list of VolumeAttachment objects faking the volume attachments. + """ + volume_attachments = [] + for i in range(0, count): + volume_attachments.append( + FakeVolumeAttachment.create_one_sdk_volume_attachment( + attrs, methods)) + + return volume_attachments diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 9623cb0abf..203e47ebb3 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -105,6 +105,9 @@ def setUp(self): self.volumes_mock = self.app.client_manager.volume.volumes self.volumes_mock.reset_mock() + self.app.client_manager.sdk_connection.volume = mock.Mock() + self.sdk_volume_client = self.app.client_manager.sdk_connection.volume + # Get a shortcut to the volume client VolumeManager Mock self.snapshots_mock = self.app.client_manager.volume.volume_snapshots self.snapshots_mock.reset_mock() @@ -146,13 +149,18 @@ def setup_sdk_servers_mock(self, count): ) # This is the return value for compute_client.find_server() - self.sdk_client.find_server = compute_fakes.FakeServer.get_servers( - servers, - 0, - ) + self.sdk_client.find_server.side_effect = servers return servers + def setup_sdk_volumes_mock(self, count): + volumes = volume_fakes.FakeVolume.create_sdk_volumes(count=count) + + # This is the return value for volume_client.find_volume() + self.sdk_volume_client.find_volume.side_effect = volumes + + return volumes + def run_method_with_servers(self, method_name, server_count): servers = self.setup_servers_mock(server_count) @@ -680,31 +688,38 @@ class TestServerVolume(TestServer): def setUp(self): super(TestServerVolume, self).setUp() - self.volume = volume_fakes.FakeVolume.create_one_volume() - self.volumes_mock.get.return_value = self.volume - self.methods = { - 'create_server_volume': None, + 'create_volume_attachment': None, } # Get the command object to test self.cmd = server.AddServerVolume(self.app, None) - def test_server_add_volume(self): - servers = self.setup_servers_mock(count=1) - volume_attachment = \ - compute_fakes.FakeVolumeAttachment.create_one_volume_attachment() - self.servers_volumes_mock.create_server_volume.return_value = \ - volume_attachment + self.servers = self.setup_sdk_servers_mock(count=1) + self.volumes = self.setup_sdk_volumes_mock(count=1) + + attrs = { + 'server_id': self.servers[0].id, + 'volume_id': self.volumes[0].id, + } + self.volume_attachment = \ + compute_fakes.FakeVolumeAttachment.\ + create_one_sdk_volume_attachment(attrs=attrs) + + self.sdk_client.create_volume_attachment.return_value = \ + self.volume_attachment + + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=False) + def test_server_add_volume(self, sm_mock): arglist = [ '--device', '/dev/sdb', - servers[0].id, - self.volume.id, + self.servers[0].id, + self.volumes[0].id, ] verifylist = [ - ('server', servers[0].id), - ('volume', self.volume.id), + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), ('device', '/dev/sdb'), ] @@ -712,39 +727,36 @@ def test_server_add_volume(self): expected_columns = ('ID', 'Server ID', 'Volume ID', 'Device') expected_data = ( - volume_attachment.id, - volume_attachment.serverId, - volume_attachment.volumeId, - volume_attachment.device, + self.volume_attachment.id, + self.volume_attachment.server_id, + self.volume_attachment.volume_id, + '/dev/sdb', ) columns, data = self.cmd.take_action(parsed_args) - self.servers_volumes_mock.create_server_volume.assert_called_once_with( - servers[0].id, self.volume.id, device='/dev/sdb') self.assertEqual(expected_columns, columns) self.assertEqual(expected_data, data) + self.sdk_client.create_volume_attachment.assert_called_once_with( + self.servers[0], volumeId=self.volumes[0].id, device='/dev/sdb') - def test_server_add_volume_with_tag(self): - # requires API 2.49 or later - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.49') - - servers = self.setup_servers_mock(count=1) - volume_attachment = \ - compute_fakes.FakeVolumeAttachment.create_one_volume_attachment() - self.servers_volumes_mock.create_server_volume.return_value = \ - volume_attachment + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_add_volume_with_tag(self, sm_mock): + def side_effect(compute_client, version): + if version == '2.49': + return True + return False + sm_mock.side_effect = side_effect arglist = [ '--device', '/dev/sdb', '--tag', 'foo', - servers[0].id, - self.volume.id, + self.servers[0].id, + self.volumes[0].id, ] verifylist = [ - ('server', servers[0].id), - ('volume', self.volume.id), + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), ('device', '/dev/sdb'), ('tag', 'foo'), ] @@ -753,33 +765,33 @@ def test_server_add_volume_with_tag(self): expected_columns = ('ID', 'Server ID', 'Volume ID', 'Device', 'Tag') expected_data = ( - volume_attachment.id, - volume_attachment.serverId, - volume_attachment.volumeId, - volume_attachment.device, - volume_attachment.tag, + self.volume_attachment.id, + self.volume_attachment.server_id, + self.volume_attachment.volume_id, + self.volume_attachment.device, + self.volume_attachment.tag, ) columns, data = self.cmd.take_action(parsed_args) - self.servers_volumes_mock.create_server_volume.assert_called_once_with( - servers[0].id, self.volume.id, device='/dev/sdb', tag='foo') self.assertEqual(expected_columns, columns) self.assertEqual(expected_data, data) + self.sdk_client.create_volume_attachment.assert_called_once_with( + self.servers[0], + volumeId=self.volumes[0].id, + device='/dev/sdb', + tag='foo') - def test_server_add_volume_with_tag_pre_v249(self): - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.48') - - servers = self.setup_servers_mock(count=1) + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=False) + def test_server_add_volume_with_tag_pre_v249(self, sm_mock): arglist = [ - servers[0].id, - self.volume.id, + self.servers[0].id, + self.volumes[0].id, '--tag', 'foo', ] verifylist = [ - ('server', servers[0].id), - ('volume', self.volume.id), + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), ('tag', 'foo'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -792,26 +804,22 @@ def test_server_add_volume_with_tag_pre_v249(self): '--os-compute-api-version 2.49 or greater is required', str(ex)) - def test_server_add_volume_with_enable_delete_on_termination(self): - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.79') - - servers = self.setup_servers_mock(count=1) - volume_attachment = \ - compute_fakes.FakeVolumeAttachment.create_one_volume_attachment() - self.servers_volumes_mock.create_server_volume.return_value = \ - volume_attachment - + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) + def test_server_add_volume_with_enable_delete_on_termination( + self, + sm_mock, + ): + self.volume_attachment.delete_on_termination = True arglist = [ '--enable-delete-on-termination', '--device', '/dev/sdb', - servers[0].id, - self.volume.id, + self.servers[0].id, + self.volumes[0].id, ] verifylist = [ - ('server', servers[0].id), - ('volume', self.volume.id), + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), ('device', '/dev/sdb'), ('enable_delete_on_termination', True), ] @@ -826,42 +834,40 @@ def test_server_add_volume_with_enable_delete_on_termination(self): 'Delete On Termination', ) expected_data = ( - volume_attachment.id, - volume_attachment.serverId, - volume_attachment.volumeId, - volume_attachment.device, - volume_attachment.tag, - volume_attachment.delete_on_termination, + self.volume_attachment.id, + self.volume_attachment.server_id, + self.volume_attachment.volume_id, + self.volume_attachment.device, + self.volume_attachment.tag, + self.volume_attachment.delete_on_termination, ) columns, data = self.cmd.take_action(parsed_args) - - self.servers_volumes_mock.create_server_volume.assert_called_once_with( - servers[0].id, self.volume.id, - device='/dev/sdb', delete_on_termination=True) self.assertEqual(expected_columns, columns) self.assertEqual(expected_data, data) + self.sdk_client.create_volume_attachment.assert_called_once_with( + self.servers[0], + volumeId=self.volumes[0].id, + device='/dev/sdb', + delete_on_termination=True) - def test_server_add_volume_with_disable_delete_on_termination(self): - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.79') - - servers = self.setup_servers_mock(count=1) - volume_attachment = \ - compute_fakes.FakeVolumeAttachment.create_one_volume_attachment() - self.servers_volumes_mock.create_server_volume.return_value = \ - volume_attachment + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) + def test_server_add_volume_with_disable_delete_on_termination( + self, + sm_mock, + ): + self.volume_attachment.delete_on_termination = False arglist = [ '--disable-delete-on-termination', '--device', '/dev/sdb', - servers[0].id, - self.volume.id, + self.servers[0].id, + self.volumes[0].id, ] verifylist = [ - ('server', servers[0].id), - ('volume', self.volume.id), + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), ('device', '/dev/sdb'), ('disable_delete_on_termination', True), ] @@ -876,37 +882,43 @@ def test_server_add_volume_with_disable_delete_on_termination(self): 'Delete On Termination', ) expected_data = ( - volume_attachment.id, - volume_attachment.serverId, - volume_attachment.volumeId, - volume_attachment.device, - volume_attachment.tag, - volume_attachment.delete_on_termination, + self.volume_attachment.id, + self.volume_attachment.server_id, + self.volume_attachment.volume_id, + self.volume_attachment.device, + self.volume_attachment.tag, + self.volume_attachment.delete_on_termination, ) columns, data = self.cmd.take_action(parsed_args) - self.servers_volumes_mock.create_server_volume.assert_called_once_with( - servers[0].id, self.volume.id, - device='/dev/sdb', delete_on_termination=False) self.assertEqual(expected_columns, columns) self.assertEqual(expected_data, data) + self.sdk_client.create_volume_attachment.assert_called_once_with( + self.servers[0], + volumeId=self.volumes[0].id, + device='/dev/sdb', + delete_on_termination=False) + @mock.patch.object(sdk_utils, 'supports_microversion') def test_server_add_volume_with_enable_delete_on_termination_pre_v279( self, + sm_mock, ): - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.78') + def side_effect(compute_client, version): + if version == '2.79': + return False + return True + sm_mock.side_effect = side_effect - servers = self.setup_servers_mock(count=1) arglist = [ - servers[0].id, - self.volume.id, + self.servers[0].id, + self.volumes[0].id, '--enable-delete-on-termination', ] verifylist = [ - ('server', servers[0].id), - ('volume', self.volume.id), + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), ('enable_delete_on_termination', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -917,21 +929,25 @@ def test_server_add_volume_with_enable_delete_on_termination_pre_v279( self.assertIn('--os-compute-api-version 2.79 or greater is required', str(ex)) + @mock.patch.object(sdk_utils, 'supports_microversion') def test_server_add_volume_with_disable_delete_on_termination_pre_v279( self, + sm_mock, ): - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.78') + def side_effect(compute_client, version): + if version == '2.79': + return False + return True + sm_mock.side_effect = side_effect - servers = self.setup_servers_mock(count=1) arglist = [ - servers[0].id, - self.volume.id, + self.servers[0].id, + self.volumes[0].id, '--disable-delete-on-termination', ] verifylist = [ - ('server', servers[0].id), - ('volume', self.volume.id), + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), ('disable_delete_on_termination', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -942,24 +958,22 @@ def test_server_add_volume_with_disable_delete_on_termination_pre_v279( self.assertIn('--os-compute-api-version 2.79 or greater is required', str(ex)) + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) def test_server_add_volume_with_disable_and_enable_delete_on_termination( self, + sm_mock, ): - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.79') - - servers = self.setup_servers_mock(count=1) arglist = [ '--enable-delete-on-termination', '--disable-delete-on-termination', '--device', '/dev/sdb', - servers[0].id, - self.volume.id, + self.servers[0].id, + self.volumes[0].id, ] verifylist = [ - ('server', servers[0].id), - ('volume', self.volume.id), + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), ('device', '/dev/sdb'), ('enable_delete_on_termination', True), ('disable_delete_on_termination', True), diff --git a/openstackclient/tests/unit/volume/v2/fakes.py b/openstackclient/tests/unit/volume/v2/fakes.py index b5f66d4b1d..96e381d3cc 100644 --- a/openstackclient/tests/unit/volume/v2/fakes.py +++ b/openstackclient/tests/unit/volume/v2/fakes.py @@ -18,6 +18,7 @@ import uuid from cinderclient import api_versions +from openstack.block_storage.v3 import volume from osc_lib.cli import format_columns from openstackclient.tests.unit import fakes @@ -46,7 +47,7 @@ class FakeTransfer(object): def create_one_transfer(attrs=None): """Create a fake transfer. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes of Transfer Request :return: A FakeResource object with volume_id, name, id. @@ -75,7 +76,7 @@ def create_one_transfer(attrs=None): def create_transfers(attrs=None, count=2): """Create multiple fake transfers. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes of transfer :param Integer count: The number of transfers to be faked @@ -116,7 +117,7 @@ class FakeTypeAccess(object): def create_one_type_access(attrs=None): """Create a fake volume type access for project. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object, with Volume_type_ID and Project_ID. @@ -148,7 +149,7 @@ class FakeService(object): def create_one_service(attrs=None): """Create a fake service. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes of service :return: A FakeResource object with host, status, etc. @@ -180,7 +181,7 @@ def create_one_service(attrs=None): def create_services(attrs=None, count=2): """Create multiple fake services. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes of service :param Integer count: The number of services to be faked @@ -201,7 +202,7 @@ class FakeCapability(object): def create_one_capability(attrs=None): """Create a fake volume backend capability. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes of the Capabilities. :return: A FakeResource object with capability name and attrs. @@ -260,7 +261,7 @@ class FakePool(object): def create_one_pool(attrs=None): """Create a fake pool. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes of the pool :return: A FakeResource object with pool name and attrs. @@ -362,7 +363,7 @@ class FakeVolume(object): def create_one_volume(attrs=None): """Create a fake volume. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes of volume :return: A FakeResource object with id, name, status, etc. @@ -405,7 +406,7 @@ def create_one_volume(attrs=None): def create_volumes(attrs=None, count=2): """Create multiple fake volumes. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes of volume :param Integer count: The number of volumes to be faked @@ -418,6 +419,61 @@ def create_volumes(attrs=None, count=2): return volumes + @staticmethod + def create_one_sdk_volume(attrs=None): + """Create a fake volume. + + :param dict attrs: + A dictionary with all attributes of volume + :return: + A FakeResource object with id, name, status, etc. + """ + attrs = attrs or {} + + # Set default attribute + volume_info = { + 'id': 'volume-id' + uuid.uuid4().hex, + 'name': 'volume-name' + uuid.uuid4().hex, + 'description': 'description' + uuid.uuid4().hex, + 'status': random.choice(['available', 'in_use']), + 'size': random.randint(1, 20), + 'volume_type': + random.choice(['fake_lvmdriver-1', 'fake_lvmdriver-2']), + 'bootable': + random.choice(['true', 'false']), + 'metadata': { + 'key' + uuid.uuid4().hex: 'val' + uuid.uuid4().hex, + 'key' + uuid.uuid4().hex: 'val' + uuid.uuid4().hex, + 'key' + uuid.uuid4().hex: 'val' + uuid.uuid4().hex}, + 'snapshot_id': random.randint(1, 5), + 'availability_zone': 'zone' + uuid.uuid4().hex, + 'attachments': [{ + 'device': '/dev/' + uuid.uuid4().hex, + 'server_id': uuid.uuid4().hex, + }, ], + } + + # Overwrite default attributes if there are some attributes set + volume_info.update(attrs) + return volume.Volume(**volume_info) + + @staticmethod + def create_sdk_volumes(attrs=None, count=2): + """Create multiple fake volumes. + + :param dict attrs: + A dictionary with all attributes of volume + :param Integer count: + The number of volumes to be faked + :return: + A list of FakeResource objects + """ + volumes = [] + for n in range(0, count): + volumes.append(FakeVolume.create_one_sdk_volume(attrs)) + + return volumes + @staticmethod def get_volumes(volumes=None, count=2): """Get an iterable MagicMock object with a list of faked volumes. @@ -484,7 +540,7 @@ class FakeAvailabilityZone(object): def create_one_availability_zone(attrs=None): """Create a fake AZ. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with zoneName, zoneState, etc. @@ -509,7 +565,7 @@ def create_one_availability_zone(attrs=None): def create_availability_zones(attrs=None, count=2): """Create multiple fake AZs. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :param int count: The number of AZs to fake @@ -532,7 +588,7 @@ class FakeBackup(object): def create_one_backup(attrs=None): """Create a fake backup. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with id, name, volume_id, etc. @@ -565,7 +621,7 @@ def create_one_backup(attrs=None): def create_backups(attrs=None, count=2): """Create multiple fake backups. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :param int count: The number of backups to fake @@ -636,7 +692,7 @@ class FakeConsistencyGroup(object): def create_one_consistency_group(attrs=None): """Create a fake consistency group. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with id, name, description, etc. @@ -666,7 +722,7 @@ def create_one_consistency_group(attrs=None): def create_consistency_groups(attrs=None, count=2): """Create multiple fake consistency groups. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :param int count: The number of consistency groups to fake @@ -713,7 +769,7 @@ class FakeConsistencyGroupSnapshot(object): def create_one_consistency_group_snapshot(attrs=None): """Create a fake consistency group snapshot. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with id, name, description, etc. @@ -742,7 +798,7 @@ def create_one_consistency_group_snapshot(attrs=None): def create_consistency_group_snapshots(attrs=None, count=2): """Create multiple fake consistency group snapshots. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :param int count: The number of consistency group snapshots to fake @@ -789,7 +845,7 @@ class FakeExtension(object): def create_one_extension(attrs=None): """Create a fake extension. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with name, namespace, etc. @@ -825,7 +881,7 @@ class FakeQos(object): def create_one_qos(attrs=None): """Create a fake Qos specification. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with id, name, consumer, etc. @@ -852,7 +908,7 @@ def create_one_qos(attrs=None): def create_one_qos_association(attrs=None): """Create a fake Qos specification association. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with id, name, association_type, etc. @@ -878,7 +934,7 @@ def create_one_qos_association(attrs=None): def create_qoses(attrs=None, count=2): """Create multiple fake Qos specifications. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :param int count: The number of Qos specifications to fake @@ -920,7 +976,7 @@ class FakeSnapshot(object): def create_one_snapshot(attrs=None): """Create a fake snapshot. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with id, name, description, etc. @@ -951,7 +1007,7 @@ def create_one_snapshot(attrs=None): def create_snapshots(attrs=None, count=2): """Create multiple fake snapshots. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :param int count: The number of snapshots to fake @@ -993,9 +1049,9 @@ class FakeVolumeType(object): def create_one_volume_type(attrs=None, methods=None): """Create a fake volume type. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes - :param Dictionary methods: + :param dict methods: A dictionary with all methods :return: A FakeResource object with id, name, description, etc. @@ -1025,7 +1081,7 @@ def create_one_volume_type(attrs=None, methods=None): def create_volume_types(attrs=None, count=2): """Create multiple fake volume_types. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :param int count: The number of types to fake @@ -1063,7 +1119,7 @@ def get_volume_types(volume_types=None, count=2): def create_one_encryption_volume_type(attrs=None): """Create a fake encryption volume type. - :param Dictionary attrs: + :param dict attrs: A dictionary with all attributes :return: A FakeResource object with volume_type_id etc. diff --git a/releasenotes/notes/migrate-server-add-volume-to-sdk-685e036a88839651.yaml b/releasenotes/notes/migrate-server-add-volume-to-sdk-685e036a88839651.yaml new file mode 100644 index 0000000000..54abdacbbc --- /dev/null +++ b/releasenotes/notes/migrate-server-add-volume-to-sdk-685e036a88839651.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Migrate openstack server add volume to using sdk. From 860d6360474b2f215097d1aa4018a57070e44924 Mon Sep 17 00:00:00 2001 From: "Dr. Jens Harbott" Date: Wed, 24 Nov 2021 06:46:22 +0100 Subject: [PATCH 014/697] Temporarily drop aodhclient from doc build Building plugin documentation is failing for aodhclient when running with latest pyparsing. Drop the plugin from docs for now until the issue can be fixed. Needed-By: https://review.opendev.org/c/openstack/requirements/+/818614/ Signed-off-by: Dr. Jens Harbott Change-Id: I1cc1efd9ff2004dd711ed9da0b1d9e8be31175f4 --- doc/source/cli/plugin-commands/aodh.rst | 4 ---- doc/source/cli/plugin-commands/index.rst | 5 ++++- 2 files changed, 4 insertions(+), 5 deletions(-) delete mode 100644 doc/source/cli/plugin-commands/aodh.rst diff --git a/doc/source/cli/plugin-commands/aodh.rst b/doc/source/cli/plugin-commands/aodh.rst deleted file mode 100644 index 5d8b4332cf..0000000000 --- a/doc/source/cli/plugin-commands/aodh.rst +++ /dev/null @@ -1,4 +0,0 @@ -aodh ----- - -.. autoprogram-cliff:: openstack.alarming.v2 diff --git a/doc/source/cli/plugin-commands/index.rst b/doc/source/cli/plugin-commands/index.rst index 4e1ce54b13..638dcbe560 100644 --- a/doc/source/cli/plugin-commands/index.rst +++ b/doc/source/cli/plugin-commands/index.rst @@ -7,7 +7,6 @@ Plugin Commands .. toctree:: :maxdepth: 1 - aodh barbican designate gnocchi @@ -29,6 +28,10 @@ Plugin Commands .. TODO(efried): Make pages for the following once they're fixed. +.. aodh +.. # aodhclient docs build is failing with recent pyparsing +.. # autoprogram-cliff:: openstack.alarming.v2 + .. cue .. # cueclient is not in global-requirements .. # list-plugins:: openstack.mb.v1 From 28cd5763de8706fb997bdd53deaf94aca0de5c52 Mon Sep 17 00:00:00 2001 From: Diwei Zhu Date: Fri, 26 Nov 2021 14:11:02 +0000 Subject: [PATCH 015/697] Add functional test for server add/remove volume. Change-Id: I86a76f32790cafcff1d94364fb72f8890a8cb025 --- .../functional/compute/v2/test_server.py | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index 59b1fad5f9..0d07950e97 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -1119,3 +1119,62 @@ def test_server_add_remove_network_port(self): self.openstack('server delete ' + name) self.openstack('port delete ' + port_name) + + def test_server_add_remove_volume(self): + volume_wait_for = volume_common.BaseVolumeTests.wait_for_status + + name = uuid.uuid4().hex + cmd_output = json.loads(self.openstack( + 'server create -f json ' + + '--network private ' + + '--flavor ' + self.flavor_name + ' ' + + '--image ' + self.image_name + ' ' + + '--wait ' + + name + )) + + self.assertIsNotNone(cmd_output['id']) + self.assertEqual(name, cmd_output['name']) + self.addCleanup(self.openstack, 'server delete --wait ' + name) + server_id = cmd_output['id'] + + volume_name = uuid.uuid4().hex + cmd_output = json.loads(self.openstack( + 'volume create -f json ' + + '--size 1 ' + + volume_name + )) + + self.assertIsNotNone(cmd_output['id']) + self.assertEqual(volume_name, cmd_output['name']) + volume_wait_for('volume', volume_name, 'available') + self.addCleanup(self.openstack, 'volume delete ' + volume_name) + volume_id = cmd_output['id'] + + cmd_output = json.loads(self.openstack( + 'server add volume -f json ' + + name + ' ' + + volume_name + ' ' + + '--tag bar' + )) + + self.assertIsNotNone(cmd_output['ID']) + self.assertEqual(server_id, cmd_output['Server ID']) + self.assertEqual(volume_id, cmd_output['Volume ID']) + volume_attachment_id = cmd_output['ID'] + + cmd_output = json.loads(self.openstack( + 'server volume list -f json ' + + name + )) + + self.assertEqual(volume_attachment_id, cmd_output[0]['ID']) + self.assertEqual(server_id, cmd_output[0]['Server ID']) + self.assertEqual(volume_id, cmd_output[0]['Volume ID']) + + volume_wait_for('volume', volume_name, 'in-use') + self.openstack('server remove volume ' + name + ' ' + volume_name) + volume_wait_for('volume', volume_name, 'available') + + raw_output = self.openstack('server volume list ' + name) + self.assertEqual('\n', raw_output) From fae293dd5218cf4ea03d0a4c44d17b97987dea12 Mon Sep 17 00:00:00 2001 From: Diwei Zhu Date: Tue, 16 Nov 2021 19:08:58 +0000 Subject: [PATCH 016/697] Switch command server remove volume to sdk Change-Id: If6f6cf93b55a67e767c54de8ce21f25252cf99ca --- openstackclient/compute/v2/server.py | 27 ++++++----- .../tests/unit/compute/v2/test_server.py | 45 +++++++++++++++++-- ...remove-volume-to-sdk-47e9befd2672dcdf.yaml | 4 ++ 3 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/switch-server-remove-volume-to-sdk-47e9befd2672dcdf.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 18c1197cc9..121a7b82e8 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -3793,22 +3793,29 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - volume_client = self.app.client_manager.volume + compute_client = self.app.client_manager.sdk_connection.compute + volume_client = self.app.client_manager.sdk_connection.volume - server = utils.find_resource( - compute_client.servers, + server = compute_client.find_server( parsed_args.server, + ignore_missing=False, ) - volume = utils.find_resource( - volume_client.volumes, + volume = volume_client.find_volume( parsed_args.volume, + ignore_missing=False, ) - compute_client.volumes.delete_server_volume( - server.id, - volume.id, - ) + volume_attachments = compute_client.volume_attachments(server) + for volume_attachment in volume_attachments: + if volume_attachment.volume_id == volume.id: + compute_client.delete_volume_attachment( + volume_attachment, + server, + ) + break + else: + msg = _('Target volume attachment not found.') + raise exceptions.CommandError(msg) class RescueServer(command.Command): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 203e47ebb3..10ea07adb3 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -692,9 +692,6 @@ def setUp(self): 'create_volume_attachment': None, } - # Get the command object to test - self.cmd = server.AddServerVolume(self.app, None) - self.servers = self.setup_sdk_servers_mock(count=1) self.volumes = self.setup_sdk_volumes_mock(count=1) @@ -709,6 +706,15 @@ def setUp(self): self.sdk_client.create_volume_attachment.return_value = \ self.volume_attachment + +class TestServerAddVolume(TestServerVolume): + + def setUp(self): + super(TestServerAddVolume, self).setUp() + + # Get the command object to test + self.cmd = server.AddServerVolume(self.app, None) + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=False) def test_server_add_volume(self, sm_mock): @@ -985,6 +991,39 @@ def test_server_add_volume_with_disable_and_enable_delete_on_termination( 'with argument --enable-delete-on-termination', str(ex)) +class TestServerRemoveVolume(TestServerVolume): + + def setUp(self): + super(TestServerRemoveVolume, self).setUp() + + # Get the command object to test + self.cmd = server.RemoveServerVolume(self.app, None) + + def test_server_remove_volume(self): + self.sdk_client.volume_attachments.return_value = [ + self.volume_attachment + ] + + arglist = [ + self.servers[0].id, + self.volumes[0].id, + ] + + verifylist = [ + ('server', self.servers[0].id), + ('volume', self.volumes[0].id), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + self.assertIsNone(result) + self.sdk_client.delete_volume_attachment.assert_called_once_with( + self.volume_attachment, + self.servers[0]) + + class TestServerAddNetwork(TestServer): def setUp(self): diff --git a/releasenotes/notes/switch-server-remove-volume-to-sdk-47e9befd2672dcdf.yaml b/releasenotes/notes/switch-server-remove-volume-to-sdk-47e9befd2672dcdf.yaml new file mode 100644 index 0000000000..3e0397d789 --- /dev/null +++ b/releasenotes/notes/switch-server-remove-volume-to-sdk-47e9befd2672dcdf.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Switch command server remove volume to using sdk. From f4629331134c40599f9baf908a391460b74d2767 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Tue, 23 Nov 2021 21:56:56 +0100 Subject: [PATCH 017/697] Allow unset port's host_id It is supported by Neutron and needs to be done like that when e.g. admin wants to unbound port from the host. Task: #44043 Story: #2009705 Change-Id: I08f1bb40f4dc72cfa7c62feeb5f513455de0ca45 --- openstackclient/network/v2/port.py | 8 ++++++++ openstackclient/tests/unit/network/v2/test_port.py | 5 ++++- .../add-option-to-unset-port-host-c76de9b1d2addf9a.yaml | 5 +++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/add-option-to-unset-port-host-c76de9b1d2addf9a.yaml diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py index 132c384a0e..8f79b80b0b 100644 --- a/openstackclient/network/v2/port.py +++ b/openstackclient/network/v2/port.py @@ -969,6 +969,12 @@ def get_parser(self, prog_name): action='store_true', help=_("Clear existing NUMA affinity policy") ) + parser.add_argument( + '--host', + action='store_true', + default=False, + help=_("Clear host binding for the port.") + ) _tag.add_tag_option_to_parser_for_unset(parser, _('port')) @@ -1026,6 +1032,8 @@ def take_action(self, parsed_args): attrs['data_plane_status'] = None if parsed_args.numa_policy: attrs['numa_affinity_policy'] = None + if parsed_args.host: + attrs['binding:host_id'] = None attrs.update( self._parse_extra_properties(parsed_args.extra_properties)) diff --git a/openstackclient/tests/unit/network/v2/test_port.py b/openstackclient/tests/unit/network/v2/test_port.py index 5f2a12836a..c8540ba057 100644 --- a/openstackclient/tests/unit/network/v2/test_port.py +++ b/openstackclient/tests/unit/network/v2/test_port.py @@ -1923,6 +1923,7 @@ def test_unset_port_parameters(self): 'subnet=042eb10a-3a18-4658-ab-cf47c8d03152,ip-address=1.0.0.0', '--binding-profile', 'Superman', '--qos-policy', + '--host', self._testport.name, ] verifylist = [ @@ -1931,6 +1932,7 @@ def test_unset_port_parameters(self): 'ip-address': '1.0.0.0'}]), ('binding_profile', ['Superman']), ('qos_policy', True), + ('host', True) ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1941,7 +1943,8 @@ def test_unset_port_parameters(self): 'subnet_id': '042eb10a-3a18-4658-ab-cf47c8d03152', 'ip_address': '0.0.0.1'}], 'binding:profile': {'batman': 'Joker'}, - 'qos_policy_id': None + 'qos_policy_id': None, + 'binding:host_id': None } self.network.update_port.assert_called_once_with( self._testport, **attrs) diff --git a/releasenotes/notes/add-option-to-unset-port-host-c76de9b1d2addf9a.yaml b/releasenotes/notes/add-option-to-unset-port-host-c76de9b1d2addf9a.yaml new file mode 100644 index 0000000000..0aa0476050 --- /dev/null +++ b/releasenotes/notes/add-option-to-unset-port-host-c76de9b1d2addf9a.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Add possibility to unbind Neutron's port from the host by unsetting its + host_id. From f82afc7f379daebd1994d9133eff801f790c0d32 Mon Sep 17 00:00:00 2001 From: Diwei Zhu Date: Sun, 14 Nov 2021 22:52:14 +0000 Subject: [PATCH 018/697] Switch openstack server remove port/network to using sdk Change-Id: I1540c1f52e9a107dba20eeea9dc323c5510fe2b1 --- openstackclient/compute/v2/server.py | 25 +++-- .../functional/compute/v2/test_server.py | 94 ++++++++++++++++--- .../tests/unit/compute/v2/test_server.py | 19 ++-- ...-network-port-to-sdk-829ba711e0e198d5.yaml | 4 + 4 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/switch-server-remove-network-port-to-sdk-829ba711e0e198d5.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 18c1197cc9..dfb4dba8fe 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -3690,10 +3690,10 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute - server = utils.find_resource( - compute_client.servers, parsed_args.server) + server = compute_client.find_server( + parsed_args.server, ignore_missing=False) if self.app.client_manager.is_network_endpoint_enabled(): network_client = self.app.client_manager.network @@ -3702,7 +3702,11 @@ def take_action(self, parsed_args): else: port_id = parsed_args.port - server.interface_detach(port_id) + compute_client.delete_server_interface( + port_id, + server=server, + ignore_missing=False, + ) class RemoveNetwork(command.Command): @@ -3723,10 +3727,10 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute - server = utils.find_resource( - compute_client.servers, parsed_args.server) + server = compute_client.find_server( + parsed_args.server, ignore_missing=False) if self.app.client_manager.is_network_endpoint_enabled(): network_client = self.app.client_manager.network @@ -3735,9 +3739,12 @@ def take_action(self, parsed_args): else: net_id = parsed_args.network - for inf in server.interface_list(): + for inf in compute_client.server_interfaces(server): if inf.net_id == net_id: - server.interface_detach(inf.port_id) + compute_client.delete_server_interface( + inf.port_id, + server=server, + ) class RemoveServerSecurityGroup(command.Command): diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index 0d07950e97..cf4bcbc2a9 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -1072,7 +1072,7 @@ def test_server_create_with_empty_network_option_latest(self): self.assertNotIn('nics are required after microversion 2.36', e.stderr) - def test_server_add_remove_network_port(self): + def test_server_add_remove_network(self): name = uuid.uuid4().hex cmd_output = json.loads(self.openstack( 'server create -f json ' + @@ -1085,18 +1085,63 @@ def test_server_add_remove_network_port(self): self.assertIsNotNone(cmd_output['id']) self.assertEqual(name, cmd_output['name']) + self.addCleanup(self.openstack, 'server delete --wait ' + name) + # add network and check 'public' is in server show self.openstack( 'server add network ' + name + ' public') + wait_time = 0 + while wait_time < 60: + cmd_output = json.loads(self.openstack( + 'server show -f json ' + name + )) + if 'public' not in cmd_output['addresses']: + # Hang out for a bit and try again + print('retrying add network check') + wait_time += 10 + time.sleep(10) + else: + break + addresses = cmd_output['addresses'] + self.assertIn('public', addresses) + + # remove network and check 'public' is not in server show + self.openstack('server remove network ' + name + ' public') + + wait_time = 0 + while wait_time < 60: + cmd_output = json.loads(self.openstack( + 'server show -f json ' + name + )) + if 'public' in cmd_output['addresses']: + # Hang out for a bit and try again + print('retrying remove network check') + wait_time += 10 + time.sleep(10) + else: + break + + addresses = cmd_output['addresses'] + self.assertNotIn('public', addresses) + + def test_server_add_remove_port(self): + name = uuid.uuid4().hex cmd_output = json.loads(self.openstack( - 'server show -f json ' + name + 'server create -f json ' + + '--network private ' + + '--flavor ' + self.flavor_name + ' ' + + '--image ' + self.image_name + ' ' + + '--wait ' + + name )) - addresses = cmd_output['addresses'] - self.assertIn('public', addresses) + self.assertIsNotNone(cmd_output['id']) + self.assertEqual(name, cmd_output['name']) + self.addCleanup(self.openstack, 'server delete --wait ' + name) - port_name = 'test-port' + # create port, record one of its ip address + port_name = uuid.uuid4().hex cmd_output = json.loads(self.openstack( 'port list -f json' @@ -1108,17 +1153,44 @@ def test_server_add_remove_network_port(self): '--network private ' + port_name )) self.assertIsNotNone(cmd_output['id']) + ip_address = cmd_output['fixed_ips'][0]['ip_address'] + self.addCleanup(self.openstack, 'port delete ' + port_name) + # add port to server, assert the ip address of the port appears self.openstack('server add port ' + name + ' ' + port_name) - cmd_output = json.loads(self.openstack( - 'server show -f json ' + name - )) + wait_time = 0 + while wait_time < 60: + cmd_output = json.loads(self.openstack( + 'server show -f json ' + name + )) + if ip_address not in cmd_output['addresses']['private']: + # Hang out for a bit and try again + print('retrying add port check') + wait_time += 10 + time.sleep(10) + else: + break + addresses = cmd_output['addresses']['private'] + self.assertIn(ip_address, addresses) - # TODO(diwei): test remove network/port after the commands are switched + # remove port, assert the ip address of the port doesn't appear + self.openstack('server remove port ' + name + ' ' + port_name) - self.openstack('server delete ' + name) - self.openstack('port delete ' + port_name) + wait_time = 0 + while wait_time < 60: + cmd_output = json.loads(self.openstack( + 'server show -f json ' + name + )) + if ip_address in cmd_output['addresses']['private']: + # Hang out for a bit and try again + print('retrying add port check') + wait_time += 10 + time.sleep(10) + else: + break + addresses = cmd_output['addresses']['private'] + self.assertNotIn(ip_address, addresses) def test_server_add_remove_volume(self): volume_wait_for = volume_common.BaseVolumeTests.wait_for_status diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 203e47ebb3..bb0a2c6768 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -6973,14 +6973,14 @@ def setUp(self): # Set method to be tested. self.methods = { - 'interface_detach': None, + 'delete_server_interface': None, } self.find_port = mock.Mock() self.app.client_manager.network.find_port = self.find_port def _test_server_remove_port(self, port_id): - servers = self.setup_servers_mock(count=1) + servers = self.setup_sdk_servers_mock(count=1) port = 'fake-port' arglist = [ @@ -6995,7 +6995,8 @@ def _test_server_remove_port(self, port_id): result = self.cmd.take_action(parsed_args) - servers[0].interface_detach.assert_called_once_with(port_id) + self.sdk_client.delete_server_interface.assert_called_with( + port_id, server=servers[0], ignore_missing=False) self.assertIsNone(result) def test_server_remove_port(self): @@ -7020,17 +7021,18 @@ def setUp(self): # Set method to be tested. self.fake_inf = mock.Mock() self.methods = { - 'interface_list': [self.fake_inf], - 'interface_detach': None, + 'server_interfaces': [self.fake_inf], + 'delete_server_interface': None, } self.find_network = mock.Mock() self.app.client_manager.network.find_network = self.find_network + self.sdk_client.server_interfaces.return_value = [self.fake_inf] def _test_server_remove_network(self, network_id): self.fake_inf.net_id = network_id self.fake_inf.port_id = 'fake-port' - servers = self.setup_servers_mock(count=1) + servers = self.setup_sdk_servers_mock(count=1) network = 'fake-network' arglist = [ @@ -7045,8 +7047,9 @@ def _test_server_remove_network(self, network_id): result = self.cmd.take_action(parsed_args) - servers[0].interface_list.assert_called_once_with() - servers[0].interface_detach.assert_called_once_with('fake-port') + self.sdk_client.server_interfaces.assert_called_once_with(servers[0]) + self.sdk_client.delete_server_interface.assert_called_once_with( + 'fake-port', server=servers[0]) self.assertIsNone(result) def test_server_remove_network(self): diff --git a/releasenotes/notes/switch-server-remove-network-port-to-sdk-829ba711e0e198d5.yaml b/releasenotes/notes/switch-server-remove-network-port-to-sdk-829ba711e0e198d5.yaml new file mode 100644 index 0000000000..6b47b1b35c --- /dev/null +++ b/releasenotes/notes/switch-server-remove-network-port-to-sdk-829ba711e0e198d5.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Switch server remove volume/port to using sdk. From b515fe61b27408e78639da8abb3acaa485ebca4e Mon Sep 17 00:00:00 2001 From: Thrivikram Mudunuri Date: Sat, 13 Nov 2021 02:37:44 -0500 Subject: [PATCH 019/697] Switch server pause and server unpause to SDK Switch the server pause and server unpause commands from novaclient to SDK. Use the SDK versions of test fakes to support fake Server resources. Change-Id: Id626f06f3d7edd44b306b7fc7b9b00d04af09621 --- openstackclient/compute/v2/server.py | 22 ++++++++--------- .../tests/unit/compute/v2/test_server.py | 24 +++++++++++++++---- ...pause-unpause-to-sdk-d74ec8536b764af6.yaml | 5 ++++ 3 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/migrate-server-pause-unpause-to-sdk-d74ec8536b764af6.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 121a7b82e8..09954c49d2 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -3130,12 +3130,13 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute for server in parsed_args.server: - utils.find_resource( - compute_client.servers, - server - ).pause() + server_id = compute_client.find_server( + server, + ignore_missing=False, + ).id + compute_client.pause_server(server_id) class RebootServer(command.Command): @@ -4674,7 +4675,6 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute for server in parsed_args.server: utils.find_resource( @@ -4697,13 +4697,13 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute for server in parsed_args.server: - utils.find_resource( - compute_client.servers, + server_id = compute_client.find_server( server, - ).unpause() + ignore_missing=False, + ).id + compute_client.unpause_server(server_id) class UnrescueServer(command.Command): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 10ea07adb3..435ddb4778 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -192,6 +192,22 @@ def run_method_with_servers(self, method_name, server_count): method.assert_called_with() self.assertIsNone(result) + def run_method_with_sdk_servers(self, method_name, server_count): + servers = self.setup_sdk_servers_mock(count=server_count) + + arglist = [s.id for s in servers] + verifylist = [ + ('server', arglist), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + calls = [call(s.id) for s in servers] + method = getattr(self.sdk_client, method_name) + method.assert_has_calls(calls) + self.assertIsNone(result) + class TestServerAddFixedIP(TestServer): @@ -6062,10 +6078,10 @@ def setUp(self): } def test_server_pause_one_server(self): - self.run_method_with_servers('pause', 1) + self.run_method_with_sdk_servers('pause_server', 1) def test_server_pause_multi_servers(self): - self.run_method_with_servers('pause', 3) + self.run_method_with_sdk_servers('pause_server', 3) class TestServerRebuild(TestServer): @@ -8308,10 +8324,10 @@ def setUp(self): } def test_server_unpause_one_server(self): - self.run_method_with_servers('unpause', 1) + self.run_method_with_sdk_servers('unpause_server', 1) def test_server_unpause_multi_servers(self): - self.run_method_with_servers('unpause', 3) + self.run_method_with_sdk_servers('unpause_server', 3) class TestServerUnset(TestServer): diff --git a/releasenotes/notes/migrate-server-pause-unpause-to-sdk-d74ec8536b764af6.yaml b/releasenotes/notes/migrate-server-pause-unpause-to-sdk-d74ec8536b764af6.yaml new file mode 100644 index 0000000000..e2d4003489 --- /dev/null +++ b/releasenotes/notes/migrate-server-pause-unpause-to-sdk-d74ec8536b764af6.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Migrate ``server pause`` and ``server unpause`` commands from novaclient + to sdk. From ff96fea0120ab43968a10230ce7899a3c6504e75 Mon Sep 17 00:00:00 2001 From: Thrivikram Mudunuri Date: Sat, 13 Nov 2021 17:39:41 -0500 Subject: [PATCH 020/697] Switch server suspend and server resume to SDK Switch the server suspend and server resume commands from novaclient to SDK. Use the SDK versions of test fakes to support fake Server resources. Change-Id: Idd0b4f13fab0f238e42844a7d759538bbda24f68 --- openstackclient/compute/v2/server.py | 20 +++++++++---------- .../tests/unit/compute/v2/test_server.py | 8 ++++---- ...uspend-resume-to-sdk-fd1709336607b496.yaml | 5 +++++ 3 files changed, 19 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/migrate-server-suspend-resume-to-sdk-fd1709336607b496.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 09954c49d2..a7479bb41a 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -4081,13 +4081,13 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute for server in parsed_args.server: - utils.find_resource( - compute_client.servers, + server_id = compute_client.find_server( server, - ).resume() + ignore_missing=False, + ).id + compute_client.resume_server(server_id) class SetServer(command.Command): @@ -4652,13 +4652,13 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute for server in parsed_args.server: - utils.find_resource( - compute_client.servers, + server_id = compute_client.find_server( server, - ).suspend() + ignore_missing=False, + ).id + compute_client.suspend_server(server_id) class UnlockServer(command.Command): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 435ddb4778..27ea12637c 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -7617,10 +7617,10 @@ def setUp(self): } def test_server_resume_one_server(self): - self.run_method_with_servers('resume', 1) + self.run_method_with_sdk_servers('resume_server', 1) def test_server_resume_multi_servers(self): - self.run_method_with_servers('resume', 3) + self.run_method_with_sdk_servers('resume_server', 3) class TestServerSet(TestServer): @@ -8284,10 +8284,10 @@ def setUp(self): } def test_server_suspend_one_server(self): - self.run_method_with_servers('suspend', 1) + self.run_method_with_sdk_servers('suspend_server', 1) def test_server_suspend_multi_servers(self): - self.run_method_with_servers('suspend', 3) + self.run_method_with_sdk_servers('suspend_server', 3) class TestServerUnlock(TestServer): diff --git a/releasenotes/notes/migrate-server-suspend-resume-to-sdk-fd1709336607b496.yaml b/releasenotes/notes/migrate-server-suspend-resume-to-sdk-fd1709336607b496.yaml new file mode 100644 index 0000000000..7d3781bbf4 --- /dev/null +++ b/releasenotes/notes/migrate-server-suspend-resume-to-sdk-fd1709336607b496.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Migrate ``server suspend`` and ``server resume`` commands from novaclient + to sdk. From 4c3de28e83babb0672950320a20492dc61803b4a Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 26 Jan 2021 16:55:05 +0000 Subject: [PATCH 021/697] compute: Reorder building of columns for 'server list' This has no impact on the end result, but it should make fixing issues introduced by API microversion 2.69 a little easier. Change-Id: I7d70eac8aa1a6197ed05a49f071e6899ec219c03 Signed-off-by: Stephen Finucane --- openstackclient/compute/v2/server.py | 192 +++++++++++++++------------ 1 file changed, 105 insertions(+), 87 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 121a7b82e8..be14074b7d 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2204,15 +2204,19 @@ def take_action(self, parsed_args): # flavor name is given, map it to ID. flavor_id = None if parsed_args.flavor: - flavor_id = utils.find_resource(compute_client.flavors, - parsed_args.flavor).id + flavor_id = utils.find_resource( + compute_client.flavors, + parsed_args.flavor, + ).id # Nova only supports list servers searching by image ID. So if a # image name is given, map it to ID. image_id = None if parsed_args.image: - image_id = image_client.find_image(parsed_args.image, - ignore_missing=False).id + image_id = image_client.find_image( + parsed_args.image, + ignore_missing=False, + ).id search_opts = { 'reservation_id': parsed_args.reservation_id, @@ -2320,95 +2324,93 @@ def take_action(self, parsed_args): try: iso8601.parse_date(search_opts['changes-since']) except (TypeError, iso8601.ParseError): + msg = _('Invalid changes-since value: %s') raise exceptions.CommandError( - _('Invalid changes-since value: %s') % - search_opts['changes-since'] + msg % search_opts['changes-since'] ) + columns = ( + 'id', + 'name', + 'status', + ) + column_headers = ( + 'ID', + 'Name', + 'Status', + ) + if parsed_args.long: - columns = ( - 'ID', - 'Name', - 'Status', + columns += ( 'OS-EXT-STS:task_state', 'OS-EXT-STS:power_state', - 'Networks', - 'Image Name', - 'Image ID', - 'Flavor Name', - 'Flavor ID', - 'OS-EXT-AZ:availability_zone', - 'OS-EXT-SRV-ATTR:host', - 'Metadata', ) - column_headers = ( - 'ID', - 'Name', - 'Status', + column_headers += ( 'Task State', 'Power State', - 'Networks', + ) + + columns += ('networks',) + column_headers += ('Networks',) + + if parsed_args.long: + columns += ( + 'image_name', + 'image_id', + ) + column_headers += ( 'Image Name', 'Image ID', + ) + else: + if parsed_args.no_name_lookup: + columns += ('image_id',) + else: + columns += ('image_name',) + column_headers += ('Image',) + + if parsed_args.long: + columns += ( + 'flavor_name', + 'flavor_id', + ) + column_headers += ( 'Flavor Name', 'Flavor ID', - 'Availability Zone', - 'Host', - 'Properties', ) - mixed_case_fields = [ - 'OS-EXT-STS:task_state', - 'OS-EXT-STS:power_state', - 'OS-EXT-AZ:availability_zone', - 'OS-EXT-SRV-ATTR:host', - ] else: if parsed_args.no_name_lookup: - columns = ( - 'ID', - 'Name', - 'Status', - 'Networks', - 'Image ID', - 'Flavor ID', - ) + columns += ('flavor_id',) else: - columns = ( - 'ID', - 'Name', - 'Status', - 'Networks', - 'Image Name', - 'Flavor Name', - ) - column_headers = ( - 'ID', - 'Name', - 'Status', - 'Networks', - 'Image', - 'Flavor', + columns += ('flavor_name',) + column_headers += ('Flavor',) + + if parsed_args.long: + columns += ( + 'OS-EXT-AZ:availability_zone', + 'OS-EXT-SRV-ATTR:host', + 'metadata', + ) + column_headers += ( + 'Availability Zone', + 'Host', + 'Properties', ) - mixed_case_fields = [] marker_id = None # support for additional columns if parsed_args.columns: - # convert tuple to list to edit them - column_headers = list(column_headers) - columns = list(columns) - for c in parsed_args.columns: if c in ('Project ID', 'project_id'): - columns.append('tenant_id') - column_headers.append('Project ID') + columns += ('tenant_id',) + column_headers += ('Project ID',) if c in ('User ID', 'user_id'): - columns.append('user_id') - column_headers.append('User ID') + columns += ('user_id',) + column_headers += ('User ID',) if c in ('Created At', 'created_at'): - columns.append('created') - column_headers.append('Created At') + columns += ('created',) + column_headers += ('Created At',) # convert back to tuple column_headers = tuple(column_headers) @@ -2422,25 +2424,29 @@ def take_action(self, parsed_args): if parsed_args.deleted: marker_id = parsed_args.marker else: - marker_id = utils.find_resource(compute_client.servers, - parsed_args.marker).id + marker_id = utils.find_resource( + compute_client.servers, + parsed_args.marker, + ).id - data = compute_client.servers.list(search_opts=search_opts, - marker=marker_id, - limit=parsed_args.limit) + data = compute_client.servers.list( + search_opts=search_opts, + marker=marker_id, + limit=parsed_args.limit) images = {} flavors = {} if data and not parsed_args.no_name_lookup: - # Create a dict that maps image_id to image object. - # Needed so that we can display the "Image Name" column. - # "Image Name" is not crucial, so we swallow any exceptions. - # The 'image' attribute can be an empty string if the server was - # booted from a volume. + # create a dict that maps image_id to image object, which is used + # to display the "Image Name" column. Note that 'image.id' can be + # empty for BFV instances and 'image' can be missing entirely if + # there are infra failures if parsed_args.name_lookup_one_by_one or image_id: - for i_id in set(filter(lambda x: x is not None, - (s.image.get('id') for s in data - if s.image))): + for i_id in set( + s.image['id'] for s in data + if s.image and s.image.get('id') + ): + # "Image Name" is not crucial, so we swallow any exceptions try: images[i_id] = image_client.get_image(i_id) except Exception: @@ -2453,12 +2459,17 @@ def take_action(self, parsed_args): except Exception: pass - # Create a dict that maps flavor_id to flavor object. - # Needed so that we can display the "Flavor Name" column. - # "Flavor Name" is not crucial, so we swallow any exceptions. + # create a dict that maps flavor_id to flavor object, which is used + # to display the "Flavor Name" column. Note that 'flavor.id' is not + # present on microversion 2.47 or later and 'flavor' won't be + # present if there are infra failures if parsed_args.name_lookup_one_by_one or flavor_id: - for f_id in set(filter(lambda x: x is not None, - (s.flavor.get('id') for s in data))): + for f_id in set( + s.flavor['id'] for s in data + if s.flavor and s.flavor.get('id') + ): + # "Flavor Name" is not crucial, so we swallow any + # exceptions try: flavors[f_id] = compute_client.flavors.get(f_id) except Exception: @@ -2482,6 +2493,7 @@ def take_action(self, parsed_args): # processing of the image and flavor informations. if not hasattr(s, 'image') or not hasattr(s, 'flavor'): continue + if 'id' in s.image: image = images.get(s.image['id']) if image: @@ -2494,6 +2506,7 @@ def take_action(self, parsed_args): # able to grep for boot-from-volume servers when using the CLI. s.image_name = IMAGE_STRING_FOR_BFV s.image_id = IMAGE_STRING_FOR_BFV + if 'id' in s.flavor: flavor = flavors.get(s.flavor['id']) if flavor: @@ -2512,11 +2525,16 @@ def take_action(self, parsed_args): ( utils.get_item_properties( s, columns, - mixed_case_fields=mixed_case_fields, + mixed_case_fields=( + 'OS-EXT-STS:task_state', + 'OS-EXT-STS:power_state', + 'OS-EXT-AZ:availability_zone', + 'OS-EXT-SRV-ATTR:host', + ), formatters={ 'OS-EXT-STS:power_state': PowerStateColumn, - 'Networks': format_columns.DictListColumn, - 'Metadata': format_columns.DictColumn, + 'networks': format_columns.DictListColumn, + 'metadata': format_columns.DictColumn, }, ) for s in data ), From 8e362402dee07744668bcf7f6774af4fbe9a07e3 Mon Sep 17 00:00:00 2001 From: Khomesh Thakre Date: Fri, 6 Nov 2020 22:45:03 +0530 Subject: [PATCH 022/697] compute: Show flavor in 'server list' with API >= 2.47 Fix the issue where the flavor name was empty in server list output. This requires somewhat invasive unit test changes to reflect the changed API response from the server, but this has the upside of meaning we don't need new tests since what we have validates things. Also drop the flavor ID column as it is removed from the compute API. Change-Id: Ica3320242a38901c1180b2b29109c9474366fde0 Signed-off-by: Khomesh Thakre Story: 2008257 Task: 41113 --- openstackclient/compute/v2/server.py | 42 +- .../tests/unit/compute/v2/test_server.py | 558 ++++++++++-------- ...st-microversion-2.47-af200e9bb4747e2d.yaml | 8 + 3 files changed, 348 insertions(+), 260 deletions(-) create mode 100644 releasenotes/notes/fix-flavor-in-server-list-microversion-2.47-af200e9bb4747e2d.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index be14074b7d..a0b27242d4 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2369,21 +2369,28 @@ def take_action(self, parsed_args): columns += ('image_name',) column_headers += ('Image',) - if parsed_args.long: - columns += ( - 'flavor_name', - 'flavor_id', - ) - column_headers += ( - 'Flavor Name', - 'Flavor ID', - ) + # microversion 2.47 puts the embedded flavor into the server response + # body but omits the id, so if not present we just expose the original + # flavor name in the output + if compute_client.api_version >= api_versions.APIVersion('2.47'): + columns += ('flavor_name',) + column_headers += ('Flavor',) else: - if parsed_args.no_name_lookup: - columns += ('flavor_id',) + if parsed_args.long: + columns += ( + 'flavor_name', + 'flavor_id', + ) + column_headers += ( + 'Flavor Name', + 'Flavor ID', + ) else: - columns += ('flavor_name',) - column_headers += ('Flavor',) + if parsed_args.no_name_lookup: + columns += ('flavor_id',) + else: + columns += ('flavor_name',) + column_headers += ('Flavor',) if parsed_args.long: columns += ( @@ -2507,18 +2514,13 @@ def take_action(self, parsed_args): s.image_name = IMAGE_STRING_FOR_BFV s.image_id = IMAGE_STRING_FOR_BFV - if 'id' in s.flavor: + if compute_client.api_version < api_versions.APIVersion('2.47'): flavor = flavors.get(s.flavor['id']) if flavor: s.flavor_name = flavor.name s.flavor_id = s.flavor['id'] else: - # TODO(mriedem): Fix this for microversion >= 2.47 where the - # flavor is embedded in the server response without the id. - # We likely need to drop the Flavor ID column in that case if - # --long is specified. - s.flavor_name = '' - s.flavor_id = '' + s.flavor_name = s.flavor['original_name'] table = ( column_headers, diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 10ea07adb3..17762d72bc 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -4081,7 +4081,7 @@ def test_server_dump_multi_servers(self): self.run_method_with_servers('trigger_crash_dump', 3) -class TestServerList(TestServer): +class _TestServerList(TestServer): # Columns to be listed up. columns = ( @@ -4109,7 +4109,7 @@ class TestServerList(TestServer): ) def setUp(self): - super(TestServerList, self).setUp() + super(_TestServerList, self).setUp() self.search_opts = { 'reservation_id': None, @@ -4148,7 +4148,7 @@ def setUp(self): }, 'OS-EXT-AZ:availability_zone': 'availability-zone-xxx', 'OS-EXT-SRV-ATTR:host': 'host-name-xxx', - 'Metadata': '', + 'Metadata': format_columns.DictColumn({}), } # The servers to be listed. @@ -4167,10 +4167,11 @@ def setUp(self): # Get the command object to test self.cmd = server.ListServer(self.app, None) - # Prepare data returned by fake Nova API. - self.data = [] - self.data_long = [] - self.data_no_name_lookup = [] + +class TestServerList(_TestServerList): + + def setUp(self): + super(TestServerList, self).setUp() Image = collections.namedtuple('Image', 'id name') self.images_mock.return_value = [ @@ -4185,8 +4186,8 @@ def setUp(self): for s in self.servers ] - for s in self.servers: - self.data.append(( + self.data = tuple( + ( s.id, s.name, s.status, @@ -4194,34 +4195,8 @@ def setUp(self): # Image will be an empty string if boot-from-volume self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, self.flavor.name, - )) - self.data_long.append(( - s.id, - s.name, - s.status, - getattr(s, 'OS-EXT-STS:task_state'), - server.PowerStateColumn( - getattr(s, 'OS-EXT-STS:power_state') - ), - format_columns.DictListColumn(s.networks), - # Image will be an empty string if boot-from-volume - self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, - s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, - self.flavor.name, - s.flavor['id'], - getattr(s, 'OS-EXT-AZ:availability_zone'), - getattr(s, 'OS-EXT-SRV-ATTR:host'), - format_columns.DictColumn({}), - )) - self.data_no_name_lookup.append(( - s.id, - s.name, - s.status, - format_columns.DictListColumn(s.networks), - # Image will be an empty string if boot-from-volume - s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, - s.flavor['id'] - )) + ) for s in self.servers + ) def test_server_list_no_option(self): arglist = [] @@ -4242,7 +4217,7 @@ def test_server_list_no_option(self): self.assertFalse(self.flavors_mock.get.call_count) self.assertFalse(self.get_image_mock.call_count) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_no_servers(self): arglist = [] @@ -4261,9 +4236,28 @@ def test_server_list_no_servers(self): self.assertEqual(0, self.images_mock.list.call_count) self.assertEqual(0, self.flavors_mock.list.call_count) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_long_option(self): + self.data = tuple( + ( + s.id, + s.name, + s.status, + getattr(s, 'OS-EXT-STS:task_state'), + server.PowerStateColumn( + getattr(s, 'OS-EXT-STS:power_state') + ), + format_columns.DictListColumn(s.networks), + # Image will be an empty string if boot-from-volume + self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, + s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, + self.flavor.name, + s.flavor['id'], + getattr(s, 'OS-EXT-AZ:availability_zone'), + getattr(s, 'OS-EXT-SRV-ATTR:host'), + s.Metadata, + ) for s in self.servers) arglist = [ '--long', ] @@ -4274,10 +4268,9 @@ def test_server_list_long_option(self): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns_long, columns) - self.assertCountEqual(tuple(self.data_long), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_column_option(self): arglist = [ @@ -4299,6 +4292,18 @@ def test_server_list_column_option(self): self.assertIn('Created At', columns) def test_server_list_no_name_lookup_option(self): + self.data = tuple( + ( + s.id, + s.name, + s.status, + format_columns.DictListColumn(s.networks), + # Image will be an empty string if boot-from-volume + s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, + s.flavor['id'] + ) for s in self.servers + ) + arglist = [ '--no-name-lookup', ] @@ -4312,9 +4317,21 @@ def test_server_list_no_name_lookup_option(self): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data_no_name_lookup), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_n_option(self): + self.data = tuple( + ( + s.id, + s.name, + s.status, + format_columns.DictListColumn(s.networks), + # Image will be an empty string if boot-from-volume + s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, + s.flavor['id'] + ) for s in self.servers + ) + arglist = [ '-n', ] @@ -4328,7 +4345,7 @@ def test_server_list_n_option(self): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data_no_name_lookup), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_name_lookup_one_by_one(self): arglist = [ @@ -4350,7 +4367,7 @@ def test_server_list_name_lookup_one_by_one(self): self.flavors_mock.get.assert_called() self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_with_image(self): @@ -4371,81 +4388,7 @@ def test_server_list_with_image(self): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) - - def test_server_list_with_locked_pre_v273(self): - - arglist = [ - '--locked' - ] - verifylist = [ - ('locked', True) - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - ex = self.assertRaises(exceptions.CommandError, - self.cmd.take_action, - parsed_args) - self.assertIn( - '--os-compute-api-version 2.73 or greater is required', str(ex)) - - def test_server_list_with_locked_v273(self): - - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') - arglist = [ - '--locked' - ] - verifylist = [ - ('locked', True) - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - - self.search_opts['locked'] = True - self.servers_mock.list.assert_called_with(**self.kwargs) - - self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) - - def test_server_list_with_unlocked_v273(self): - - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') - arglist = [ - '--unlocked' - ] - verifylist = [ - ('unlocked', True) - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - - self.search_opts['locked'] = False - self.servers_mock.list.assert_called_with(**self.kwargs) - - self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) - - def test_server_list_with_locked_and_unlocked_v273(self): - - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') - arglist = [ - '--locked', - '--unlocked' - ] - verifylist = [ - ('locked', True), - ('unlocked', True) - ] - - ex = self.assertRaises( - utils.ParserException, - self.check_parser, self.cmd, arglist, verifylist) - self.assertIn('Argument parse failed', str(ex)) + self.assertEqual(self.data, tuple(data)) def test_server_list_with_flavor(self): @@ -4465,7 +4408,7 @@ def test_server_list_with_flavor(self): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_with_changes_since(self): @@ -4486,7 +4429,7 @@ def test_server_list_with_changes_since(self): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) @mock.patch.object(iso8601, 'parse_date', side_effect=iso8601.ParseError) def test_server_list_with_invalid_changes_since(self, mock_parse_isotime): @@ -4509,123 +4452,6 @@ def test_server_list_with_invalid_changes_since(self, mock_parse_isotime): 'Invalid time value' ) - def test_server_list_v266_with_changes_before(self): - self.app.client_manager.compute.api_version = ( - api_versions.APIVersion('2.66')) - arglist = [ - '--changes-before', '2016-03-05T06:27:59Z', - '--deleted' - ] - verifylist = [ - ('changes_before', '2016-03-05T06:27:59Z'), - ('deleted', True), - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - - self.search_opts['changes-before'] = '2016-03-05T06:27:59Z' - self.search_opts['deleted'] = True - - self.servers_mock.list.assert_called_with(**self.kwargs) - - self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) - - @mock.patch.object(iso8601, 'parse_date', side_effect=iso8601.ParseError) - def test_server_list_v266_with_invalid_changes_before( - self, mock_parse_isotime): - self.app.client_manager.compute.api_version = ( - api_versions.APIVersion('2.66')) - - arglist = [ - '--changes-before', 'Invalid time value', - ] - verifylist = [ - ('changes_before', 'Invalid time value'), - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - try: - self.cmd.take_action(parsed_args) - self.fail('CommandError should be raised.') - except exceptions.CommandError as e: - self.assertEqual('Invalid changes-before value: Invalid time ' - 'value', str(e)) - mock_parse_isotime.assert_called_once_with( - 'Invalid time value' - ) - - def test_server_with_changes_before_pre_v266(self): - self.app.client_manager.compute.api_version = ( - api_versions.APIVersion('2.65')) - - arglist = [ - '--changes-before', '2016-03-05T06:27:59Z', - '--deleted' - ] - verifylist = [ - ('changes_before', '2016-03-05T06:27:59Z'), - ('deleted', True), - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - - self.assertRaises(exceptions.CommandError, - self.cmd.take_action, - parsed_args) - - def test_server_list_v269_with_partial_constructs(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.69') - - arglist = [] - verifylist = [] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - - # include "partial results" from non-responsive part of - # infrastructure. - server_dict = { - "id": "server-id-95a56bfc4xxxxxx28d7e418bfd97813a", - "status": "UNKNOWN", - "tenant_id": "6f70656e737461636b20342065766572", - "created": "2018-12-03T21:06:18Z", - "links": [ - { - "href": "http://fake/v2.1/", - "rel": "self" - }, - { - "href": "http://fake", - "rel": "bookmark" - } - ], - # We need to pass networks as {} because its defined as a property - # of the novaclient Server class which gives {} by default. If not - # it will fail at formatting the networks info later on. - "networks": {} - } - _server = compute_fakes.fakes.FakeResource( - info=server_dict, - ) - self.servers.append(_server) - columns, data = self.cmd.take_action(parsed_args) - # get the first three servers out since our interest is in the partial - # server. - next(data) - next(data) - next(data) - partial_server = next(data) - expected_row = ( - 'server-id-95a56bfc4xxxxxx28d7e418bfd97813a', - '', - 'UNKNOWN', - format_columns.DictListColumn({}), - '', - '', - ) - self.assertEqual(expected_row, partial_server) - def test_server_list_with_tag(self): self.app.client_manager.compute.api_version = api_versions.APIVersion( '2.26') @@ -4646,7 +4472,7 @@ def test_server_list_with_tag(self): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_with_tag_pre_v225(self): self.app.client_manager.compute.api_version = api_versions.APIVersion( @@ -4689,7 +4515,7 @@ def test_server_list_with_not_tag(self): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_with_not_tag_pre_v226(self): self.app.client_manager.compute.api_version = api_versions.APIVersion( @@ -4850,6 +4676,258 @@ def test_server_list_with_power_state(self): self.assertEqual(tuple(self.data), tuple(data)) +class TestServerListV273(_TestServerList): + + # Columns to be listed up. + columns = ( + 'ID', + 'Name', + 'Status', + 'Networks', + 'Image', + 'Flavor', + ) + columns_long = ( + 'ID', + 'Name', + 'Status', + 'Task State', + 'Power State', + 'Networks', + 'Image Name', + 'Image ID', + 'Flavor', + 'Availability Zone', + 'Host', + 'Properties', + ) + + def setUp(self): + super(TestServerListV273, self).setUp() + + # The fake servers' attributes. Use the original attributes names in + # nova, not the ones printed by "server list" command. + self.attrs['flavor'] = { + 'vcpus': self.flavor.vcpus, + 'ram': self.flavor.ram, + 'disk': self.flavor.disk, + 'ephemeral': self.flavor.ephemeral, + 'swap': self.flavor.swap, + 'original_name': self.flavor.name, + 'extra_specs': self.flavor.extra_specs, + } + + # The servers to be listed. + self.servers = self.setup_servers_mock(3) + self.servers_mock.list.return_value = self.servers + + Image = collections.namedtuple('Image', 'id name') + self.images_mock.return_value = [ + Image(id=s.image['id'], name=self.image.name) + # Image will be an empty string if boot-from-volume + for s in self.servers if s.image + ] + + # The flavor information is embedded, so now reason for this to be + # called + self.flavors_mock.list = mock.NonCallableMock() + + self.data = tuple( + ( + s.id, + s.name, + s.status, + format_columns.DictListColumn(s.networks), + # Image will be an empty string if boot-from-volume + self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, + self.flavor.name, + ) for s in self.servers) + + def test_server_list_with_locked_pre_v273(self): + + arglist = [ + '--locked' + ] + verifylist = [ + ('locked', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + ex = self.assertRaises(exceptions.CommandError, + self.cmd.take_action, + parsed_args) + self.assertIn( + '--os-compute-api-version 2.73 or greater is required', str(ex)) + + def test_server_list_with_locked(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--locked' + ] + verifylist = [ + ('locked', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + self.search_opts['locked'] = True + self.servers_mock.list.assert_called_with(**self.kwargs) + + self.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) + + def test_server_list_with_unlocked_v273(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--unlocked' + ] + verifylist = [ + ('unlocked', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + self.search_opts['locked'] = False + self.servers_mock.list.assert_called_with(**self.kwargs) + + self.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) + + def test_server_list_with_locked_and_unlocked(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--locked', + '--unlocked' + ] + verifylist = [ + ('locked', True), + ('unlocked', True) + ] + + ex = self.assertRaises( + utils.ParserException, + self.check_parser, self.cmd, arglist, verifylist) + self.assertIn('Argument parse failed', str(ex)) + + def test_server_list_with_changes_before(self): + self.app.client_manager.compute.api_version = ( + api_versions.APIVersion('2.66')) + arglist = [ + '--changes-before', '2016-03-05T06:27:59Z', + '--deleted' + ] + verifylist = [ + ('changes_before', '2016-03-05T06:27:59Z'), + ('deleted', True), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + self.search_opts['changes-before'] = '2016-03-05T06:27:59Z' + self.search_opts['deleted'] = True + + self.servers_mock.list.assert_called_with(**self.kwargs) + + self.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) + + @mock.patch.object(iso8601, 'parse_date', side_effect=iso8601.ParseError) + def test_server_list_with_invalid_changes_before( + self, mock_parse_isotime): + self.app.client_manager.compute.api_version = ( + api_versions.APIVersion('2.66')) + + arglist = [ + '--changes-before', 'Invalid time value', + ] + verifylist = [ + ('changes_before', 'Invalid time value'), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + try: + self.cmd.take_action(parsed_args) + self.fail('CommandError should be raised.') + except exceptions.CommandError as e: + self.assertEqual('Invalid changes-before value: Invalid time ' + 'value', str(e)) + mock_parse_isotime.assert_called_once_with( + 'Invalid time value' + ) + + def test_server_with_changes_before_pre_v266(self): + self.app.client_manager.compute.api_version = ( + api_versions.APIVersion('2.65')) + + arglist = [ + '--changes-before', '2016-03-05T06:27:59Z', + '--deleted' + ] + verifylist = [ + ('changes_before', '2016-03-05T06:27:59Z'), + ('deleted', True), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises(exceptions.CommandError, + self.cmd.take_action, + parsed_args) + + def test_server_list_v269_with_partial_constructs(self): + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.69') + arglist = [] + verifylist = [] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + # include "partial results" from non-responsive part of + # infrastructure. + server_dict = { + "id": "server-id-95a56bfc4xxxxxx28d7e418bfd97813a", + "status": "UNKNOWN", + "tenant_id": "6f70656e737461636b20342065766572", + "created": "2018-12-03T21:06:18Z", + "links": [ + { + "href": "http://fake/v2.1/", + "rel": "self" + }, + { + "href": "http://fake", + "rel": "bookmark" + } + ], + # We need to pass networks as {} because its defined as a property + # of the novaclient Server class which gives {} by default. If not + # it will fail at formatting the networks info later on. + "networks": {} + } + server = compute_fakes.fakes.FakeResource( + info=server_dict, + ) + self.servers.append(server) + columns, data = self.cmd.take_action(parsed_args) + # get the first three servers out since our interest is in the partial + # server. + next(data) + next(data) + next(data) + partial_server = next(data) + expected_row = ( + 'server-id-95a56bfc4xxxxxx28d7e418bfd97813a', '', + 'UNKNOWN', format_columns.DictListColumn({}), '', '') + self.assertEqual(expected_row, partial_server) + + class TestServerLock(TestServer): def setUp(self): diff --git a/releasenotes/notes/fix-flavor-in-server-list-microversion-2.47-af200e9bb4747e2d.yaml b/releasenotes/notes/fix-flavor-in-server-list-microversion-2.47-af200e9bb4747e2d.yaml new file mode 100644 index 0000000000..fdb37bbbc9 --- /dev/null +++ b/releasenotes/notes/fix-flavor-in-server-list-microversion-2.47-af200e9bb4747e2d.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Add support for compute API microversion 2.47, which changes how flavor + details are included in server detail responses. In 2.46 and below, + only the flavor ID was shown in the server detail response. Starting in + 2.47, flavor information is embedded in the server response. The newer + behavior is now supported. From c8c4f76498de3380c7cbf80c5dc800a588bed649 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 26 Oct 2021 13:03:22 +0000 Subject: [PATCH 023/697] Add --security-group to port list The neutron API supports filtering ports by security group. Closes-Bug: #1405057 Depends-On: https://review.opendev.org/c/openstack/openstacksdk/+/804979 Change-Id: I0f626882716c21ac200c1b929ea04664d21874d8 --- openstackclient/network/v2/port.py | 9 +++++++++ .../tests/unit/network/v2/test_port.py | 20 +++++++++++++++++++ ...-list-security-group-4af5d2e789174ff9.yaml | 5 +++++ 3 files changed, 34 insertions(+) create mode 100644 releasenotes/notes/port-list-security-group-4af5d2e789174ff9.yaml diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py index 132c384a0e..887c531808 100644 --- a/openstackclient/network/v2/port.py +++ b/openstackclient/network/v2/port.py @@ -610,6 +610,13 @@ def get_parser(self, prog_name): metavar='', help=_("List ports according to their name") ) + parser.add_argument( + '--security-group', + action='append', + dest='security_groups', + metavar='', + help=_("List only ports associated with this security group") + ) identity_common.add_project_domain_option_to_parser(parser) parser.add_argument( '--fixed-ip', @@ -682,6 +689,8 @@ def take_action(self, parsed_args): if parsed_args.fixed_ip: filters['fixed_ips'] = _prepare_filter_fixed_ips( self.app.client_manager, parsed_args) + if parsed_args.security_groups: + filters['security_groups'] = parsed_args.security_groups _tag.get_tag_filtering_args(parsed_args, filters) diff --git a/openstackclient/tests/unit/network/v2/test_port.py b/openstackclient/tests/unit/network/v2/test_port.py index 5f2a12836a..96edb74df8 100644 --- a/openstackclient/tests/unit/network/v2/test_port.py +++ b/openstackclient/tests/unit/network/v2/test_port.py @@ -1296,6 +1296,26 @@ def test_list_with_tag_options(self): self.assertEqual(self.columns, columns) self.assertCountEqual(self.data, list(data)) + def test_port_list_security_group(self): + arglist = [ + '--security-group', 'sg-id1', + '--security-group', 'sg-id2', + ] + verifylist = [ + ('security_groups', ['sg-id1', 'sg-id2']), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + filters = { + 'security_groups': ['sg-id1', 'sg-id2'], + 'fields': LIST_FIELDS_TO_RETRIEVE, + } + + self.network.ports.assert_called_once_with(**filters) + self.assertEqual(self.columns, columns) + self.assertCountEqual(self.data, list(data)) + class TestSetPort(TestPort): diff --git a/releasenotes/notes/port-list-security-group-4af5d2e789174ff9.yaml b/releasenotes/notes/port-list-security-group-4af5d2e789174ff9.yaml new file mode 100644 index 0000000000..c68eeafbce --- /dev/null +++ b/releasenotes/notes/port-list-security-group-4af5d2e789174ff9.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Added ``--security-group`` option to the ``os port list`` command. This + option is appendable and multiple security group IDs can be provided. From bef70397a3e1240cc593b3fb34049f2ff6601e68 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 27 Jul 2021 16:45:24 +0000 Subject: [PATCH 024/697] Add network update quota "limit_check" parameter This new parameter commands the Neutron server to first check the resource usage before setting the new quota limit. If the resource usage is below the new limit, the Neutron server will raise an exception. Depends-On: https://review.opendev.org/c/openstack/openstacksdk/+/806254 Depends-On: https://review.opendev.org/c/openstack/neutron/+/801470 Partial-Bug: #1936408 Change-Id: Idc1b99492d609eb699d0a6bef6cd760458a774f6 --- openstackclient/common/quota.py | 9 ++++ .../tests/functional/common/test_quota.py | 25 +++++++++++ .../tests/unit/common/test_quota.py | 43 +++++++++++++++++++ .../check-limit-quota-cc7f291dd1b537c1.yaml | 5 +++ 4 files changed, 82 insertions(+) create mode 100644 releasenotes/notes/check-limit-quota-cc7f291dd1b537c1.yaml diff --git a/openstackclient/common/quota.py b/openstackclient/common/quota.py index 643cb4e474..677cba038e 100644 --- a/openstackclient/common/quota.py +++ b/openstackclient/common/quota.py @@ -535,6 +535,12 @@ def get_parser(self, prog_name): action='store_true', help=_('Force quota update (only supported by compute)') ) + parser.add_argument( + '--check-limit', + action='store_true', + help=_('Check quota limit when updating (only supported by ' + 'network)') + ) return parser def take_action(self, parsed_args): @@ -561,6 +567,9 @@ def take_action(self, parsed_args): volume_kwargs[k] = value network_kwargs = {} + if parsed_args.check_limit: + network_kwargs['check_limit'] = True + if self.app.client_manager.is_network_endpoint_enabled(): for k, v in NETWORK_QUOTAS.items(): value = getattr(parsed_args, k, None) diff --git a/openstackclient/tests/functional/common/test_quota.py b/openstackclient/tests/functional/common/test_quota.py index 9c05746077..bf67101a32 100644 --- a/openstackclient/tests/functional/common/test_quota.py +++ b/openstackclient/tests/functional/common/test_quota.py @@ -11,6 +11,9 @@ # under the License. import json +import uuid + +from tempest.lib import exceptions from openstackclient.tests.functional import base @@ -165,3 +168,25 @@ def test_quota_set_class(self): # returned attributes self.assertTrue(cmd_output["key-pairs"] >= 0) self.assertTrue(cmd_output["snapshots"] >= 0) + + def test_quota_network_set_with_check_limit(self): + if not self.haz_network: + self.skipTest('No Network service present') + if not self.is_extension_enabled('quota-check-limit'): + self.skipTest('No "quota-check-limit" extension present') + + self.openstack('quota set --networks 40 ' + self.PROJECT_NAME) + cmd_output = json.loads(self.openstack( + 'quota list -f json --network' + )) + self.assertIsNotNone(cmd_output) + self.assertEqual(40, cmd_output[0]['Networks']) + + # That will ensure we have at least two networks in the system. + for _ in range(2): + self.openstack('network create --project %s %s' % + (self.PROJECT_NAME, uuid.uuid4().hex)) + + self.assertRaises(exceptions.CommandFailed, self.openstack, + 'quota set --networks 1 --check-limit ' + + self.PROJECT_NAME) diff --git a/openstackclient/tests/unit/common/test_quota.py b/openstackclient/tests/unit/common/test_quota.py index 8771359cb5..896a63a7bc 100644 --- a/openstackclient/tests/unit/common/test_quota.py +++ b/openstackclient/tests/unit/common/test_quota.py @@ -950,6 +950,49 @@ def test_quota_set_with_force(self): ) self.assertIsNone(result) + def test_quota_set_with_check_limit(self): + arglist = [ + '--subnets', str(network_fakes.QUOTA['subnet']), + '--volumes', str(volume_fakes.QUOTA['volumes']), + '--cores', str(compute_fakes.core_num), + '--check-limit', + self.projects[0].name, + ] + verifylist = [ + ('subnet', network_fakes.QUOTA['subnet']), + ('volumes', volume_fakes.QUOTA['volumes']), + ('cores', compute_fakes.core_num), + ('check_limit', True), + ('project', self.projects[0].name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + kwargs_compute = { + 'cores': compute_fakes.core_num, + } + kwargs_volume = { + 'volumes': volume_fakes.QUOTA['volumes'], + } + kwargs_network = { + 'subnet': network_fakes.QUOTA['subnet'], + 'check_limit': True, + } + self.compute_quotas_mock.update.assert_called_once_with( + self.projects[0].id, + **kwargs_compute + ) + self.volume_quotas_mock.update.assert_called_once_with( + self.projects[0].id, + **kwargs_volume + ) + self.network_mock.update_quota.assert_called_once_with( + self.projects[0].id, + **kwargs_network + ) + self.assertIsNone(result) + class TestQuotaShow(TestQuota): diff --git a/releasenotes/notes/check-limit-quota-cc7f291dd1b537c1.yaml b/releasenotes/notes/check-limit-quota-cc7f291dd1b537c1.yaml new file mode 100644 index 0000000000..171b4a5ae0 --- /dev/null +++ b/releasenotes/notes/check-limit-quota-cc7f291dd1b537c1.yaml @@ -0,0 +1,5 @@ +--- +features: + - Add ``--check-limit`` option to the ``openstack quota set`` command (only + for network commands). The network quota engine will check the resource + usage before setting the new quota limit. From 32e18253faa742aae5a4c9708a8a505c85ebb317 Mon Sep 17 00:00:00 2001 From: "Dr. Jens Harbott" Date: Tue, 7 Dec 2021 18:33:54 +0100 Subject: [PATCH 025/697] Fix RemoveServerVolume The nova API we're using to delete a server volume attachment needs to be handed a volume, not a volume attachment. Also make sure that we create an error if the volume isn't actually attached to the server. Signed-off-by: Dr. Jens Harbott Co-authored-by: Stephen Finucane Change-Id: I12abd3787ea47acb4da282d00fdc1989405a0564 --- openstackclient/compute/v2/server.py | 16 +++++----------- .../tests/unit/compute/v2/test_server.py | 10 ++++------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index fb13d30572..a18ce81012 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -3833,17 +3833,11 @@ def take_action(self, parsed_args): ignore_missing=False, ) - volume_attachments = compute_client.volume_attachments(server) - for volume_attachment in volume_attachments: - if volume_attachment.volume_id == volume.id: - compute_client.delete_volume_attachment( - volume_attachment, - server, - ) - break - else: - msg = _('Target volume attachment not found.') - raise exceptions.CommandError(msg) + compute_client.delete_volume_attachment( + volume, + server, + ignore_missing=False, + ) class RescueServer(command.Command): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 00733e4a76..15be07fc6c 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -1016,10 +1016,6 @@ def setUp(self): self.cmd = server.RemoveServerVolume(self.app, None) def test_server_remove_volume(self): - self.sdk_client.volume_attachments.return_value = [ - self.volume_attachment - ] - arglist = [ self.servers[0].id, self.volumes[0].id, @@ -1036,8 +1032,10 @@ def test_server_remove_volume(self): self.assertIsNone(result) self.sdk_client.delete_volume_attachment.assert_called_once_with( - self.volume_attachment, - self.servers[0]) + self.volumes[0], + self.servers[0], + ignore_missing=False, + ) class TestServerAddNetwork(TestServer): From 4e9b9298429f5db505987853f98d2388b6745b13 Mon Sep 17 00:00:00 2001 From: "Dr. Jens Harbott" Date: Tue, 30 Nov 2021 09:21:56 +0100 Subject: [PATCH 026/697] Allow setting gateway when creating a router These options are not only valid when modifying a router, but also when one is created initially. Signed-off-by: Dr. Jens Harbott Change-Id: I3e12901f37cbd1639ac9dc9cc49b04114b80474c --- openstackclient/network/v2/router.py | 79 +++++++++++++------ .../tests/unit/network/v2/test_router.py | 37 +++++++++ ...ptions-create-router-97910a882b604652.yaml | 8 ++ 3 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/options-create-router-97910a882b604652.yaml diff --git a/openstackclient/network/v2/router.py b/openstackclient/network/v2/router.py index dde4eda99f..aeeec93175 100644 --- a/openstackclient/network/v2/router.py +++ b/openstackclient/network/v2/router.py @@ -111,6 +111,30 @@ def _get_attrs(client_manager, parsed_args): parsed_args.project_domain, ).id attrs['tenant_id'] = project_id + if parsed_args.external_gateway: + gateway_info = {} + n_client = client_manager.network + network = n_client.find_network( + parsed_args.external_gateway, ignore_missing=False) + gateway_info['network_id'] = network.id + if parsed_args.disable_snat: + gateway_info['enable_snat'] = False + if parsed_args.enable_snat: + gateway_info['enable_snat'] = True + if parsed_args.fixed_ip: + ips = [] + for ip_spec in parsed_args.fixed_ip: + if ip_spec.get('subnet', False): + subnet_name_id = ip_spec.pop('subnet') + if subnet_name_id: + subnet = n_client.find_subnet(subnet_name_id, + ignore_missing=False) + ip_spec['subnet_id'] = subnet.id + if ip_spec.get('ip-address', False): + ip_spec['ip_address'] = ip_spec.pop('ip-address') + ips.append(ip_spec) + gateway_info['external_fixed_ips'] = ips + attrs['external_gateway_info'] = gateway_info return attrs @@ -320,6 +344,32 @@ def get_parser(self, prog_name): "repeat option to set multiple availability zones)") ) _tag.add_tag_option_to_parser_for_create(parser, _('router')) + parser.add_argument( + '--external-gateway', + metavar="", + help=_("External Network used as router's gateway (name or ID)") + ) + parser.add_argument( + '--fixed-ip', + metavar='subnet=,ip-address=', + action=parseractions.MultiKeyValueAction, + optional_keys=['subnet', 'ip-address'], + help=_("Desired IP and/or subnet (name or ID) " + "on external gateway: " + "subnet=,ip-address= " + "(repeat option to set multiple fixed IP addresses)") + ) + snat_group = parser.add_mutually_exclusive_group() + snat_group.add_argument( + '--enable-snat', + action='store_true', + help=_("Enable Source NAT on external gateway") + ) + snat_group.add_argument( + '--disable-snat', + action='store_true', + help=_("Disable Source NAT on external gateway") + ) return parser @@ -338,6 +388,12 @@ def take_action(self, parsed_args): # tags cannot be set when created, so tags need to be set later. _tag.update_tags_for_set(client, obj, parsed_args) + if (parsed_args.disable_snat or parsed_args.enable_snat or + parsed_args.fixed_ip) and not parsed_args.external_gateway: + msg = (_("You must specify '--external-gateway' in order " + "to specify SNAT or fixed-ip values")) + raise exceptions.CommandError(msg) + display_columns, columns = _get_columns(obj) data = utils.get_item_properties(obj, columns, formatters=_formatters) @@ -725,29 +781,6 @@ def take_action(self, parsed_args): msg = (_("You must specify '--external-gateway' in order " "to update the SNAT or fixed-ip values")) raise exceptions.CommandError(msg) - if parsed_args.external_gateway: - gateway_info = {} - network = client.find_network( - parsed_args.external_gateway, ignore_missing=False) - gateway_info['network_id'] = network.id - if parsed_args.disable_snat: - gateway_info['enable_snat'] = False - if parsed_args.enable_snat: - gateway_info['enable_snat'] = True - if parsed_args.fixed_ip: - ips = [] - for ip_spec in parsed_args.fixed_ip: - if ip_spec.get('subnet', False): - subnet_name_id = ip_spec.pop('subnet') - if subnet_name_id: - subnet = client.find_subnet(subnet_name_id, - ignore_missing=False) - ip_spec['subnet_id'] = subnet.id - if ip_spec.get('ip-address', False): - ip_spec['ip_address'] = ip_spec.pop('ip-address') - ips.append(ip_spec) - gateway_info['external_fixed_ips'] = ips - attrs['external_gateway_info'] = gateway_info if ((parsed_args.qos_policy or parsed_args.no_qos_policy) and not parsed_args.external_gateway): diff --git a/openstackclient/tests/unit/network/v2/test_router.py b/openstackclient/tests/unit/network/v2/test_router.py index 0324674817..04d9fe4836 100644 --- a/openstackclient/tests/unit/network/v2/test_router.py +++ b/openstackclient/tests/unit/network/v2/test_router.py @@ -186,6 +186,43 @@ def test_create_default_options(self): self.assertEqual(self.columns, columns) self.assertCountEqual(self.data, data) + def test_create_with_gateway(self): + _network = network_fakes.FakeNetwork.create_one_network() + _subnet = network_fakes.FakeSubnet.create_one_subnet() + self.network.find_network = mock.Mock(return_value=_network) + self.network.find_subnet = mock.Mock(return_value=_subnet) + arglist = [ + self.new_router.name, + '--external-gateway', _network.name, + '--enable-snat', + '--fixed-ip', 'ip-address=2001:db8::1' + ] + verifylist = [ + ('name', self.new_router.name), + ('enable', True), + ('distributed', False), + ('ha', False), + ('external_gateway', _network.name), + ('enable_snat', True), + ('fixed_ip', [{'ip-address': '2001:db8::1'}]), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = (self.cmd.take_action(parsed_args)) + + self.network.create_router.assert_called_once_with(**{ + 'admin_state_up': True, + 'name': self.new_router.name, + 'external_gateway_info': { + 'network_id': _network.id, + 'enable_snat': True, + 'external_fixed_ips': [{'ip_address': '2001:db8::1'}], + }, + }) + self.assertFalse(self.network.set_tags.called) + self.assertEqual(self.columns, columns) + self.assertCountEqual(self.data, data) + def _test_create_with_ha_options(self, option, ha): arglist = [ option, diff --git a/releasenotes/notes/options-create-router-97910a882b604652.yaml b/releasenotes/notes/options-create-router-97910a882b604652.yaml new file mode 100644 index 0000000000..f7d90b75c2 --- /dev/null +++ b/releasenotes/notes/options-create-router-97910a882b604652.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + It is now possible to add an external gateway to a router + immediately on creation. Previously it could only be done by + modifying the router after it had been created. This includes + the options to en- or disable SNAT and to specify a fixed-ip + on the external network. From b3cb85f1123b15c1ec4fafac9dcedc9381072a8b Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 6 Dec 2021 10:24:15 +0000 Subject: [PATCH 027/697] tests: Improve logging for executed commands We're seeing failures in a recently added tests, 'ServerTests.test_server_add_remove_volume' from 'openstackclient/tests/functional/compute/v2/test_server.py'. These failures are likely the result of slow CI nodes, but we don't have enough information in the CI logs to debug them. Starting logging the various commands executed in tests so that we can see these logs if and when tests fail. Change-Id: I4584dc5e6343fe8c8544431a527d8c3c7e7b3c5b Signed-off-by: Stephen Finucane --- openstackclient/tests/functional/base.py | 21 ++++++++---- .../functional/compute/v2/test_server.py | 33 ++++++++++++++----- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/openstackclient/tests/functional/base.py b/openstackclient/tests/functional/base.py index 0ed7dff8c4..e89c5b9737 100644 --- a/openstackclient/tests/functional/base.py +++ b/openstackclient/tests/functional/base.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +import logging import os import shlex import subprocess @@ -18,22 +19,30 @@ from tempest.lib import exceptions import testtools - ADMIN_CLOUD = os.environ.get('OS_ADMIN_CLOUD', 'devstack-admin') +LOG = logging.getLogger(__name__) def execute(cmd, fail_ok=False, merge_stderr=False): """Executes specified command for the given action.""" + LOG.debug('Executing: %s', cmd) cmdlist = shlex.split(cmd) stdout = subprocess.PIPE stderr = subprocess.STDOUT if merge_stderr else subprocess.PIPE + proc = subprocess.Popen(cmdlist, stdout=stdout, stderr=stderr) - result, result_err = proc.communicate() - result = result.decode('utf-8') + + result_out, result_err = proc.communicate() + result_out = result_out.decode('utf-8') + LOG.debug('stdout: %s', result_out) + LOG.debug('stderr: %s', result_err) + if not fail_ok and proc.returncode != 0: - raise exceptions.CommandFailed(proc.returncode, cmd, result, - result_err) - return result + raise exceptions.CommandFailed( + proc.returncode, cmd, result_out, result_err, + ) + + return result_out class TestCase(testtools.TestCase): diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index cf4bcbc2a9..0558ef6213 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -1195,19 +1195,19 @@ def test_server_add_remove_port(self): def test_server_add_remove_volume(self): volume_wait_for = volume_common.BaseVolumeTests.wait_for_status - name = uuid.uuid4().hex + server_name = uuid.uuid4().hex cmd_output = json.loads(self.openstack( 'server create -f json ' + '--network private ' + '--flavor ' + self.flavor_name + ' ' + '--image ' + self.image_name + ' ' + '--wait ' + - name + server_name )) self.assertIsNotNone(cmd_output['id']) - self.assertEqual(name, cmd_output['name']) - self.addCleanup(self.openstack, 'server delete --wait ' + name) + self.assertEqual(server_name, cmd_output['name']) + self.addCleanup(self.openstack, 'server delete --wait ' + server_name) server_id = cmd_output['id'] volume_name = uuid.uuid4().hex @@ -1225,7 +1225,7 @@ def test_server_add_remove_volume(self): cmd_output = json.loads(self.openstack( 'server add volume -f json ' + - name + ' ' + + server_name + ' ' + volume_name + ' ' + '--tag bar' )) @@ -1237,7 +1237,7 @@ def test_server_add_remove_volume(self): cmd_output = json.loads(self.openstack( 'server volume list -f json ' + - name + server_name )) self.assertEqual(volume_attachment_id, cmd_output[0]['ID']) @@ -1245,8 +1245,25 @@ def test_server_add_remove_volume(self): self.assertEqual(volume_id, cmd_output[0]['Volume ID']) volume_wait_for('volume', volume_name, 'in-use') - self.openstack('server remove volume ' + name + ' ' + volume_name) + + cmd_output = json.loads(self.openstack( + 'server event list -f json ' + + server_name + )) + self.assertEqual(2, len(cmd_output)) + self.assertIn('attach_volume', {x['Action'] for x in cmd_output}) + + self.openstack( + 'server remove volume ' + server_name + ' ' + volume_name + ) volume_wait_for('volume', volume_name, 'available') - raw_output = self.openstack('server volume list ' + name) + cmd_output = json.loads(self.openstack( + 'server event list -f json ' + + server_name + )) + self.assertEqual(3, len(cmd_output)) + self.assertIn('detach_volume', {x['Action'] for x in cmd_output}) + + raw_output = self.openstack('server volume list ' + server_name) self.assertEqual('\n', raw_output) From 9971d7253e9c3abd2e3940bf549ef8532ef929f9 Mon Sep 17 00:00:00 2001 From: Ritvik Vinodkumar Date: Wed, 1 Dec 2021 16:54:53 +0000 Subject: [PATCH 028/697] Switch add fixed IP to SDK Switch the add fixed IP command from novaclient to SDK. Change-Id: I4752ea7b4bfc17e04b8f46dbe9a68d938501a89e --- openstackclient/compute/v2/server.py | 44 ++-- .../tests/unit/compute/v2/test_server.py | 224 ++++++++++++++---- ...-add-fixed-ip-to-sdk-3d932d77633bc765.yaml | 3 + 3 files changed, 211 insertions(+), 60 deletions(-) create mode 100644 releasenotes/notes/migrate-add-fixed-ip-to-sdk-3d932d77633bc765.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index a18ce81012..0931a6ca01 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -237,30 +237,44 @@ def get_parser(self, prog_name): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - - server = utils.find_resource( - compute_client.servers, parsed_args.server) - - network = compute_client.api.network_find(parsed_args.network) - - kwargs = { - 'port_id': None, - 'net_id': network['id'], - 'fixed_ip': parsed_args.fixed_ip_address, - } + compute_client = self.app.client_manager.sdk_connection.compute + server = compute_client.find_server( + parsed_args.server, + ignore_missing=False + ) if parsed_args.tag: - if compute_client.api_version < api_versions.APIVersion('2.49'): + if not sdk_utils.supports_microversion(compute_client, '2.49'): msg = _( '--os-compute-api-version 2.49 or greater is required to ' 'support the --tag option' ) raise exceptions.CommandError(msg) - kwargs['tag'] = parsed_args.tag + if self.app.client_manager.is_network_endpoint_enabled(): + network_client = self.app.client_manager.network + net_id = network_client.find_network( + parsed_args.network, + ignore_missing=False + ).id + else: + net_id = parsed_args.network + + if not sdk_utils.supports_microversion(compute_client, '2.44'): + compute_client.add_fixed_ip_to_server( + server.id, + net_id + ) + return + + kwargs = { + 'net_id': net_id, + 'fixed_ip': parsed_args.fixed_ip_address, + } - server.interface_attach(**kwargs) + if parsed_args.tag: + kwargs['tag'] = parsed_args.tag + compute_client.create_server_interface(server.id, **kwargs) class AddFloatingIP(network_common.NetworkAndComputeCommand): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 15be07fc6c..5d47b68784 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -217,104 +217,104 @@ def setUp(self): # Get the command object to test self.cmd = server.AddFixedIP(self.app, None) + # Mock network methods + self.find_network = mock.Mock() + self.app.client_manager.network.find_network = self.find_network + # Set add_fixed_ip method to be tested. self.methods = { 'interface_attach': None, } - def _test_server_add_fixed_ip(self, extralist, fixed_ip_address): - servers = self.setup_servers_mock(count=1) + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_add_fixed_ip_pre_2_44(self, sm_mock): + sm_mock.return_value = False + + servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() - with mock.patch( - 'openstackclient.api.compute_v2.APIv2.network_find' - ) as net_mock: - net_mock.return_value = network + with mock.patch.object( + self.app.client_manager, + 'is_network_endpoint_enabled', + return_value=False + ): arglist = [ servers[0].id, network['id'], - ] + extralist + ] verifylist = [ ('server', servers[0].id), ('network', network['id']), - ('fixed_ip_address', fixed_ip_address), + ('fixed_ip_address', None), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - servers[0].interface_attach.assert_called_once_with( - port_id=None, - net_id=network['id'], - fixed_ip=fixed_ip_address, + self.sdk_client.add_fixed_ip_to_server.assert_called_once_with( + servers[0].id, + network['id'] ) self.assertIsNone(result) - def test_server_add_fixed_ip(self): - self._test_server_add_fixed_ip([], None) - - def test_server_add_specific_fixed_ip(self): - extralist = ['--fixed-ip-address', '5.6.7.8'] - self._test_server_add_fixed_ip(extralist, '5.6.7.8') - - def test_server_add_fixed_ip_with_tag(self): - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.49') + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_add_fixed_ip_pre_2_44_with_fixed_ip(self, sm_mock): + sm_mock.return_value = False - servers = self.setup_servers_mock(count=1) + servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() - with mock.patch( - 'openstackclient.api.compute_v2.APIv2.network_find' - ) as net_mock: - net_mock.return_value = network + with mock.patch.object( + self.app.client_manager, + 'is_network_endpoint_enabled', + return_value=False + ): arglist = [ servers[0].id, network['id'], - '--fixed-ip-address', '5.6.7.8', - '--tag', 'tag1', + '--fixed-ip-address', '5.6.7.8' ] verifylist = [ ('server', servers[0].id), ('network', network['id']), ('fixed_ip_address', '5.6.7.8'), - ('tag', 'tag1'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) - servers[0].interface_attach.assert_called_once_with( - port_id=None, - net_id=network['id'], - fixed_ip='5.6.7.8', - tag='tag1' + self.sdk_client.add_fixed_ip_to_server.assert_called_once_with( + servers[0].id, + network['id'] ) self.assertIsNone(result) - def test_server_add_fixed_ip_with_tag_pre_v249(self): - self.app.client_manager.compute.api_version = api_versions.APIVersion( - '2.48') + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_add_fixed_ip_pre_2_44_with_tag(self, sm_mock): + sm_mock.return_value = False - servers = self.setup_servers_mock(count=1) + servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() - with mock.patch( - 'openstackclient.api.compute_v2.APIv2.network_find' - ) as net_mock: - net_mock.return_value = network + with mock.patch.object( + self.app.client_manager, + 'is_network_endpoint_enabled', + return_value=False + ): arglist = [ servers[0].id, network['id'], '--fixed-ip-address', '5.6.7.8', - '--tag', 'tag1', + '--tag', 'tag1' ] verifylist = [ ('server', servers[0].id), ('network', network['id']), ('fixed_ip_address', '5.6.7.8'), - ('tag', 'tag1'), + ('tag', 'tag1') ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + ex = self.assertRaises( exceptions.CommandError, self.cmd.take_action, @@ -323,6 +323,140 @@ def test_server_add_fixed_ip_with_tag_pre_v249(self): '--os-compute-api-version 2.49 or greater is required', str(ex)) + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_add_fixed_ip_pre_2_49_with_tag(self, sm_mock): + sm_mock.side_effect = [False, True] + + servers = self.setup_sdk_servers_mock(count=1) + network = compute_fakes.FakeNetwork.create_one_network() + + with mock.patch.object( + self.app.client_manager, + 'is_network_endpoint_enabled', + return_value=False + ): + arglist = [ + servers[0].id, + network['id'], + '--fixed-ip-address', '5.6.7.8', + '--tag', 'tag1' + ] + verifylist = [ + ('server', servers[0].id), + ('network', network['id']), + ('fixed_ip_address', '5.6.7.8'), + ('tag', 'tag1') + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + ex = self.assertRaises( + exceptions.CommandError, + self.cmd.take_action, + parsed_args) + self.assertIn( + '--os-compute-api-version 2.49 or greater is required', + str(ex)) + + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_add_fixed_ip_post_2_49(self, sm_mock): + sm_mock.side_effect = [True, True] + + servers = self.setup_sdk_servers_mock(count=1) + network = compute_fakes.FakeNetwork.create_one_network() + + with mock.patch.object( + self.app.client_manager, + 'is_network_endpoint_enabled', + return_value=False + ): + arglist = [ + servers[0].id, + network['id'] + ] + verifylist = [ + ('server', servers[0].id), + ('network', network['id']) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + self.sdk_client.create_server_interface.assert_called_once_with( + servers[0].id, + net_id=network['id'], + fixed_ip=None + ) + self.assertIsNone(result) + + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_add_fixed_ip_post_2_49_with_fixedip(self, sm_mock): + sm_mock.side_effect = [True, True] + + servers = self.setup_sdk_servers_mock(count=1) + network = compute_fakes.FakeNetwork.create_one_network() + + with mock.patch.object( + self.app.client_manager, + 'is_network_endpoint_enabled', + return_value=False + ): + arglist = [ + servers[0].id, + network['id'], + '--fixed-ip-address', '5.6.7.8' + ] + verifylist = [ + ('server', servers[0].id), + ('network', network['id']), + ('fixed_ip_address', '5.6.7.8') + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + self.sdk_client.create_server_interface.assert_called_once_with( + servers[0].id, + net_id=network['id'], + fixed_ip='5.6.7.8' + ) + self.assertIsNone(result) + + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_add_fixed_ip_post_2_49_with_tag(self, sm_mock): + sm_mock.side_effect = [True, True] + + servers = self.setup_sdk_servers_mock(count=1) + network = compute_fakes.FakeNetwork.create_one_network() + + with mock.patch.object( + self.app.client_manager, + 'is_network_endpoint_enabled', + return_value=False + ): + arglist = [ + servers[0].id, + network['id'], + '--fixed-ip-address', '5.6.7.8', + '--tag', 'tag1' + ] + verifylist = [ + ('server', servers[0].id), + ('network', network['id']), + ('fixed_ip_address', '5.6.7.8'), + ('tag', 'tag1') + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + self.sdk_client.create_server_interface.assert_called_once_with( + servers[0].id, + net_id=network['id'], + fixed_ip='5.6.7.8', + tag='tag1' + ) + self.assertIsNone(result) + @mock.patch( 'openstackclient.api.compute_v2.APIv2.floating_ip_add' diff --git a/releasenotes/notes/migrate-add-fixed-ip-to-sdk-3d932d77633bc765.yaml b/releasenotes/notes/migrate-add-fixed-ip-to-sdk-3d932d77633bc765.yaml new file mode 100644 index 0000000000..79899b3e92 --- /dev/null +++ b/releasenotes/notes/migrate-add-fixed-ip-to-sdk-3d932d77633bc765.yaml @@ -0,0 +1,3 @@ +features: + - | + Switch the add fixed IP command from novaclient to SDK. From 0cde82dcd86205f9e190b1a4f02136e1d83797b9 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 14 Dec 2021 15:14:49 +0000 Subject: [PATCH 029/697] compute: Return information about fixed IP The compute API provides this information to us. We might as well use it. Change-Id: I5608fa80745975ce49712718452cfe296c0f64d2 Signed-off-by: Stephen Finucane --- openstackclient/compute/v2/server.py | 33 ++++- .../tests/unit/compute/v2/fakes.py | 28 +++++ .../tests/unit/compute/v2/test_server.py | 117 +++++++++++++----- 3 files changed, 145 insertions(+), 33 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 0931a6ca01..2c1e42cc79 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -204,7 +204,7 @@ def boolenv(*vars, default=False): return default -class AddFixedIP(command.Command): +class AddFixedIP(command.ShowOne): _description = _("Add fixed IP address to server") def get_parser(self, prog_name): @@ -231,7 +231,7 @@ def get_parser(self, prog_name): metavar='', help=_( 'Tag for the attached interface. ' - '(supported by --os-compute-api-version 2.52 or above)' + '(supported by --os-compute-api-version 2.49 or above)' ) ) return parser @@ -265,16 +265,39 @@ def take_action(self, parsed_args): server.id, net_id ) - return + return ((), ()) kwargs = { 'net_id': net_id, 'fixed_ip': parsed_args.fixed_ip_address, } - if parsed_args.tag: kwargs['tag'] = parsed_args.tag - compute_client.create_server_interface(server.id, **kwargs) + + interface = compute_client.create_server_interface(server.id, **kwargs) + + columns = ( + 'port_id', 'server_id', 'net_id', 'mac_addr', 'port_state', + 'fixed_ips', + ) + column_headers = ( + 'Port ID', 'Server ID', 'Network ID', 'MAC Address', 'Port State', + 'Fixed IPs', + ) + if sdk_utils.supports_microversion(compute_client, '2.49'): + columns += ('tag',) + column_headers += ('Tag',) + + return ( + column_headers, + utils.get_item_properties( + interface, + columns, + formatters={ + 'fixed_ips': format_columns.ListDictColumn, + }, + ), + ) class AddFloatingIP(network_common.NetworkAndComputeCommand): diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index 7618c229c5..2a53164735 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -21,6 +21,7 @@ from novaclient import api_versions from openstack.compute.v2 import flavor as _flavor from openstack.compute.v2 import server +from openstack.compute.v2 import server_interface as _server_interface from openstack.compute.v2 import volume_attachment from openstackclient.api import compute_v2 @@ -1859,3 +1860,30 @@ def create_sdk_volume_attachments(attrs=None, methods=None, count=2): attrs, methods)) return volume_attachments + + +def create_one_server_interface(attrs=None): + """Create a fake SDK ServerInterface. + + :param dict attrs: A dictionary with all attributes + :param dict methods: A dictionary with all methods + :return: A fake ServerInterface object with various attributes set + """ + attrs = attrs or {} + + # Set default attributes. + server_interface_info = { + "fixed_ips": uuid.uuid4().hex, + "mac_addr": "aa:aa:aa:aa:aa:aa", + "net_id": uuid.uuid4().hex, + "port_id": uuid.uuid4().hex, + "port_state": "ACTIVE", + "server_id": uuid.uuid4().hex, + # introduced in API microversion 2.70 + "tag": "foo", + } + + # Overwrite default attributes. + server_interface_info.update(attrs) + + return _server_interface.ServerInterface(**server_interface_info) diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 5d47b68784..6553f90465 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -212,7 +212,7 @@ def run_method_with_sdk_servers(self, method_name, server_count): class TestServerAddFixedIP(TestServer): def setUp(self): - super(TestServerAddFixedIP, self).setUp() + super().setUp() # Get the command object to test self.cmd = server.AddFixedIP(self.app, None) @@ -221,13 +221,8 @@ def setUp(self): self.find_network = mock.Mock() self.app.client_manager.network.find_network = self.find_network - # Set add_fixed_ip method to be tested. - self.methods = { - 'interface_attach': None, - } - @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_pre_2_44(self, sm_mock): + def test_server_add_fixed_ip_pre_v244(self, sm_mock): sm_mock.return_value = False servers = self.setup_sdk_servers_mock(count=1) @@ -255,10 +250,11 @@ def test_server_add_fixed_ip_pre_2_44(self, sm_mock): servers[0].id, network['id'] ) - self.assertIsNone(result) + # the legacy API operates asynchronously + self.assertEqual(((), ()), result) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_pre_2_44_with_fixed_ip(self, sm_mock): + def test_server_add_fixed_ip_pre_v244_with_fixed_ip(self, sm_mock): sm_mock.return_value = False servers = self.setup_sdk_servers_mock(count=1) @@ -287,10 +283,11 @@ def test_server_add_fixed_ip_pre_2_44_with_fixed_ip(self, sm_mock): servers[0].id, network['id'] ) - self.assertIsNone(result) + # the legacy API operates asynchronously + self.assertEqual(((), ()), result) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_pre_2_44_with_tag(self, sm_mock): + def test_server_add_fixed_ip_pre_v244_with_tag(self, sm_mock): sm_mock.return_value = False servers = self.setup_sdk_servers_mock(count=1) @@ -324,7 +321,7 @@ def test_server_add_fixed_ip_pre_2_44_with_tag(self, sm_mock): str(ex)) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_pre_2_49_with_tag(self, sm_mock): + def test_server_add_fixed_ip_pre_v249_with_tag(self, sm_mock): sm_mock.side_effect = [False, True] servers = self.setup_sdk_servers_mock(count=1) @@ -358,11 +355,13 @@ def test_server_add_fixed_ip_pre_2_49_with_tag(self, sm_mock): str(ex)) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_post_2_49(self, sm_mock): - sm_mock.side_effect = [True, True] + def test_server_add_fixed_ip(self, sm_mock): + sm_mock.side_effect = [True, False] servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() + interface = compute_fakes.create_one_server_interface() + self.sdk_client.create_server_interface.return_value = interface with mock.patch.object( self.app.client_manager, @@ -379,21 +378,41 @@ def test_server_add_fixed_ip_post_2_49(self, sm_mock): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + expected_columns = ( + 'Port ID', + 'Server ID', + 'Network ID', + 'MAC Address', + 'Port State', + 'Fixed IPs', + ) + expected_data = ( + interface.port_id, + interface.server_id, + interface.net_id, + interface.mac_addr, + interface.port_state, + format_columns.ListDictColumn(interface.fixed_ips), + ) + + columns, data = self.cmd.take_action(parsed_args) + self.assertEqual(expected_columns, columns) + self.assertEqual(expected_data, tuple(data)) self.sdk_client.create_server_interface.assert_called_once_with( servers[0].id, net_id=network['id'], fixed_ip=None ) - self.assertIsNone(result) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_post_2_49_with_fixedip(self, sm_mock): + def test_server_add_fixed_ip_with_fixed_ip(self, sm_mock): sm_mock.side_effect = [True, True] servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() + interface = compute_fakes.create_one_server_interface() + self.sdk_client.create_server_interface.return_value = interface with mock.patch.object( self.app.client_manager, @@ -412,21 +431,43 @@ def test_server_add_fixed_ip_post_2_49_with_fixedip(self, sm_mock): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + expected_columns = ( + 'Port ID', + 'Server ID', + 'Network ID', + 'MAC Address', + 'Port State', + 'Fixed IPs', + 'Tag', + ) + expected_data = ( + interface.port_id, + interface.server_id, + interface.net_id, + interface.mac_addr, + interface.port_state, + format_columns.ListDictColumn(interface.fixed_ips), + interface.tag, + ) + columns, data = self.cmd.take_action(parsed_args) + + self.assertEqual(expected_columns, columns) + self.assertEqual(expected_data, tuple(data)) self.sdk_client.create_server_interface.assert_called_once_with( servers[0].id, net_id=network['id'], fixed_ip='5.6.7.8' ) - self.assertIsNone(result) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_post_2_49_with_tag(self, sm_mock): - sm_mock.side_effect = [True, True] + def test_server_add_fixed_ip_with_tag(self, sm_mock): + sm_mock.side_effect = [True, True, True] servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() + interface = compute_fakes.create_one_server_interface() + self.sdk_client.create_server_interface.return_value = interface with mock.patch.object( self.app.client_manager, @@ -447,15 +488,35 @@ def test_server_add_fixed_ip_post_2_49_with_tag(self, sm_mock): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + expected_columns = ( + 'Port ID', + 'Server ID', + 'Network ID', + 'MAC Address', + 'Port State', + 'Fixed IPs', + 'Tag', + ) + expected_data = ( + interface.port_id, + interface.server_id, + interface.net_id, + interface.mac_addr, + interface.port_state, + format_columns.ListDictColumn(interface.fixed_ips), + interface.tag, + ) + columns, data = self.cmd.take_action(parsed_args) + + self.assertEqual(expected_columns, columns) + self.assertEqual(expected_data, tuple(data)) self.sdk_client.create_server_interface.assert_called_once_with( servers[0].id, net_id=network['id'], fixed_ip='5.6.7.8', - tag='tag1' + tag='tag1', ) - self.assertIsNone(result) @mock.patch( @@ -5265,7 +5326,7 @@ def test_server_migrate_with_disk_overcommit(self): self.assertNotCalled(self.servers_mock.live_migrate) self.assertNotCalled(self.servers_mock.migrate) - def test_server_migrate_with_host_pre_2_56(self): + def test_server_migrate_with_host_pre_v256(self): # Tests that --host is not allowed for a cold migration # before microversion 2.56 (the test defaults to 2.1). arglist = [ @@ -6682,7 +6743,7 @@ def test_rebuild_with_user_data(self, mock_open): self.image, None, userdata=mock_file,) - def test_rebuild_with_user_data_pre_257(self): + def test_rebuild_with_user_data_pre_v257(self): self.app.client_manager.compute.api_version = \ api_versions.APIVersion('2.56') @@ -6722,7 +6783,7 @@ def test_rebuild_with_no_user_data(self): self.server.rebuild.assert_called_with( self.image, None, userdata=None) - def test_rebuild_with_no_user_data_pre_254(self): + def test_rebuild_with_no_user_data_pre_v254(self): self.app.client_manager.compute.api_version = \ api_versions.APIVersion('2.53') @@ -6814,7 +6875,7 @@ def test_rebuild_with_no_trusted_image_cert(self): self.server.rebuild.assert_called_with( self.image, None, trusted_image_certificates=None) - def test_rebuild_with_no_trusted_image_cert_pre_257(self): + def test_rebuild_with_no_trusted_image_cert_pre_v263(self): self.app.client_manager.compute.api_version = \ api_versions.APIVersion('2.62') From ba69870d86b5840dec06c6c30c8ddf50398bdb44 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 15 Dec 2021 17:32:10 +0000 Subject: [PATCH 030/697] compute: Fix weird option definition for 'server ssh' argparse allows you to specify multiple options for a given argument when declaring the argument. For some reason, we weren't doing this for the 'server ssh' command. There's no apparent reason for doing things this way and it's been that way since the beginning (2013) so let's not do that. We also add unit tests since they were missing and should exist. Change-Id: I67a9e6516d7057266210cd4083e9ddeb1cfaa5de Signed-off-by: Stephen Finucane --- openstackclient/compute/v2/server.py | 33 +------- .../tests/unit/compute/v2/test_server.py | 77 +++++++++++++++++++ 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index a18ce81012..5c603d04da 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -4462,51 +4462,26 @@ def get_parser(self, prog_name): help=_('Server (name or ID)'), ) parser.add_argument( - '--login', + '--login', '-l', metavar='', help=_('Login name (ssh -l option)'), ) parser.add_argument( - '-l', - dest='login', - metavar='', - help=argparse.SUPPRESS, - ) - parser.add_argument( - '--port', + '--port', '-p', metavar='', type=int, help=_('Destination port (ssh -p option)'), ) parser.add_argument( - '-p', - metavar='', - dest='port', - type=int, - help=argparse.SUPPRESS, - ) - parser.add_argument( - '--identity', + '--identity', '-i', metavar='', help=_('Private key file (ssh -i option)'), ) parser.add_argument( - '-i', - metavar='', - dest='identity', - help=argparse.SUPPRESS, - ) - parser.add_argument( - '--option', + '--option', '-o', metavar='', help=_('Options in ssh_config(5) format (ssh -o option)'), ) - parser.add_argument( - '-o', - metavar='