Skip to content

Commit 339af2c

Browse files
author
Dean Troyer
committed
Fix floating IP delete and show by IP
The floating IP delete and show commands did not work using IP addresses as the selector, only ID. The SDK floating_ip resource does not support but OSC does, so we have to do it ourselves. Now with more SDK 0.9.10 support! Change-Id: Iea1b57cded6b16a56a06af87ab8f1fa001a3485e Closes-bug: 1656402
1 parent 339ab40 commit 339af2c

5 files changed

Lines changed: 157 additions & 27 deletions

File tree

‎openstackclient/network/v2/floating_ip.py‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
import logging
1717

18+
from openstack import exceptions as sdk_exceptions
19+
from openstack.network.v2 import floating_ip as _floating_ip
1820
from osc_lib import utils
1921

2022
from openstackclient.i18n import _
@@ -79,6 +81,58 @@ def _get_attrs(client_manager, parsed_args):
7981
return attrs
8082

8183

84+
def _find_floating_ip(
85+
session,
86+
ip_cache,
87+
name_or_id,
88+
ignore_missing=True,
89+
**params
90+
):
91+
"""Find a floating IP by IP or ID
92+
93+
The SDK's find_ip() can only locate a floating IP by ID so we have
94+
to do this ourselves.
95+
"""
96+
97+
def _get_one_match(name_or_id):
98+
"""Given a list of results, return the match"""
99+
the_result = None
100+
for maybe_result in ip_cache:
101+
id_value = maybe_result.id
102+
ip_value = maybe_result.floating_ip_address
103+
104+
if (id_value == name_or_id) or (ip_value == name_or_id):
105+
# Only allow one resource to be found. If we already
106+
# found a match, raise an exception to show it.
107+
if the_result is None:
108+
the_result = maybe_result
109+
else:
110+
msg = "More than one %s exists with the name '%s'."
111+
msg = (msg % (_floating_ip.FloatingIP, name_or_id))
112+
raise sdk_exceptions.DuplicateResource(msg)
113+
114+
return the_result
115+
116+
# Try to short-circuit by looking directly for a matching ID.
117+
try:
118+
match = _floating_ip.FloatingIP.existing(id=name_or_id, **params)
119+
return (match.get(session), ip_cache)
120+
except sdk_exceptions.NotFoundException:
121+
pass
122+
123+
if len(ip_cache) == 0:
124+
ip_cache = list(_floating_ip.FloatingIP.list(session, **params))
125+
126+
result = _get_one_match(name_or_id)
127+
if result is not None:
128+
return (result, ip_cache)
129+
130+
if ignore_missing:
131+
return (None, ip_cache)
132+
raise sdk_exceptions.ResourceNotFound(
133+
"No %s found for %s" % (_floating_ip.FloatingIP.__name__, name_or_id))
134+
135+
82136
class CreateFloatingIP(common.NetworkAndComputeShowOne):
83137
_description = _("Create floating IP")
84138

@@ -186,13 +240,28 @@ def update_parser_common(self, parser):
186240
return parser
187241

188242
def take_action_network(self, client, parsed_args):
189-
obj = client.find_ip(self.r, ignore_missing=False)
243+
(obj, self.ip_cache) = _find_floating_ip(
244+
client.session,
245+
self.ip_cache,
246+
self.r,
247+
ignore_missing=False,
248+
)
190249
client.delete_ip(obj)
191250

192251
def take_action_compute(self, client, parsed_args):
193252
obj = utils.find_resource(client.floating_ips, self.r)
194253
client.floating_ips.delete(obj.id)
195254

255+
def take_action(self, parsed_args):
256+
"""Implements a naive cache for the list of floating IPs"""
257+
258+
# NOTE(dtroyer): This really only prevents multiple list()
259+
# calls when performing multiple resource deletes
260+
# in a single command. In an interactive session
261+
# each delete command will call list().
262+
self.ip_cache = []
263+
super(DeleteFloatingIP, self).take_action(parsed_args)
264+
196265

197266
class DeleteIPFloating(DeleteFloatingIP):
198267
_description = _("Delete floating IP(s)")
@@ -390,6 +459,9 @@ def take_action_compute(self, client, parsed_args):
390459
class ShowFloatingIP(common.NetworkAndComputeShowOne):
391460
_description = _("Display floating IP details")
392461

