openstack/python-openstackclient
Code Issues Proposed changes

Handle multiple ports in AddFloatingIP

Browse Source 
AddFloatingIP refers to an old nova proxy API to neutron that was
deprecated in nova. The neutron API for floating IP associate requires
a port to be specified. Currently, the code is selecting the first port
if the server has multiple ports. But, an attempt to associate the
first port with a floating IP can fail if the first port is not on a
network that is attached to an external gateway.

In order to make the command work better for users who have a server
with multiple ports, we can:

  1. Select the port corresponding to the fixed_ip_address, if one was
     specified

  2. Try to associate the floating IP with each port until one of the
     attempts succeeds, else re-raise the last exception.
     (404 ExternalGatewayForFloatingIPNotFound from neutron)

This also fixes incorrect FakeFloatingIP attributes that were being set
in the TestServerAddFloatingIPNetwork unit tests, which were causing
the tests to use None as parsed args for ip-address and
--fixed-ip-address and thus bypassing code in the
'if parsed_args.fixed_ip_address:' block.

Task: 27800
Story: 2004263

Change-Id: I11fbcebf6b00f12a030b000c84dcf1d6b5e86250
This commit is contained in:
melanie witt 2018-11-02 22:27:55 +00:00
parent 1894a3cb17
commit 013c9a4f3a
3 changed files with 164 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -23,6 +23,7 @@ import os
from novaclient import api_versions
from novaclient.v2 import servers
from openstack import exceptions as sdk_exceptions
from osc_lib.cli import parseractions
from osc_lib.command import command
from osc_lib import exceptions
@ -251,13 +252,16 @@ class AddFloatingIP(network_common.NetworkAndComputeCommand):
parser.add_argument(
"ip_address",
metavar="<ip-address>",
help=_("Floating IP address to assign to server (IP only)"),
help=_("Floating IP address to assign to the first available "
"server port (IP only)"),
)
parser.add_argument(
"--fixed-ip-address",
metavar="<ip-address>",
help=_(
"Fixed IP address to associate with this floating IP address"
"Fixed IP address to associate with this floating IP address. "
"The first server port containing the fixed IP address will "
"be used"
),
)
return parser
@ -274,12 +278,45 @@ class AddFloatingIP(network_common.NetworkAndComputeCommand):
compute_client.servers,
parsed_args.server,
)
port = list(client.ports(device_id=server.id))[0]
attrs['port_id'] = port.id
ports = list(client.ports(device_id=server.id))
# If the fixed IP address was specified, we need to find the
# corresponding port.
if parsed_args.fixed_ip_address:
attrs['fixed_ip_address'] = parsed_args.fixed_ip_address
client.update_ip(obj, **attrs)
fip_address = parsed_args.fixed_ip_address
attrs['fixed_ip_address'] = fip_address
for port in ports:
for ip in port.fixed_ips:
if ip['ip_address'] == fip_address:
attrs['port_id'] = port.id
break
else:
continue
break
if 'port_id' not in attrs:
msg = _('No port found for fixed IP address %s')
raise exceptions.CommandError(msg % fip_address)
client.update_ip(obj, **attrs)
else:
# It's possible that one or more ports are not connected to a
# router and thus could fail association with a floating IP.
# Try each port until one succeeds. If none succeed, re-raise the
# last exception.
error = None
for port in ports:
attrs['port_id'] = port.id
try:
client.update_ip(obj, **attrs)
except sdk_exceptions.NotFoundException as exp:
# 404 ExternalGatewayForFloatingIPNotFound from neutron
LOG.info('Skipped port %s because it is not attached to '
'an external gateway', port.id)
error = exp
continue
else:
error = None
break
if error:
raise error
def take_action_compute(self, client, parsed_args):
client.api.floating_ip_add(

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

@ -19,6 +19,7 @@ import getpass
import mock
from mock import call
from novaclient import api_versions
from openstack import exceptions as sdk_exceptions
from osc_lib import exceptions
from osc_lib import utils as common_utils
from oslo_utils import timeutils
@ -222,11 +223,11 @@ class TestServerAddFloatingIPNetwork(
self.network.ports = mock.Mock(return_value=[_port])
arglist = [
_server.id,
_floating_ip['ip'],
_floating_ip['floating_ip_address'],
]
verifylist = [
('server', _server.id),
('ip_address', _floating_ip['ip']),
('ip_address', _floating_ip['floating_ip_address']),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -237,7 +238,7 @@ class TestServerAddFloatingIPNetwork(
}
self.network.find_ip.assert_called_once_with(
_floating_ip['ip'],
_floating_ip['floating_ip_address'],
ignore_missing=False,
)
self.network.ports.assert_called_once_with(
@ -248,6 +249,64 @@ class TestServerAddFloatingIPNetwork(
**attrs
)
def test_server_add_floating_ip_default_no_external_gateway(self,
success=False):
_server = compute_fakes.FakeServer.create_one_server()
self.servers_mock.get.return_value = _server
_port = network_fakes.FakePort.create_one_port()
_floating_ip = network_fakes.FakeFloatingIP.create_one_floating_ip()
self.network.find_ip = mock.Mock(return_value=_floating_ip)
return_value = [_port]
# In the success case, we'll have two ports, where the first port is
# not attached to an external gateway but the second port is.
if success:
return_value.append(_port)
self.network.ports = mock.Mock(return_value=return_value)
side_effect = [sdk_exceptions.NotFoundException()]
if success:
side_effect.append(None)
self.network.update_ip = mock.Mock(side_effect=side_effect)
arglist = [
_server.id,
_floating_ip['floating_ip_address'],
]
verifylist = [
('server', _server.id),
('ip_address', _floating_ip['floating_ip_address']),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
if success:
self.cmd.take_action(parsed_args)
else:
self.assertRaises(sdk_exceptions.NotFoundException,
self.cmd.take_action, parsed_args)
attrs = {
'port_id': _port.id,
}
self.network.find_ip.assert_called_once_with(
_floating_ip['floating_ip_address'],
ignore_missing=False,
)
self.network.ports.assert_called_once_with(
device_id=_server.id,
)
if success:
self.assertEqual(2, self.network.update_ip.call_count)
calls = [mock.call(_floating_ip, **attrs)] * 2
self.network.update_ip.assert_has_calls(calls)
else:
self.network.update_ip.assert_called_once_with(
_floating_ip,
**attrs
)
def test_server_add_floating_ip_default_one_external_gateway(self):
self.test_server_add_floating_ip_default_no_external_gateway(
success=True)
def test_server_add_floating_ip_fixed(self):
_server = compute_fakes.FakeServer.create_one_server()
self.servers_mock.get.return_value = _server
@ -255,26 +314,31 @@ class TestServerAddFloatingIPNetwork(
_floating_ip = network_fakes.FakeFloatingIP.create_one_floating_ip()
self.network.find_ip = mock.Mock(return_value=_floating_ip)
self.network.ports = mock.Mock(return_value=[_port])
# The user has specified a fixed ip that matches one of the ports
# already attached to the instance.
arglist = [
'--fixed-ip-address', _floating_ip['fixed_ip'],
'--fixed-ip-address', _port.fixed_ips[0]['ip_address'],
_server.id,
_floating_ip['ip'],
_floating_ip['floating_ip_address'],
]
verifylist = [
('fixed_ip_address', _floating_ip['fixed_ip']),
('fixed_ip_address', _port.fixed_ips[0]['ip_address']),
('server', _server.id),
('ip_address', _floating_ip['ip']),
('ip_address', _floating_ip['floating_ip_address']),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.cmd.take_action(parsed_args)
# We expect the update_ip call to specify a new fixed_ip_address which
# will overwrite the floating ip's existing fixed_ip_address.
attrs = {
'port_id': _port.id,
'fixed_ip_address': _port.fixed_ips[0]['ip_address'],
}
self.network.find_ip.assert_called_once_with(
_floating_ip['ip'],
_floating_ip['floating_ip_address'],
ignore_missing=False,
)
self.network.ports.assert_called_once_with(
@ -285,6 +349,40 @@ class TestServerAddFloatingIPNetwork(
**attrs
)
def test_server_add_floating_ip_fixed_no_port_found(self):
_server = compute_fakes.FakeServer.create_one_server()
self.servers_mock.get.return_value = _server
_port = network_fakes.FakePort.create_one_port()
_floating_ip = network_fakes.FakeFloatingIP.create_one_floating_ip()
self.network.find_ip = mock.Mock(return_value=_floating_ip)
self.network.ports = mock.Mock(return_value=[_port])
# The user has specified a fixed ip that does not match any of the
# ports already attached to the instance.
nonexistent_ip = '10.0.0.9'
arglist = [
'--fixed-ip-address', nonexistent_ip,
_server.id,
_floating_ip['floating_ip_address'],
]
verifylist = [
('fixed_ip_address', nonexistent_ip),
('server', _server.id),
('ip_address', _floating_ip['floating_ip_address']),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(exceptions.CommandError, self.cmd.take_action,
parsed_args)
self.network.find_ip.assert_called_once_with(
_floating_ip['floating_ip_address'],
ignore_missing=False,
)
self.network.ports.assert_called_once_with(
device_id=_server.id,
)
self.network.update_ip.assert_not_called()
class TestServerAddPort(TestServer):

14
releasenotes/notes/floating-ip-multi-port-9779e88f590cae23.yaml Normal file
View File

@ -0,0 +1,14 @@
---
fixes:
- |
The ``openstack server add floating ip`` command has been fixed to handle
servers with multiple ports attached. Previously, the command was using
the first port in the port list when attempting to associate the floating
ip. This could fail if the server had multiple ports and the first port
in the list was not attached to an external gateway. Another way it could
fail is if the ``--fixed-ip-address`` option was passed and the first port
did not have the specified fixed IP address attached to it.
Now, the ``openstack server add floating ip`` command will find the port
attached to the specified ``--fixed-ip-address``, if provided, else it will
try multiple ports until one is found attached to an external gateway. If
a suitable port is not found in the port list, an error will be returned.