Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 9726a4d

Browse filesBrowse files
slawqoslawqo
committed
Make block-storage endpoint optional for select commands
Several commands unconditionally initialized the Cinder client even when block-storage was not present in the service catalog. This caused failures such as "public endpoint for block-storage service not found" on clouds without Cinder. Defer volume client initialization until it is actually needed, using the existing is_volume_endpoint_enabled() helper: - server create: only load Cinder when --volume, --snapshot, or --block-device-mapping with volume/snapshot sources are used - extension list: skip block-storage extensions when unavailable - availability zone list: skip volume availability zones when unavailable Volume-related options still require Cinder and now fail with a clear error when the endpoint is missing. Closes-Bug: #2160039 Assisted-by: Cursor Composer 2.5 Change-Id: I79e2efe3e79727c7055ca47c0eca05833be6eb1b Signed-off-by: Slawek Kaplonski <skaplons@redhat.com>
1 parent caec5c3 commit 9726a4d
Copy full SHA for 9726a4d

6 files changed

+161-6Lines changed: 161 additions & 6 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎openstackclient/common/availability_zone.py‎

Copy file name to clipboardExpand all lines: openstackclient/common/availability_zone.py
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,14 @@ def _get_share_availability_zone(
205205
def _get_volume_availability_zones(
206206
self, parsed_args: argparse.Namespace
207207
) -> list[dict[str, str]]:
208+
if not self.app.client_manager.is_volume_endpoint_enabled():
209+
if parsed_args.volume:
210+
message = _(
211+
"Block Storage API is not available in the current cloud"
212+
)
213+
LOG.warning(message)
214+
return []
215+
208216
volume_client = sdk_utils.ensure_service_version(
209217
self.app.client_manager.volume, '3'
210218
)
Collapse file

‎openstackclient/common/extension.py‎

Copy file name to clipboardExpand all lines: openstackclient/common/extension.py
+11-5Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,18 @@ def take_action(
120120
LOG.warning(message)
121121

122122
if parsed_args.volume or show_all:
123-
volume_client = self.app.client_manager.volume
124-
try:
125-
data += volume_client.extensions()
126-
except Exception:
123+
if self.app.client_manager.is_volume_endpoint_enabled():
124+
volume_client = self.app.client_manager.volume
125+
try:
126+
data += volume_client.extensions()
127+
except Exception:
128+
message = _(
129+
"Extensions list not supported by Block Storage API"
130+
)
131+
LOG.warning(message)
132+
elif parsed_args.volume:
127133
message = _(
128-
"Extensions list not supported by Block Storage API"
134+
"Block Storage API is not available in the current cloud"
129135
)
130136
LOG.warning(message)
131137

Collapse file

‎openstackclient/compute/v2/server.py‎

Copy file name to clipboardExpand all lines: openstackclient/compute/v2/server.py
+28-1Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,26 @@ def __call__(
11431143
super().__call__(parser, namespace, values, option_string)
11441144

11451145

1146+
def _server_create_needs_volume_client(
1147+
parsed_args: argparse.Namespace,
1148+
) -> bool:
1149+
if parsed_args.volume or parsed_args.snapshot:
1150+
return True
1151+
1152+
for mapping in parsed_args.block_device_mapping:
1153+
if mapping['source_type'] in ('volume', 'snapshot'):
1154+
return True
1155+
1156+
return False
1157+
1158+
1159+
def _get_required_volume_client(app: Any) -> Any:
1160+
if not app.client_manager.is_volume_endpoint_enabled():
1161+
msg = _('Volume service is not available in the current cloud')
1162+
raise exceptions.CommandError(msg)
1163+
return app.client_manager.volume
1164+
1165+
11461166
class CreateServer(command.ShowOne):
11471167
_description = _("Create a new server")
11481168

@@ -1596,9 +1616,12 @@ def _show_progress(progress: int | None) -> None:
15961616
self.app.stdout.flush()
15971617

15981618
compute_client = self.app.client_manager.compute
1599-
volume_client = self.app.client_manager.volume
16001619
image_client = self.app.client_manager.image
16011620

1621+
volume_client = None
1622+
if _server_create_needs_volume_client(parsed_args):
1623+
volume_client = _get_required_volume_client(self.app)
1624+
16021625
# Lookup parsed_args.image
16031626
image = None
16041627
if parsed_args.image:
@@ -1669,6 +1692,7 @@ def _match_image(image_api: Any, wanted_properties: Any) -> Any:
16691692
msg = _('--volume is not allowed with --boot-from-volume')
16701693
raise exceptions.CommandError(msg)
16711694

1695+
assert volume_client is not None # narrow type
16721696
volume = volume_client.find_volume(
16731697
parsed_args.volume,
16741698
ignore_missing=False,
@@ -1681,6 +1705,7 @@ def _match_image(image_api: Any, wanted_properties: Any) -> Any:
16811705
msg = _('--snapshot is not allowed with --boot-from-volume')
16821706
raise exceptions.CommandError(msg)
16831707

1708+
assert volume_client is not None # narrow type
16841709
snapshot = volume_client.find_snapshot(
16851710
parsed_args.snapshot,
16861711
ignore_missing=False,
@@ -1823,12 +1848,14 @@ def _match_image(image_api: Any, wanted_properties: Any) -> Any:
18231848
# The 'uuid' field isn't necessarily a UUID yet; let's validate it
18241849
# just in case
18251850
if mapping['source_type'] == 'volume':
1851+
assert volume_client is not None # narrow type
18261852
volume_id = volume_client.find_volume(
18271853
mapping['uuid'],
18281854
ignore_missing=False,
18291855
).id
18301856
mapping['uuid'] = volume_id
18311857
elif mapping['source_type'] == 'snapshot':
1858+
assert volume_client is not None # narrow type
18321859
snapshot_id = volume_client.find_snapshot(
18331860
mapping['uuid'],
18341861
ignore_missing=False,
Collapse file

‎openstackclient/tests/unit/common/test_availability_zone.py‎

Copy file name to clipboardExpand all lines: openstackclient/tests/unit/common/test_availability_zone.py
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,30 @@ def test_availability_zone_list_no_options(self):
146146
datalist += _build_volume_az_datalist(volume_az)
147147
self.assertEqual(datalist, tuple(data))
148148

149+
def test_availability_zone_list_no_volume_endpoint(self):
150+
self.app.client_manager.volume_endpoint_enabled = False
151+
152+
arglist = []
153+
verifylist = []
154+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
155+
156+
columns, data = self.cmd.take_action(parsed_args)
157+
158+
self.compute_client.availability_zones.assert_called_with(details=True)
159+
self.network_client.availability_zones.assert_called_with()
160+
self.share_sdk_client.availability_zones.assert_called_with()
161+
self.volume_client.availability_zones.assert_not_called()
162+
163+
self.assertEqual(self.short_columnslist, columns)
164+
datalist = ()
165+
for compute_az in self.compute_azs:
166+
datalist += _build_compute_az_datalist(compute_az)
167+
for network_az in self.network_azs:
168+
datalist += _build_network_az_datalist(network_az)
169+
for share_az in self.share_azs:
170+
datalist += _build_share_az_datalist(share_az)
171+
self.assertEqual(datalist, tuple(data))
172+
149173
def test_availability_zone_list_long(self):
150174
arglist = [
151175
'--long',
Collapse file

‎openstackclient/tests/unit/common/test_extension.py‎

Copy file name to clipboardExpand all lines: openstackclient/tests/unit/common/test_extension.py
+25Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,31 @@ def test_extension_list_no_options(self):
113113
self.volume_client.extensions.assert_called_with()
114114
self.network_client.extensions.assert_called_with()
115115

116+
def test_extension_list_no_volume_endpoint(self):
117+
self.app.client_manager.volume_endpoint_enabled = False
118+
119+
arglist = []
120+
verifylist = []
121+
datalist = (
122+
(
123+
self.identity_extension.name,
124+
self.identity_extension.alias,
125+
self.identity_extension.description,
126+
),
127+
(
128+
self.compute_extension.name,
129+
self.compute_extension.alias,
130+
self.compute_extension.description,
131+
),
132+
(
133+
self.network_extension.name,
134+
self.network_extension.alias,
135+
self.network_extension.description,
136+
),
137+
)
138+
self._test_extension_list_helper(arglist, verifylist, datalist)
139+
self.volume_client.extensions.assert_not_called()
140+
116141
def test_extension_list_long(self):
117142
arglist = [
118143
'--long',
Collapse file

‎openstackclient/tests/unit/compute/v2/test_server.py‎

Copy file name to clipboardExpand all lines: openstackclient/tests/unit/compute/v2/test_server.py
+65Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,6 +1307,71 @@ def test_server_create_minimal(self):
13071307
self.assertEqual(self.columns, columns)
13081308
self.assertEqual(self.datalist(), data)
13091309

1310+
def test_server_create_without_volume_endpoint(self):
1311+
self.app.client_manager.volume_endpoint_enabled = False
1312+
1313+
arglist = [
1314+
'--image',
1315+
self.image.id,
1316+
'--flavor',
1317+
self.flavor.id,
1318+
self.server.name,
1319+
]
1320+
verifylist = [
1321+
('image', self.image.id),
1322+
('flavor', self.flavor.id),
1323+
('config_drive', False),
1324+
('server_name', self.server.name),
1325+
]
1326+
1327+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1328+
columns, data = self.cmd.take_action(parsed_args)
1329+
1330+
self.compute_client.create_server.assert_called_once_with(
1331+
name=self.server.name,
1332+
image_id=self.image.id,
1333+
flavor_id=self.flavor.id,
1334+
min_count=1,
1335+
max_count=1,
1336+
networks=[],
1337+
block_device_mapping=[
1338+
{
1339+
'uuid': self.image.id,
1340+
'boot_index': 0,
1341+
'source_type': 'image',
1342+
'destination_type': 'local',
1343+
'delete_on_termination': True,
1344+
},
1345+
],
1346+
)
1347+
self.assertEqual(self.columns, columns)
1348+
self.assertEqual(self.datalist(), data)
1349+
1350+
def test_server_create_volume_requires_volume_endpoint(self):
1351+
self.app.client_manager.volume_endpoint_enabled = False
1352+
1353+
arglist = [
1354+
'--flavor',
1355+
self.flavor.id,
1356+
'--volume',
1357+
self.volume.name,
1358+
self.server.name,
1359+
]
1360+
verifylist = [
1361+
('flavor', self.flavor.id),
1362+
('volume', self.volume.name),
1363+
('server_name', self.server.name),
1364+
]
1365+
1366+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1367+
1368+
ex = self.assertRaises(
1369+
exceptions.CommandError, self.cmd.take_action, parsed_args
1370+
)
1371+
self.assertIn(
1372+
'Volume service is not available in the current cloud', str(ex)
1373+
)
1374+
13101375
def test_server_create_with_options(self):
13111376
server_group = sdk_fakes.generate_fake_resource(
13121377
_server_group.ServerGroup

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.