From: Oleg Bondarev Date: Tue, 13 Oct 2015 08:55:02 +0000 (+0300) Subject: Always send status update for processed floating ips X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=7592b17b8793c0e296c77bd17929f3db37f1483e;p=openstack-build%2Fneutron-build.git Always send status update for processed floating ips Currently l3 agent skips status update for floating ips in case status didn't change: this might be wrong if status has changed on server side while agent was processing. See bug for details. L3 agent skips floating ip processing in case ip address exists on external device. So we can still skip status update for such floating ips. Closes-Bug: #1505557 Change-Id: I908fe5a0555f68ab85e7d199c36a903b915e103f --- diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index d6e6302e9..30d01122c 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -233,11 +233,10 @@ class RouterInfo(object): LOG.debug('Floating ip %(id)s added, status %(status)s', {'id': fip['id'], 'status': fip_statuses.get(fip['id'])}) - + elif fip_statuses[fip['id']] == fip['status']: # mark the status as not changed. we can't remove it because # that's how the caller determines that it was removed - if fip_statuses[fip['id']] == fip['status']: - fip_statuses[fip['id']] = FLOATINGIP_STATUS_NOCHANGE + fip_statuses[fip['id']] = FLOATINGIP_STATUS_NOCHANGE fips_to_remove = ( ip_cidr for ip_cidr in existing_cidrs - new_cidrs if common_utils.is_cidr_host(ip_cidr)) diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 9bf76b630..6b66df4a0 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -1415,25 +1415,53 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): def test_process_router_floatingip_nochange(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + router = l3_test_common.prepare_router_data(num_internal_ports=1) + fip1 = {'id': _uuid(), 'floating_ip_address': '8.8.8.8', + 'fixed_ip_address': '7.7.7.7', 'status': 'ACTIVE', + 'port_id': router[l3_constants.INTERFACE_KEY][0]['id']} + fip2 = copy.copy(fip1) + fip2.update({'id': _uuid(), 'status': 'DOWN', + 'floating_ip_address': '9.9.9.9'}) + router[l3_constants.FLOATINGIP_KEY] = [fip1, fip2] + + ri = legacy_router.LegacyRouter(router['id'], router, + **self.ri_kwargs) + ri.external_gateway_added = mock.Mock() with mock.patch.object( agent.plugin_rpc, 'update_floatingip_statuses' - ) as mock_update_fip_status: - router = l3_test_common.prepare_router_data(num_internal_ports=1) - fip1 = {'id': _uuid(), 'floating_ip_address': '8.8.8.8', - 'fixed_ip_address': '7.7.7.7', 'status': 'ACTIVE', - 'port_id': router[l3_constants.INTERFACE_KEY][0]['id']} - fip2 = copy.copy(fip1) - fip2.update({'id': _uuid(), 'status': 'DOWN'}) - router[l3_constants.FLOATINGIP_KEY] = [fip1, fip2] - - ri = legacy_router.LegacyRouter(router['id'], router, - **self.ri_kwargs) - ri.external_gateway_added = mock.Mock() + ) as mock_update_fip_status,\ + mock.patch.object(ri, 'get_router_cidrs') as mock_get_cidrs: + mock_get_cidrs.return_value = set( + [fip1['floating_ip_address'] + '/32']) ri.process(agent) - # make sure only the one that went from DOWN->ACTIVE was sent + # make sure only the one that wasn't in existing cidrs was sent mock_update_fip_status.assert_called_once_with( mock.ANY, ri.router_id, {fip2['id']: 'ACTIVE'}) + def test_process_router_floatingip_status_update_if_processed(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + router = l3_test_common.prepare_router_data(num_internal_ports=1) + fip1 = {'id': _uuid(), 'floating_ip_address': '8.8.8.8', + 'fixed_ip_address': '7.7.7.7', 'status': 'ACTIVE', + 'port_id': router[l3_constants.INTERFACE_KEY][0]['id']} + fip2 = copy.copy(fip1) + fip2.update({'id': _uuid(), 'status': 'DOWN', }) + router[l3_constants.FLOATINGIP_KEY] = [fip1, fip2] + + ri = legacy_router.LegacyRouter(router['id'], router, + **self.ri_kwargs) + ri.external_gateway_added = mock.Mock() + with mock.patch.object( + agent.plugin_rpc, 'update_floatingip_statuses' + ) as mock_update_fip_status,\ + mock.patch.object(ri, 'get_router_cidrs') as mock_get_cidrs: + mock_get_cidrs.return_value = set() + ri.process(agent) + # make sure both was sent since not existed in existing cidrs + mock_update_fip_status.assert_called_once_with( + mock.ANY, ri.router_id, {fip1['id']: 'ACTIVE', + fip2['id']: 'ACTIVE'}) + def test_process_router_floatingip_disabled(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) with mock.patch.object( diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py index 66cafa41f..3033e571a 100644 --- a/neutron/tests/unit/agent/l3/test_router_info.py +++ b/neutron/tests/unit/agent/l3/test_router_info.py @@ -258,7 +258,8 @@ class TestFloatingIpWithMockDevice(BasicRouterTestCaseFramework): fip = { 'id': fip_id, 'port_id': _uuid(), 'floating_ip_address': '15.1.2.3', - 'fixed_ip_address': '192.168.0.2' + 'fixed_ip_address': '192.168.0.2', + 'status': l3_constants.FLOATINGIP_STATUS_DOWN } IPDevice.return_value = device = mock.Mock()