When SG delete fails on vip deallocate, try harder

Go out and find the ports that are using the SG and remove them.

Change-Id: I2f97f969ea4ff699bef6a7fcd86d5aac6ad3b423
This commit is contained in:
Adam Harwell 2018-03-02 15:34:35 +00:00
parent f33c461cb9
commit 254d4d4d86
5 changed files with 93 additions and 13 deletions

View File

@ -243,43 +243,58 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver):
LOG.exception(message)
raise base.DeallocateVIPException(message)
@staticmethod
def _filter_amphora(amp):
return amp.status == constants.AMPHORA_ALLOCATED
def _delete_security_group(self, vip, port):
if self.sec_grp_enabled:
sec_grp = self._get_lb_security_group(vip.load_balancer.id)
if sec_grp:
sec_grp = sec_grp.get('id')
sec_grp_id = sec_grp.get('id')
LOG.info(
"Removing security group %(sg)s from port %(port)s",
{'sg': sec_grp, 'port': vip.port_id})
{'sg': sec_grp_id, 'port': vip.port_id})
raw_port = None
try:
if port:
raw_port = self.neutron_client.show_port(port.id)
except Exception:
LOG.warning('Unable to get port information for port '
' %s. Continuing to delete the security '
'%s. Continuing to delete the security '
'group.', port.id)
if raw_port:
sec_grps = raw_port.get(
'port', {}).get('security_groups', [])
if sec_grp in sec_grps:
sec_grps.remove(sec_grp)
if sec_grp_id in sec_grps:
sec_grps.remove(sec_grp_id)
port_update = {'port': {'security_groups': sec_grps}}
self.neutron_client.update_port(port.id, port_update)
self._delete_vip_security_group(sec_grp)
try:
self._delete_vip_security_group(sec_grp_id)
except base.DeallocateVIPException:
# Try to delete any leftover ports on this security group.
# Because this security group is created and managed by us,
# it *should* only return ports that we own / can delete.
LOG.warning('Failed to delete security group on first '
'pass: %s', sec_grp_id)
extra_ports = self._get_ports_by_security_group(sec_grp_id)
for port in extra_ports:
port_id = port.get('id')
try:
LOG.warning('Deleting extra port %s on security '
'group %s...', port_id, sec_grp_id)
self.neutron_client.delete_port(port_id)
except Exception:
LOG.warning('Failed to delete extra port %s on '
'security group %s.',
port_id, sec_grp_id)
# Now try it again
self._delete_vip_security_group(sec_grp_id)
def deallocate_vip(self, vip):
"""Delete the vrrp_port (instance port) in case nova didn't
This can happen if a failover has occurred.
"""
for amphora in six.moves.filter(self._filter_amphora,
vip.load_balancer.amphorae):
for amphora in vip.load_balancer.amphorae:
try:
self.neutron_client.delete_port(amphora.vrrp_port_id)
except (neutron_client_exceptions.NotFound,

View File

@ -46,6 +46,8 @@ class BaseNeutronDriver(base.AbstractNetworkDriver):
self.sec_grp_enabled = self._check_extension_enabled(SEC_GRP_EXT_ALIAS)
self.dns_integration_enabled = self._check_extension_enabled(
DNS_INT_EXT_ALIAS)
self.project_id = self.neutron_client.get_auth_info().get(
'auth_tenant_id')
def _check_extension_enabled(self, extension_alias):
if extension_alias in self._check_extension_cache:
@ -115,6 +117,14 @@ class BaseNeutronDriver(base.AbstractNetworkDriver):
except Exception as e:
raise base.NetworkException(str(e))
def _get_ports_by_security_group(self, sec_grp_id):
all_ports = self.neutron_client.list_ports(project=self.project_id)
filtered_ports = []
for port in all_ports.get('ports', []):
if sec_grp_id in port.get('security_groups', []):
filtered_ports.append(port)
return filtered_ports
def _create_security_group(self, name):
new_sec_grp = {'security_group': {'name': name}}
sec_grp = self.neutron_client.create_security_group(new_sec_grp)

View File

@ -81,7 +81,8 @@ MOCK_NEUTRON_PORT = {'port': {'network_id': MOCK_NETWORK_ID,
'status': MOCK_STATUS,
'mac_address': MOCK_MAC_ADDR,
'fixed_ips': [{'ip_address': MOCK_IP_ADDRESS,
'subnet_id': MOCK_SUBNET_ID}]}}
'subnet_id': MOCK_SUBNET_ID}],
'security_groups': [MOCK_SECURITY_GROUP_ID]}}
MOCK_NEUTRON_QOS_POLICY_ID = 'mock-qos-id'
MOCK_QOS_POLICY_ID1 = 'qos1-id'
MOCK_QOS_POLICY_ID2 = 'qos2-id'