462+
# ip_cache is unused here but is a side effect of _find_floating_ip()
463+
ip_cache = []
464+
393465
def update_parser_common(self, parser):
394466
parser.add_argument(
395467
'floating_ip',
@@ -399,7 +471,12 @@ def update_parser_common(self, parser):
399471
return parser
400472

401473
def take_action_network(self, client, parsed_args):
402-
obj = client.find_ip(parsed_args.floating_ip, ignore_missing=False)
474+
(obj, self.ip_cache) = _find_floating_ip(
475+
client.session,
476+
[],
477+
parsed_args.floating_ip,
478+
ignore_missing=False,
479+
)
403480
display_columns, columns = _get_network_columns(obj)
404481
data = utils.get_item_properties(obj, columns)
405482
return (display_columns, data)

‎openstackclient/tests/functional/network/v2/test_floating_ip.py‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
import re
1515
import uuid
1616

17-
import testtools
18-
1917
from openstackclient.tests.functional import base
2018

2119

@@ -25,7 +23,6 @@ class FloatingIpTests(base.TestCase):
2523
NETWORK_NAME = uuid.uuid4().hex
2624

2725
@classmethod
28-
@testtools.skip('broken SDK testing')
2926
def setUpClass(cls):
3027
# Set up some regex for matching below
3128
cls.re_id = re.compile("id\s+\|\s+(\S+)")
@@ -62,7 +59,6 @@ def tearDownClass(cls):
6259
raw_output = cls.openstack('network delete ' + cls.NETWORK_NAME)
6360
cls.assertOutput('', raw_output)
6461

65-
@testtools.skip('broken SDK testing')
6662
def test_floating_ip_delete(self):
6763
"""Test create, delete multiple"""
6864
raw_output = self.openstack(
@@ -93,7 +89,6 @@ def test_floating_ip_delete(self):
9389
raw_output = self.openstack('floating ip delete ' + ip1 + ' ' + ip2)
9490
self.assertOutput('', raw_output)
9591

96-
@testtools.skip('broken SDK testing')
9792
def test_floating_ip_list(self):
9893
"""Test create defaults, list filters, delete"""
9994
raw_output = self.openstack(
@@ -135,7 +130,6 @@ def test_floating_ip_list(self):
135130

136131
# TODO(dtroyer): add more filter tests
137132

138-
@testtools.skip('broken SDK testing')
139133
def test_floating_ip_show(self):
140134
"""Test show"""
141135
raw_output = self.openstack(

‎openstackclient/tests/unit/network/v2/fakes.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
class FakeNetworkV2Client(object):
5353

5454
def __init__(self, **kwargs):
55+
self.session = mock.Mock()
5556
self.extensions = mock.Mock()
5657
self.extensions.resource_class = fakes.FakeResource(None, {})
5758

‎openstackclient/tests/unit/network/v2/test_floating_ip.py‎

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -208,13 +208,19 @@ def setUp(self):
208208
super(TestDeleteFloatingIPNetwork, self).setUp()
209209

210210
self.network.delete_ip = mock.Mock(return_value=None)
211-
self.network.find_ip = (
212-
network_fakes.FakeFloatingIP.get_floating_ips(self.floating_ips))
213211

214212
# Get the command object to test
215213
self.cmd = floating_ip.DeleteFloatingIP(self.app, self.namespace)
216214

217-
def test_floating_ip_delete(self):
215+
@mock.patch(
216+
"openstackclient.tests.unit.network.v2.test_floating_ip." +
217+
"floating_ip._find_floating_ip"
218+
)
219+
def test_floating_ip_delete(self, find_floating_ip_mock):
220+
find_floating_ip_mock.side_effect = [
221+
(self.floating_ips[0], []),
222+
(self.floating_ips[1], []),
223+
]
218224
arglist = [
219225
self.floating_ips[0].id,
220226
]
@@ -225,12 +231,24 @@ def test_floating_ip_delete(self):
225231

226232
result = self.cmd.take_action(parsed_args)
227233

228-
self.network.find_ip.assert_called_once_with(
229-
self.floating_ips[0].id, ignore_missing=False)
234+
find_floating_ip_mock.assert_called_once_with(
235+
mock.ANY,
236+
[],
237+
self.floating_ips[0].id,
238+
ignore_missing=False,
239+
)
230240
self.network.delete_ip.assert_called_once_with(self.floating_ips[0])
231241
self.assertIsNone(result)
232242

233-
def test_multi_floating_ips_delete(self):
243+
@mock.patch(
244+
"openstackclient.tests.unit.network.v2.test_floating_ip." +
245+
"floating_ip._find_floating_ip"
246+
)
247+
def test_floating_ip_delete_multi(self, find_floating_ip_mock):
248+
find_floating_ip_mock.side_effect = [
249+
(self.floating_ips[0], []),
250+
(self.floating_ips[1], []),
251+
]
234252
arglist = []
235253
verifylist = []
236254

@@ -243,13 +261,37 @@ def test_multi_floating_ips_delete(self):
243261

244262
result = self.cmd.take_action(parsed_args)
245263

264+
calls = [
265+
call(
266+
mock.ANY,
267+
[],
268+
self.floating_ips[0].id,
269+
ignore_missing=False,
270+
),
271+
call(
272+
mock.ANY,
273+
[],
274+
self.floating_ips[1].id,
275+
ignore_missing=False,
276+
),
277+
]
278+
find_floating_ip_mock.assert_has_calls(calls)
279+
246280
calls = []
247281
for f in self.floating_ips:
248282
calls.append(call(f))
249283
self.network.delete_ip.assert_has_calls(calls)
250284
self.assertIsNone(result)
251285

252-
def test_multi_floating_ips_delete_with_exception(self):
286+
@mock.patch(
287+
"openstackclient.tests.unit.network.v2.test_floating_ip." +
288+
"floating_ip._find_floating_ip"
289+
)
290+
def test_floating_ip_delete_multi_exception(self, find_floating_ip_mock):
291+
find_floating_ip_mock.side_effect = [
292+
(self.floating_ips[0], []),
293+
exceptions.CommandError,
294+
]
253295
arglist = [
254296
self.floating_ips[0].id,
255297
'unexist_floating_ip',
@@ -260,21 +302,24 @@ def test_multi_floating_ips_delete_with_exception(self):
260302
]
261303
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
262304

263-
find_mock_result = [self.floating_ips[0], exceptions.CommandError]
264-
self.network.find_ip = (
265-
mock.Mock(side_effect=find_mock_result)
266-
)
267-
268305
try:
269306
self.cmd.take_action(parsed_args)
270307
self.fail('CommandError should be raised.')
271308
except exceptions.CommandError as e:
272309
self.assertEqual('1 of 2 floating_ips failed to delete.', str(e))
273310

274-
self.network.find_ip.assert_any_call(
275-
self.floating_ips[0].id, ignore_missing=False)
276-
self.network.find_ip.assert_any_call(
277-
'unexist_floating_ip', ignore_missing=False)
311+
find_floating_ip_mock.assert_any_call(
312+
mock.ANY,
313+
[],
314+
self.floating_ips[0].id,
315+
ignore_missing=False,
316+
)
317+
find_floating_ip_mock.assert_any_call(
318+
mock.ANY,
319+
[],
320+
'unexist_floating_ip',
321+
ignore_missing=False,
322+
)
278323
self.network.delete_ip.assert_called_once_with(
279324
self.floating_ips[0]
280325
)
@@ -534,7 +579,12 @@ def setUp(self):
534579
# Get the command object to test
535580
self.cmd = floating_ip.ShowFloatingIP(self.app, self.namespace)
536581

537-
def test_floating_ip_show(self):
582+
@mock.patch(
583+
"openstackclient.tests.unit.network.v2.test_floating_ip." +
584+
"floating_ip._find_floating_ip"
585+
)
586+
def test_floating_ip_show(self, find_floating_ip_mock):
587+
find_floating_ip_mock.return_value = (self.floating_ip, [])
538588
arglist = [
539589
self.floating_ip.id,
540590
]
@@ -545,9 +595,11 @@ def test_floating_ip_show(self):
545595

546596
columns, data = self.cmd.take_action(parsed_args)
547597

548-
self.network.find_ip.assert_called_once_with(
598+
find_floating_ip_mock.assert_called_once_with(
599+
mock.ANY,
600+
[],
549601
self.floating_ip.id,
550-
ignore_missing=False
602+
ignore_missing=False,
551603
)
552604
self.assertEqual(self.columns, columns)
553605
self.assertEqual(self.data, data)

‎releasenotes/notes/bug-1656402-88b12760fb2d4ef9.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
Fix ``floating ip delete`` and ``floating ip show`` to accept IP addresses
5+
in addition to IDs to select floating IPs to delete or show.
6+
[Bug `1656402 <https://bugs.launchpad.net/bugs/1656402>`_

0 commit comments

Comments
 (0)