Skip to content

Commit 9726a4d

Browse 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

6 files changed

Lines changed: 161 additions & 6 deletions

File tree

‎openstackclient/common/availability_zone.py‎

Lines 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
)

‎openstackclient/common/extension.py‎

Lines 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

‎openstackclient/compute/v2/server.py‎

Lines 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,

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

Lines 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',

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

Lines 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',

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

Lines 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)