openstack/kuryr-kubernetes
Code Issues Proposed changes

Ensure leftover LBaaS are deleted upon Controller start

Browse Source 
When the deletion of a SVC is triggered while the load balancer
is still creating and the controller restarts, the deletion
event will be gone and the lbaas remains. This commit fixes
the issue, by removing the leftover lbaas upon controller restart.

Change-Id: I2d7dd14c3f05b0b1da6db7ac9b58731e34b593e6
This commit is contained in:
Maysa Macedo 2019-11-22 11:53:16 +00:00
parent 9c8cc8b092
commit d2e3aea728
6 changed files with 93 additions and 19 deletions
kuryr_kubernetes
controller
drivers
base.pylbaasv2.py
handlers
lbaas.py
tests/unit/controller
drivers
test_lbaasv2.py
handlers
test_ingress_lbaas.pytest_lbaas.py

5
kuryr_kubernetes/controller/drivers/base.py

@ -725,6 +725,11 @@ class LBaaSDriver(DriverBase):
"""
raise NotImplementedError()
@abc.abstractmethod
def add_tags(self, resource, req):
"""Add tags to a request if the resource supports it"""
raise NotImplementedError()
@six.add_metaclass(abc.ABCMeta)
class VIFPoolDriver(PodVIFDriver):

14
kuryr_kubernetes/controller/drivers/lbaasv2.py

@ -85,7 +85,7 @@ class LBaaSv2Driver(base.LBaaSDriver):
def get_loadbalancer_pool_name(self, loadbalancer, namespace, svc_name):
return "%s/%s/%s" % (loadbalancer.name, namespace, svc_name)
def _add_tags(self, resource, req):
def add_tags(self, resource, req):
if CONF.neutron_defaults.resource_tags:
if self._octavia_tags:
req['tags'] = CONF.neutron_defaults.resource_tags
@ -554,7 +554,7 @@ class LBaaSv2Driver(base.LBaaSDriver):
if loadbalancer.provider is not None:
request['provider'] = loadbalancer.provider
self._add_tags('loadbalancer', request)
self.add_tags('loadbalancer', request)
response = self._post_lb_resource(o_lb.LoadBalancer, request)
@ -597,7 +597,7 @@ class LBaaSv2Driver(base.LBaaSDriver):
'protocol': listener.protocol,
'protocol_port': listener.port,
}
self._add_tags('listener', request)
self.add_tags('listener', request)
response = self._post_lb_resource(o_lis.Listener, request)
listener.id = response.id
return listener
@ -630,7 +630,7 @@ class LBaaSv2Driver(base.LBaaSDriver):
'protocol': pool.protocol,
'lb_algorithm': lb_algorithm,
}
self._add_tags('pool', request)
self.add_tags('pool', request)
response = self._post_lb_resource(o_pool.Pool, request)
pool.id = response.id
return pool
@ -666,7 +666,7 @@ class LBaaSv2Driver(base.LBaaSDriver):
'address': str(member.ip),
'protocol_port': member.port,
}
self._add_tags('member', request)
self.add_tags('member', request)
response = self._post_lb_resource(o_mem.Member, request,
pool_id=member.pool_id)
member.id = response.id
@ -870,7 +870,7 @@ class LBaaSv2Driver(base.LBaaSDriver):
'project_id': l7_policy.project_id,
'redirect_pool_id': l7_policy.redirect_pool_id,
}
self._add_tags('l7policy', request)
self.add_tags('l7policy', request)
response = self._post_lb_resource(o_l7p.L7Policy, request)
l7_policy.id = response['id']
return l7_policy
@ -904,7 +904,7 @@ class LBaaSv2Driver(base.LBaaSDriver):
'type': l7_rule.type,
'value': l7_rule.value
}
self._add_tags('rule', request)
self.add_tags('rule', request)
response = self._post_lb_resource(o_l7r.L7Rule, request,
l7policy_id=l7_rule.l7policy_id)
l7_rule.id = response['id']

48
kuryr_kubernetes/controller/handlers/lbaas.py

@ -13,6 +13,9 @@
# License for the specific language governing permissions and limitations
# under the License.
import eventlet
import time
from kuryr.lib._i18n import _
from oslo_log import log as logging
@ -20,6 +23,7 @@ from kuryr_kubernetes import clients
from kuryr_kubernetes import config
from kuryr_kubernetes import constants as k_const
from kuryr_kubernetes.controller.drivers import base as drv_base
from kuryr_kubernetes.controller.drivers import utils as driver_utils
from kuryr_kubernetes import exceptions as k_exc
from kuryr_kubernetes.handlers import k8s_base
from kuryr_kubernetes.objects import lbaas as obj_lbaas
@ -168,6 +172,7 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
!= 'default'):
self._lb_provider = (
config.CONF.kubernetes.endpoints_driver_octavia_provider)
eventlet.spawn(self._cleanup_leftover_lbaas)
def on_present(self, endpoints):
lbaas_spec = utils.get_lbaas_spec(endpoints)
@ -628,3 +633,46 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
lbaas_state.loadbalancer = lb
return changed
def _cleanup_leftover_lbaas(self):
lbaas_client = clients.get_loadbalancer_client()
services = []
try:
services = driver_utils.get_services().get('items')
except k_exc.K8sClientException:
LOG.debug("Skipping cleanup of leftover lbaas."
"Error retriving Kubernetes services")
return
services_set = set('{}/{}'.format(
service['metadata']['namespace'],
service['metadata']['name'])
for service in services)
lbaas_spec = {}
self._drv_lbaas.add_tags('loadbalancer', lbaas_spec)
loadbalancers = lbaas_client.load_balancers(**lbaas_spec)
for loadbalancer in loadbalancers:
lb_obj = obj_lbaas.LBaaSLoadBalancer(**loadbalancer)
if lb_obj.name not in services_set:
eventlet.spawn(self._ensure_release_lbaas, lb_obj)
def _ensure_release_lbaas(self, lb_obj):
attempts = 0
deadline = 0
retry = True
timeout = config.CONF.kubernetes.watch_retry_timeout
while retry:
try:
if attempts == 1:
deadline = time.time() + timeout
if (attempts > 0 and
utils.exponential_sleep(deadline, attempts) == 0):
LOG.error("Failed releasing lbaas '%s': deadline exceeded",
lb_obj.name)
return
self._drv_lbaas.release_loadbalancer(lb_obj)
retry = False
except k_exc.ResourceNotReady:
LOG.debug("Attempt (%s) of loadbalancer release %s failed."
" A retry will be triggered.", attempts, lb_obj.name)
attempts += 1
retry = True

8
kuryr_kubernetes/tests/unit/controller/drivers/test_lbaasv2.py

@ -44,7 +44,7 @@ class TestLBaaSv2Driver(test_base.TestCase):
lbaas.get_api_major_version.return_value = (2, 5)
d = d_lbaasv2.LBaaSv2Driver()
req = {}
d._add_tags('loadbalancer', req)
d.add_tags('loadbalancer', req)
self.assertEqual({'tags': ['foo']}, req)
def test_add_tags_no_tag(self):
@ -52,7 +52,7 @@ class TestLBaaSv2Driver(test_base.TestCase):
lbaas.get_api_major_version.return_value = (2, 5)
d = d_lbaasv2.LBaaSv2Driver()
req = {}
d._add_tags('loadbalancer', req)
d.add_tags('loadbalancer', req)
self.assertEqual({}, req)
def test_add_tags_no_support(self):
@ -64,7 +64,7 @@ class TestLBaaSv2Driver(test_base.TestCase):
d = d_lbaasv2.LBaaSv2Driver()
for res in ('loadbalancer', 'listener', 'pool', 'l7policy'):
req = {}
d._add_tags(res, req)
d.add_tags(res, req)
self.assertEqual({'description': 'foo'}, req,
'No description added to resource %s' % res)
@ -77,7 +77,7 @@ class TestLBaaSv2Driver(test_base.TestCase):
d = d_lbaasv2.LBaaSv2Driver()
for res in ('member', 'rule'):
req = {}
d._add_tags(res, req)
d.add_tags(res, req)
self.assertEqual({}, req, 'Unnecessary description added to '
'resource %s' % res)

9
kuryr_kubernetes/tests/unit/controller/handlers/test_ingress_lbaas.py

@ -24,9 +24,11 @@ from kuryr_kubernetes.tests.unit.controller.handlers import \
class TestIngressLoadBalancerHandler(t_lbaas.TestLoadBalancerHandler):
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas'
'.LoadBalancerHandler._cleanup_leftover_lbaas')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
'.LBaaSDriver.get_instance')
def test_init(self, m_get_drv_lbaas):
def test_init(self, m_get_drv_lbaas, m_cleanup_leftover_lbaas):
m_get_drv_lbaas.return_value = mock.sentinel.drv_lbaas
handler = h_ing_lbaas.IngressLoadBalancerHandler()
@ -122,6 +124,8 @@ class TestIngressLoadBalancerHandler(t_lbaas.TestLoadBalancerHandler):
for member in state.members)
return observed_targets
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas'
'.LoadBalancerHandler._cleanup_leftover_lbaas')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
'.PodSubnetsDriver.get_instance')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
@ -129,7 +133,8 @@ class TestIngressLoadBalancerHandler(t_lbaas.TestLoadBalancerHandler):
@mock.patch('kuryr_kubernetes.controller.drivers.base'
'.LBaaSDriver.get_instance')
def test__sync_lbaas_route_members(self, m_get_drv_lbaas,
m_get_drv_project, m_get_drv_subnets):
m_get_drv_project, m_get_drv_subnets,
m_cleanup_leftover_lbaas):
project_id = str(uuid.uuid4())
subnet_id = str(uuid.uuid4())
current_ip = '1.1.1.1'

28
kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py

@ -392,6 +392,9 @@ class FakeLBaaSDriver(drv_base.LBaaSDriver):
class TestLoadBalancerHandler(test_base.TestCase):
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas'
'.LoadBalancerHandler._cleanup_leftover_lbaas')
@mock.patch('kuryr_kubernetes.config.CONF')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
'.ServicePubIpDriver.get_instance')
@ -402,7 +405,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
@mock.patch('kuryr_kubernetes.controller.drivers.base'
'.LBaaSDriver.get_instance')
def test_init(self, m_get_drv_lbaas, m_get_drv_project,
m_get_drv_subnets, m_get_drv_service_pub_ip, m_cfg):
m_get_drv_subnets, m_get_drv_service_pub_ip, m_cfg,
m_cleanup_leftover_lbaas):
m_get_drv_lbaas.return_value = mock.sentinel.drv_lbaas
m_get_drv_project.return_value = mock.sentinel.drv_project
m_get_drv_subnets.return_value = mock.sentinel.drv_subnets
@ -416,6 +420,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
self.assertEqual(mock.sentinel.drv_lb_ip, handler._drv_service_pub_ip)
self.assertIsNone(handler._lb_provider)
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas'
'.LoadBalancerHandler._cleanup_leftover_lbaas')
@mock.patch('kuryr_kubernetes.config.CONF')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
'.ServicePubIpDriver.get_instance')
@ -427,7 +433,7 @@ class TestLoadBalancerHandler(test_base.TestCase):
'.LBaaSDriver.get_instance')
def test_init_provider_ovn(self, m_get_drv_lbaas, m_get_drv_project,
m_get_drv_subnets, m_get_drv_service_pub_ip,
m_cfg):
m_cfg, m_cleanup_leftover_lbaas):
m_get_drv_lbaas.return_value = mock.sentinel.drv_lbaas
m_get_drv_project.return_value = mock.sentinel.drv_project
m_get_drv_subnets.return_value = mock.sentinel.drv_subnets
@ -742,6 +748,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
for member in state.members)
return observed_targets
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas'
'.LoadBalancerHandler._cleanup_leftover_lbaas')
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas.'
'LoadBalancerHandler._sync_lbaas_sgs')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
@ -751,7 +759,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
@mock.patch('kuryr_kubernetes.controller.drivers.base'
'.LBaaSDriver.get_instance')
def test_sync_lbaas_members(self, m_get_drv_lbaas, m_get_drv_project,
m_get_drv_subnets, m_sync_lbaas_sgs):
m_get_drv_subnets, m_sync_lbaas_sgs,
m_cleanup_leftover_lbaas):
# REVISIT(ivc): test methods separately and verify ensure/release
project_id = str(uuid.uuid4())
subnet_id = str(uuid.uuid4())
@ -778,6 +787,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
self.assertEqual(sorted(expected_targets.items()), observed_targets)
self.assertEqual(expected_ip, str(state.loadbalancer.ip))
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas'
'.LoadBalancerHandler._cleanup_leftover_lbaas')
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas.'
'LoadBalancerHandler._sync_lbaas_sgs')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
@ -788,7 +799,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
'.LBaaSDriver.get_instance')
def test_sync_lbaas_members_udp(self, m_get_drv_lbaas,
m_get_drv_project, m_get_drv_subnets,
m_sync_lbaas_sgs):
m_sync_lbaas_sgs,
m_cleanup_leftover_lbaas):
# REVISIT(ivc): test methods separately and verify ensure/release
project_id = str(uuid.uuid4())
subnet_id = str(uuid.uuid4())
@ -815,6 +827,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
self.assertEqual([], observed_targets)
self.assertEqual(expected_ip, str(state.loadbalancer.ip))
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas'
'.LoadBalancerHandler._cleanup_leftover_lbaas')
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas.'
'LoadBalancerHandler._sync_lbaas_sgs')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
@ -825,7 +839,7 @@ class TestLoadBalancerHandler(test_base.TestCase):
'.LBaaSDriver.get_instance')
def test_sync_lbaas_members_svc_listener_port_edit(
self, m_get_drv_lbaas, m_get_drv_project, m_get_drv_subnets,
m_sync_lbaas_sgs):
m_sync_lbaas_sgs, m_cleanup_leftover_lbaas):
# REVISIT(ivc): test methods separately and verify ensure/release
project_id = str(uuid.uuid4())
subnet_id = str(uuid.uuid4())
@ -872,6 +886,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
self.skipTest("skipping until generalised annotation handling is "
"implemented")
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas'
'.LoadBalancerHandler._cleanup_leftover_lbaas')
@mock.patch('kuryr_kubernetes.controller.handlers.lbaas.'
'LoadBalancerHandler._sync_lbaas_sgs')
@mock.patch('kuryr_kubernetes.controller.drivers.base'
@ -882,7 +898,7 @@ class TestLoadBalancerHandler(test_base.TestCase):
'.LBaaSDriver.get_instance')
def test_add_new_members_udp(self, m_get_drv_lbaas,
m_get_drv_project, m_get_drv_subnets,
m_sync_lbaas_sgs):
m_sync_lbaas_sgs, m_cleanup_leftover_lbaas):
project_id = str(uuid.uuid4())
subnet_id = str(uuid.uuid4())
current_ip = '1.1.1.1'