View File

@ -201,6 +201,49 @@ class TestAllowedAddressPairsDriver(base.TestCase):
self.assertRaises(network_base.DeallocateVIPException,
self.driver.deallocate_vip, vip)
def test_deallocate_vip_when_secgrp_has_allocated_ports(self):
max_retries = 1
conf = oslo_fixture.Config(cfg.CONF)
conf.config(group="networking", max_retries=max_retries)
lb = dmh.generate_load_balancer_tree()
lb.vip.load_balancer = lb
vip = lb.vip
show_port = self.driver.neutron_client.show_port
show_port.return_value = {'port': {
'device_owner': allowed_address_pairs.OCTAVIA_OWNER}}
delete_port = self.driver.neutron_client.delete_port
list_ports = self.driver.neutron_client.list_ports
list_security_groups = self.driver.neutron_client.list_security_groups
delete_sec_grp = self.driver.neutron_client.delete_security_group
security_groups = {
'security_groups': [
{'id': t_constants.MOCK_SECURITY_GROUP_ID}
]
}
list_security_groups.return_value = security_groups
delete_grp_results = [
network_base.DeallocateVIPException
for _ in range(max_retries + 1)] # Total tries = max_retries + 1
delete_grp_results.append(None)
delete_sec_grp.side_effect = delete_grp_results
list_ports.side_effect = [{
"ports": [t_constants.MOCK_NEUTRON_PORT['port'],
t_constants.MOCK_NEUTRON_PORT2['port']]}]
self.driver.deallocate_vip(vip)
# First we expect the amp's ports to be deleted
dp_calls = [mock.call(amp.vrrp_port_id) for amp in lb.amphorae]
# Then after the SG delete fails, extra hanging-on ports are removed
dp_calls.append(mock.call(t_constants.MOCK_PORT_ID))
# Lastly we remove the vip port
dp_calls.append(mock.call(vip.port_id))
self.assertEqual(len(dp_calls), delete_port.call_count)
delete_port.assert_has_calls(dp_calls)
dsg_calls = [mock.call(t_constants.MOCK_SECURITY_GROUP_ID)
for _ in range(max_retries + 2)] # Max fail + one success
self.assertEqual(len(dsg_calls), delete_sec_grp.call_count)
delete_sec_grp.assert_has_calls(dsg_calls)
def test_deallocate_vip_when_port_not_found(self):
lb = dmh.generate_load_balancer_tree()
vip = data_models.Vip(port_id='1')

View File

@ -106,6 +106,17 @@ class TestBaseNeutronNetworkDriver(base.TestCase):
self.driver._add_security_group_to_port,
t_constants.MOCK_SECURITY_GROUP_ID, t_constants.MOCK_PORT_ID)
def test__get_ports_by_security_group(self):
self.driver.neutron_client.list_ports.return_value = {
"ports": [
t_constants.MOCK_NEUTRON_PORT['port'],
t_constants.MOCK_NEUTRON_PORT2['port']]
}
ports = self.driver._get_ports_by_security_group(
t_constants.MOCK_SECURITY_GROUP_ID)
self.assertEqual(1, len(ports))
self.assertIn(t_constants.MOCK_NEUTRON_PORT['port'], ports)
def test__create_security_group(self):
sg_return = self.driver._create_security_group(
t_constants.MOCK_SECURITY_GROUP_NAME)