openstack/python-openstackclient
Code Issues Proposed changes

compute: Address bug in shelve offload logic

Browse Source 
We were reusing a variable from a previous loop, which meant this would
never work with multiple servers. Correct the mistake.

Change-Id: I52246e183fb2cf0d855d92058dd305b48783589d
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane
2023-11-15 11:19:14 +00:00
parent 1678f87d7a
commit 885f5912eb
2 changed files with 25 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
openstackclient/compute/v2/server.py
View File

@@ -4416,6 +4416,7 @@ class ShelveServer(command.Command):
self.app.stdout.flush() self.app.stdout.flush()
compute_client = self.app.client_manager.sdk_connection.compute compute_client = self.app.client_manager.sdk_connection.compute
server_ids = []
for server in parsed_args.servers: for server in parsed_args.servers:
server_obj = compute_client.find_server( server_obj = compute_client.find_server(
@@ -4425,6 +4426,8 @@ class ShelveServer(command.Command):
if server_obj.status.lower() in ('shelved', 'shelved_offloaded'): if server_obj.status.lower() in ('shelved', 'shelved_offloaded'):
continue continue
server_ids.append(server_obj.id)
compute_client.shelve_server(server_obj.id) compute_client.shelve_server(server_obj.id)
# if we don't have to wait, either because it was requested explicitly # if we don't have to wait, either because it was requested explicitly
@@ -4432,54 +4435,51 @@ class ShelveServer(command.Command):
if not parsed_args.wait and not parsed_args.offload: if not parsed_args.wait and not parsed_args.offload:
return return
for server in parsed_args.servers: for server_id in server_ids:
# We use osc-lib's wait_for_status since that allows for a callback # We use osc-lib's wait_for_status since that allows for a callback
# TODO(stephenfin): We should wait for these in parallel using e.g. # TODO(stephenfin): We should wait for these in parallel using e.g.
# https://review.opendev.org/c/openstack/osc-lib/+/762503/ # https://review.opendev.org/c/openstack/osc-lib/+/762503/
if not utils.wait_for_status( if not utils.wait_for_status(
compute_client.get_server, compute_client.get_server,
server_obj.id, server_id,
success_status=('shelved', 'shelved_offloaded'), success_status=('shelved', 'shelved_offloaded'),
callback=_show_progress, callback=_show_progress,
): ):
LOG.error(_('Error shelving server: %s'), server_obj.id) LOG.error(_('Error shelving server: %s'), server_id)
self.app.stdout.write( self.app.stdout.write(
_('Error shelving server: %s\n') % server_obj.id _('Error shelving server: %s\n') % server_id
) )
raise SystemExit raise SystemExit
if not parsed_args.offload: if not parsed_args.offload:
return return
for server in parsed_args.servers: for server_id in server_ids:
server_obj = compute_client.find_server( server_obj = compute_client.get_server(server_id)
server,
ignore_missing=False,
)
if server_obj.status.lower() == 'shelved_offloaded': if server_obj.status.lower() == 'shelved_offloaded':
continue continue
compute_client.shelve_offload_server(server_obj.id) compute_client.shelve_offload_server(server_id)
if not parsed_args.wait: if not parsed_args.wait:
return return
for server in parsed_args.servers: for server_id in server_ids:
# We use osc-lib's wait_for_status since that allows for a callback # We use osc-lib's wait_for_status since that allows for a callback
# TODO(stephenfin): We should wait for these in parallel using e.g. # TODO(stephenfin): We should wait for these in parallel using e.g.
# https://review.opendev.org/c/openstack/osc-lib/+/762503/ # https://review.opendev.org/c/openstack/osc-lib/+/762503/
if not utils.wait_for_status( if not utils.wait_for_status(
compute_client.get_server, compute_client.get_server,
server_obj.id, server_id,
success_status=('shelved_offloaded',), success_status=('shelved_offloaded',),
callback=_show_progress, callback=_show_progress,
): ):
LOG.error( LOG.error(
_('Error offloading shelved server %s'), _('Error offloading shelved server %s'),
server_obj.id, server_id,
) )
self.app.stdout.write( self.app.stdout.write(
_('Error offloading shelved server: %s\n') % server_obj.id _('Error offloading shelved server: %s\n') % server_id
) )
raise SystemExit raise SystemExit

18
openstackclient/tests/unit/compute/v2/test_server.py
View File

@@ -8149,20 +8149,24 @@ class TestServerShelve(TestServer):
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
self.assertIsNone(result) self.assertIsNone(result)
# two calls - one to retrieve the server state before shelving and # one call to retrieve to retrieve the server state before shelving
# another to do this before offloading self.compute_sdk_client.find_server.assert_called_once_with(
self.compute_sdk_client.find_server.assert_has_calls( self.server.name,
[ ignore_missing=False,
mock.call(self.server.name, ignore_missing=False),
mock.call(self.server.name, ignore_missing=False),
]
) )
# one call to retrieve the server state before offloading
self.compute_sdk_client.get_server.assert_called_once_with(
self.server.id
)
# one call to shelve the server
self.compute_sdk_client.shelve_server.assert_called_with( self.compute_sdk_client.shelve_server.assert_called_with(
self.server.id self.server.id
) )
# one call to shelve offload the server
self.compute_sdk_client.shelve_offload_server.assert_called_once_with( self.compute_sdk_client.shelve_offload_server.assert_called_once_with(
self.server.id, self.server.id,
) )
# one call to wait for the shelve offload to complete
mock_wait_for_status.assert_called_once_with( mock_wait_for_status.assert_called_once_with(
self.compute_sdk_client.get_server, self.compute_sdk_client.get_server,
self.server.id, self.server.id,