Fix octavia lb deleting_race with cascading
Octavia can cascade resource deletion by passing &cascade=True as a parameter to the HTTP request. This patch enables us to do that, fixing the children deletion race and greatly simplifying the callstack of service deletion Closes-Bug: #1767074 Change-Id: I97303b54b900bc40f791309bd2589e617b61177a Signed-off-by: Antoni Segura Puimedon <antonisp@celebdor.com>
This commit is contained in:
parent
15172507e4
commit
55973f2505
|
@ -52,6 +52,7 @@ def setup_loadbalancer_client():
|
||||||
if any(ext['alias'] == 'lbaasv2' for
|
if any(ext['alias'] == 'lbaasv2' for
|
||||||
ext in neutron_client.list_extensions()['extensions']):
|
ext in neutron_client.list_extensions()['extensions']):
|
||||||
_clients[_LB_CLIENT] = neutron_client
|
_clients[_LB_CLIENT] = neutron_client
|
||||||
|
neutron_client.cascading_capable = False
|
||||||
else:
|
else:
|
||||||
# Since Octavia is lbaasv2 API compatible (A superset of it) we'll just
|
# Since Octavia is lbaasv2 API compatible (A superset of it) we'll just
|
||||||
# wire an extra neutron client instance to point to it
|
# wire an extra neutron client instance to point to it
|
||||||
|
@ -63,6 +64,7 @@ def setup_loadbalancer_client():
|
||||||
service_type='load-balancer')
|
service_type='load-balancer')
|
||||||
lbaas_client.httpclient = octo_httpclient
|
lbaas_client.httpclient = octo_httpclient
|
||||||
_clients[_LB_CLIENT] = lbaas_client
|
_clients[_LB_CLIENT] = lbaas_client
|
||||||
|
lbaas_client.cascading_capable = True
|
||||||
|
|
||||||
|
|
||||||
def setup_kubernetes_client():
|
def setup_kubernetes_client():
|
||||||
|
|
|
@ -40,6 +40,11 @@ _SUPPORTED_LISTENER_PROT = ('HTTP', 'HTTPS', 'TCP')
|
||||||
class LBaaSv2Driver(base.LBaaSDriver):
|
class LBaaSv2Driver(base.LBaaSDriver):
|
||||||
"""LBaaSv2Driver implements LBaaSDriver for Neutron LBaaSv2 API."""
|
"""LBaaSv2Driver implements LBaaSDriver for Neutron LBaaSv2 API."""
|
||||||
|
|
||||||
|
@property
|
||||||
|
def cascading_capable(self):
|
||||||
|
lbaas = clients.get_loadbalancer_client()
|
||||||
|
return lbaas.cascading_capable
|
||||||
|
|
||||||
def ensure_loadbalancer(self, endpoints, project_id, subnet_id, ip,
|
def ensure_loadbalancer(self, endpoints, project_id, subnet_id, ip,
|
||||||
security_groups_ids, service_type):
|
security_groups_ids, service_type):
|
||||||
name = "%(namespace)s/%(name)s" % endpoints['metadata']
|
name = "%(namespace)s/%(name)s" % endpoints['metadata']
|
||||||
|
@ -67,6 +72,15 @@ class LBaaSv2Driver(base.LBaaSDriver):
|
||||||
def release_loadbalancer(self, endpoints, loadbalancer):
|
def release_loadbalancer(self, endpoints, loadbalancer):
|
||||||
neutron = clients.get_neutron_client()
|
neutron = clients.get_neutron_client()
|
||||||
lbaas = clients.get_loadbalancer_client()
|
lbaas = clients.get_loadbalancer_client()
|
||||||
|
if lbaas.cascading_capable:
|
||||||
|
self._release(
|
||||||
|
loadbalancer,
|
||||||
|
loadbalancer,
|
||||||
|
lbaas.delete,
|
||||||
|
lbaas.lbaas_loadbalancer_path % loadbalancer.id,
|
||||||
|
params={'cascade': True})
|
||||||
|
|
||||||
|
else:
|
||||||
self._release(loadbalancer, loadbalancer,
|
self._release(loadbalancer, loadbalancer,
|
||||||
lbaas.delete_loadbalancer, loadbalancer.id)
|
lbaas.delete_loadbalancer, loadbalancer.id)
|
||||||
|
|
||||||
|
|
|
@ -271,6 +271,14 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
|
||||||
if not lbaas_state:
|
if not lbaas_state:
|
||||||
return
|
return
|
||||||
# NOTE(ivc): deleting pool deletes its members
|
# NOTE(ivc): deleting pool deletes its members
|
||||||
|
if self._drv_lbaas.cascading_capable:
|
||||||
|
self._drv_lbaas.release_loadbalancer(
|
||||||
|
endpoints=endpoints,
|
||||||
|
loadbalancer=lbaas_state.loadbalancer)
|
||||||
|
if lbaas_state.service_pub_ip_info:
|
||||||
|
self._drv_service_pub_ip.release_pub_ip(
|
||||||
|
lbaas_state.service_pub_ip_info)
|
||||||
|
else:
|
||||||
lbaas_state.members = []
|
lbaas_state.members = []
|
||||||
self._sync_lbaas_members(endpoints, lbaas_state,
|
self._sync_lbaas_members(endpoints, lbaas_state,
|
||||||
obj_lbaas.LBaaSServiceSpec())
|
obj_lbaas.LBaaSServiceSpec())
|
||||||
|
|
|
@ -75,6 +75,7 @@ class TestLBaaSv2Driver(test_base.TestCase):
|
||||||
def test_release_loadbalancer(self):
|
def test_release_loadbalancer(self):
|
||||||
self.useFixture(k_fix.MockNeutronClient()).client
|
self.useFixture(k_fix.MockNeutronClient()).client
|
||||||
lbaas = self.useFixture(k_fix.MockLBaaSClient()).client
|
lbaas = self.useFixture(k_fix.MockLBaaSClient()).client
|
||||||
|
lbaas.cascading_capable = False
|
||||||
cls = d_lbaasv2.LBaaSv2Driver
|
cls = d_lbaasv2.LBaaSv2Driver
|
||||||
m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver)
|
m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver)
|
||||||
endpoints = mock.sentinel.endpoints
|
endpoints = mock.sentinel.endpoints
|
||||||
|
@ -86,6 +87,23 @@ class TestLBaaSv2Driver(test_base.TestCase):
|
||||||
lbaas.delete_loadbalancer,
|
lbaas.delete_loadbalancer,
|
||||||
loadbalancer.id)
|
loadbalancer.id)
|
||||||
|
|
||||||
|
def test_cascade_release_loadbalancer(self):
|
||||||
|
self.useFixture(k_fix.MockNeutronClient()).client
|
||||||
|
lbaas = self.useFixture(k_fix.MockLBaaSClient()).client
|
||||||
|
lbaas.cascading_capable = True
|
||||||
|
lbaas.lbaas_loadbalancer_path = "boo %s"
|
||||||
|
cls = d_lbaasv2.LBaaSv2Driver
|
||||||
|
m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver)
|
||||||
|
endpoints = mock.sentinel.endpoints
|
||||||
|
loadbalancer = mock.Mock()
|
||||||
|
|
||||||
|
cls.release_loadbalancer(m_driver, endpoints, loadbalancer)
|
||||||
|
|
||||||
|
m_driver._release.assert_called_once_with(
|
||||||
|
loadbalancer, loadbalancer, lbaas.delete,
|
||||||
|
lbaas.lbaas_loadbalancer_path % loadbalancer.id,
|
||||||
|
params={'cascade': True})
|
||||||
|
|
||||||
def test_ensure_listener(self):
|
def test_ensure_listener(self):
|
||||||
cls = d_lbaasv2.LBaaSv2Driver
|
cls = d_lbaasv2.LBaaSv2Driver
|
||||||
m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver)
|
m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver)
|
||||||
|
|
|
@ -516,6 +516,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
|
||||||
|
|
||||||
m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
|
m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
|
||||||
m_handler._get_lbaas_state.return_value = lbaas_state
|
m_handler._get_lbaas_state.return_value = lbaas_state
|
||||||
|
m_handler._drv_lbaas = mock.Mock()
|
||||||
|
m_handler._drv_lbaas.cascading_capable = False
|
||||||
|
|
||||||
h_lbaas.LoadBalancerHandler.on_deleted(m_handler, endpoints)
|
h_lbaas.LoadBalancerHandler.on_deleted(m_handler, endpoints)
|
||||||
|
|
||||||
|
@ -523,6 +525,29 @@ class TestLoadBalancerHandler(test_base.TestCase):
|
||||||
m_handler._sync_lbaas_members.assert_called_once_with(
|
m_handler._sync_lbaas_members.assert_called_once_with(
|
||||||
endpoints, lbaas_state, empty_spec)
|
endpoints, lbaas_state, empty_spec)
|
||||||
|
|
||||||
|
@mock.patch('kuryr_kubernetes.objects.lbaas'
|
||||||
|
'.LBaaSServiceSpec')
|
||||||
|
def test_on_cascade_deleted_lb_service(self, m_svc_spec_ctor):
|
||||||
|
endpoints = mock.sentinel.endpoints
|
||||||
|
empty_spec = mock.sentinel.empty_spec
|
||||||
|
lbaas_state = mock.Mock()
|
||||||
|
lbaas_state.loadbalancer = mock.sentinel.loadbalancer
|
||||||
|
lbaas_state.service_pub_ip_info = mock.sentinel.pub_ip
|
||||||
|
m_svc_spec_ctor.return_value = empty_spec
|
||||||
|
|
||||||
|
m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
|
||||||
|
m_handler._get_lbaas_state.return_value = lbaas_state
|
||||||
|
m_handler._drv_lbaas = mock.Mock()
|
||||||
|
m_handler._drv_service_pub_ip = mock.Mock()
|
||||||
|
m_handler._drv_lbaas.cascading_capable = True
|
||||||
|
|
||||||
|
h_lbaas.LoadBalancerHandler.on_deleted(m_handler, endpoints)
|
||||||
|
|
||||||
|
m_handler._drv_lbaas.release_loadbalancer.assert_called_once_with(
|
||||||
|
endpoints=endpoints, loadbalancer=lbaas_state.loadbalancer)
|
||||||
|
m_handler._drv_service_pub_ip.release_pub_ip.assert_called_once_with(
|
||||||
|
lbaas_state.service_pub_ip_info)
|
||||||
|
|
||||||
def test_should_ignore(self):
|
def test_should_ignore(self):
|
||||||
endpoints = mock.sentinel.endpoints
|
endpoints = mock.sentinel.endpoints
|
||||||
lbaas_spec = mock.sentinel.lbaas_spec
|
lbaas_spec = mock.sentinel.lbaas_spec
|
||||||
|
|
Loading…
Reference in New Issue