From 9209d0bf896c8b7c717e072317794f0b141306fc Mon Sep 17 00:00:00 2001 From: Zhao Chao Date: Tue, 6 Mar 2018 18:08:59 +0800 Subject: [PATCH] Use neutronclient for floatingip operations As add_floating_ip and remove_floating_ip were removed from novaclient, migrating to neutronclient for floatingip operations. This is the first patch for fixing unittests. As integration tests didn't fail, floatingip operations are not covered there, so it may be necessary to add new scenario tests for this in following patches. Change-Id: Ic41f5898db77a09158e0f8bfa4196bdb5e40b49a Signed-off-by: Zhao Chao --- trove/instance/models.py | 9 +++ trove/taskmanager/models.py | 36 ++++++++---- .../unittests/taskmanager/test_models.py | 56 +++++++++++++------ 3 files changed, 73 insertions(+), 28 deletions(-) diff --git a/trove/instance/models.py b/trove/instance/models.py index dbc9487d29..ebe8a41135 100644 --- a/trove/instance/models.py +++ b/trove/instance/models.py @@ -36,6 +36,7 @@ from trove.common.notification import StartNotification from trove.common.remote import create_cinder_client from trove.common.remote import create_dns_client from trove.common.remote import create_guest_client +from trove.common.remote import create_neutron_client from trove.common.remote import create_nova_client from trove.common import server_group as srv_grp from trove.common import template @@ -616,6 +617,7 @@ class BaseInstance(SimpleInstance): self._guest = None self._nova_client = None self._volume_client = None + self._neutron_client = None self._server_group = None self._server_group_loaded = False @@ -709,6 +711,13 @@ class BaseInstance(SimpleInstance): self.context, region_name=self.db_info.region_id) return self._volume_client + @property + def neutron_client(self): + if not self._neutron_client: + self._neutron_client = create_neutron_client( + self.context, region_name=self.db_info.region_id) + return self._neutron_client + def reset_task_status(self): LOG.info("Resetting task status to NONE on instance %s.", self.id) diff --git a/trove/taskmanager/models.py b/trove/taskmanager/models.py index 683ea50ead..b5a3c8b88e 100755 --- a/trove/taskmanager/models.py +++ b/trove/taskmanager/models.py @@ -21,6 +21,7 @@ from eventlet import greenthread from eventlet.timeout import Timeout from novaclient import exceptions as nova_exceptions from oslo_log import log as logging +from oslo_utils import netutils from swiftclient.client import ClientException from trove.backup import models as bkup_models @@ -1170,31 +1171,46 @@ class BuiltInstanceTasks(BuiltInstance, NotifyMixin, ConfigurationMixin): def _get_floating_ips(self): """Returns floating ips as a dict indexed by the ip.""" floating_ips = {} - neutron_client = remote.create_neutron_client(self.context) - network_floating_ips = neutron_client.list_floatingips() + network_floating_ips = self.neutron_client.list_floatingips() for ip in network_floating_ips.get('floatingips'): - floating_ips.update({ip.get('floating_ip_address'): ip}) + floating_ips.update( + {ip.get('floating_ip_address'): ip.get('id')}) LOG.debug("In _get_floating_ips(), returning %s", floating_ips) return floating_ips def detach_public_ips(self): LOG.debug("Begin detach_public_ips for instance %s", self.id) removed_ips = [] - server_id = self.db_info.compute_instance_id - nova_instance = self.nova_client.servers.get(server_id) floating_ips = self._get_floating_ips() for ip in self.get_visible_ip_addresses(): if ip in floating_ips: - nova_instance.remove_floating_ip(ip) - removed_ips.append(ip) + fip_id = floating_ips[ip] + self.neutron_client.update_floatingip( + fip_id, {'floatingip': {'port_id': None}}) + removed_ips.append(fip_id) return removed_ips def attach_public_ips(self, ips): LOG.debug("Begin attach_public_ips for instance %s", self.id) server_id = self.db_info.compute_instance_id - nova_instance = self.nova_client.servers.get(server_id) - for ip in ips: - nova_instance.add_floating_ip(ip) + + # NOTE(zhaochao): in Nova's addFloatingIp, the new floating ip will + # always be associated with the first IPv4 fixed address of the Nova + # instance, we're doing the same thing here, after add_floating_ip is + # removed from novaclient. + server_ports = (self.neutron_client.list_ports(device_id=server_id) + .get('ports')) + fixed_address, port_id = next( + (fixed_ip['ip_address'], port['id']) + for port in server_ports + for fixed_ip in port.get('fixed_ips') + if netutils.is_valid_ipv4(fixed_ip['ip_address'])) + + for fip_id in ips: + self.neutron_client.update_floatingip( + fip_id, {'floatingip': { + 'port_id': port_id, + 'fixed_ip_address': fixed_address}}) def enable_as_master(self): LOG.debug("Calling enable_as_master on %s", self.id) diff --git a/trove/tests/unittests/taskmanager/test_models.py b/trove/tests/unittests/taskmanager/test_models.py index 837620275d..b0d136a9a0 100644 --- a/trove/tests/unittests/taskmanager/test_models.py +++ b/trove/tests/unittests/taskmanager/test_models.py @@ -19,6 +19,7 @@ from cinderclient import exceptions as cinder_exceptions import cinderclient.v2.client as cinderclient from cinderclient.v2 import volumes as cinderclient_volumes from mock import Mock, MagicMock, patch, PropertyMock, call +import neutronclient.v2_0.client as neutronclient from novaclient import exceptions as nova_exceptions import novaclient.v2.flavors import novaclient.v2.servers @@ -59,11 +60,6 @@ INST_ID = 'dbinst-id-1' VOLUME_ID = 'volume-id-1' -class _fake_neutron_client(object): - def list_floatingips(self): - return {'floatingips': [{'floating_ip_address': '192.168.10.1'}]} - - class FakeOptGroup(object): def __init__(self, tcp_ports=['3306', '3301-3307'], udp_ports=[], icmp=False): @@ -693,6 +689,19 @@ class BuiltInstanceTasksTest(trove_testtools.TestCase): stub_volume_mgr.get = MagicMock(return_value=stub_new_volume) stub_volume_mgr.attach = MagicMock(return_value=None) + def _stub_neutron_client(self): + stub_neutron_client = self.instance_task._neutron_client = MagicMock( + spec=neutronclient.Client) + stub_neutron_client.list_floatingips = MagicMock( + return_value={'floatingips': [{ + 'floating_ip_address': '192.168.10.1', + 'id': 'fake-floatingip-id'}]}) + stub_neutron_client.list_ports = MagicMock( + return_value={'ports': [{ + 'fixed_ips': [{'ip_address': '10.0.0.1'}, + {'ip_address': 'fd4a:7bef:d1ed:1::1'}], + 'id': 'fake-port-id'}]}) + def setUp(self): super(BuiltInstanceTasksTest, self).setUp() self.new_flavor = {'id': 8, 'ram': 768, 'name': 'bigger_flavor'} @@ -802,6 +811,10 @@ class BuiltInstanceTasksTest(trove_testtools.TestCase): if 'volume' in self._testMethodName: self._stub_volume_client() + if ('floating_ips' in self._testMethodName or + 'public_ips' in self._testMethodName): + self._stub_neutron_client() + def tearDown(self): super(BuiltInstanceTasksTest, self).tearDown() @@ -932,25 +945,32 @@ class BuiltInstanceTasksTest(trove_testtools.TestCase): Mock()) def test_get_floating_ips(self): - with patch.object(remote, 'create_neutron_client', - return_value=_fake_neutron_client()): - floating_ips = self.instance_task._get_floating_ips() - self.assertEqual('192.168.10.1', - floating_ips['192.168.10.1'].get( - 'floating_ip_address')) + floating_ips = self.instance_task._get_floating_ips() + self.assertEqual({'192.168.10.1': 'fake-floatingip-id'}, + floating_ips) @patch.object(BaseInstance, 'get_visible_ip_addresses', return_value=['192.168.10.1']) def test_detach_public_ips(self, mock_address): - with patch.object(remote, 'create_neutron_client', - return_value=_fake_neutron_client()): - removed_ips = self.instance_task.detach_public_ips() - self.assertEqual(['192.168.10.1'], removed_ips) + removed_ips = self.instance_task.detach_public_ips() + self.assertEqual(['fake-floatingip-id'], removed_ips) + mock_update_floatingip = (self.instance_task.neutron_client + .update_floatingip) + mock_update_floatingip.assert_called_once_with( + removed_ips[0], {'floatingip': {'port_id': None}}) def test_attach_public_ips(self): - self.instance_task.attach_public_ips(['192.168.10.1']) - self.stub_verifying_server.add_floating_ip.assert_called_with( - '192.168.10.1') + self.instance_task.attach_public_ips(['fake-floatingip-id']) + mock_list_ports = (self.instance_task.neutron_client + .list_ports) + mock_list_ports.assert_called_once_with(device_id='computeinst-id-1') + + mock_update_floatingip = (self.instance_task.neutron_client + .update_floatingip) + mock_update_floatingip.assert_called_once_with( + 'fake-floatingip-id', + {'floatingip': {'port_id': 'fake-port-id', + 'fixed_ip_address': '10.0.0.1'}}) @patch.object(BaseInstance, 'update_db') def test_enable_as_master(self, mock_update_db):