openstack
/
kuryr-kubernetes
Code Issues Proposed changes

Fix LBaaS SG rules update 

The LBaaS SG update is failing when the pods selected by the selector
in the rule block are removed after the pod, on which the policy is
enforced, is removed. This commit fixes the issue by changing from
LBaaSServiceSpec object to LBaaSLoadBalancer, which is the object
type expected by '_apply_members_security_groups' function.

Change-Id: I17f2f632e02bc0f46ccc7434173acce68aef957b
Closes-Bug: 1823022
This commit is contained in:
Maysa Macedo 2019-04-03 14:35:22 +00:00
parent 4a3b23d17b
commit ae1d1dd51a
4 changed files with 67 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
kuryr_kubernetes/controller/drivers/lbaasv2.py
View File

@ -940,12 +940,19 @@ class LBaaSv2Driver(base.LBaaSDriver):
svc_ports = service['spec']['ports']
lbaas_name = "%s/%s" % (svc_namespace, svc_name)
lbaas = utils.get_lbaas_spec(service)
endpoints_link = utils.get_endpoints_link(service)
k8s = clients.get_kubernetes_client()
endpoint = k8s.get(endpoints_link)
lbaas = utils.get_lbaas_state(endpoint)
if not lbaas:
return
lbaas.security_groups_ids = sgs
utils.set_lbaas_spec(service, lbaas)
lbaas_obj = lbaas.loadbalancer
lbaas_obj.security_groups = sgs
utils.set_lbaas_state(endpoint, lbaas)
for port in svc_ports:
port_protocol = port['protocol']
@ -953,6 +960,6 @@ class LBaaSv2Driver(base.LBaaSDriver):
target_port = port['targetPort']
sg_rule_name = "%s:%s:%s" % (lbaas_name, port_protocol, lbaas_port)
self._apply_members_security_groups(lbaas, lbaas_port,
self._apply_members_security_groups(lbaas_obj, lbaas_port,
target_port, port_protocol,
sg_rule_name, sgs)

33
kuryr_kubernetes/controller/handlers/lbaas.py
View File

@ -177,7 +177,7 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
endpoints['metadata']['name'])
return
lbaas_state = self._get_lbaas_state(endpoints)
lbaas_state = utils.get_lbaas_state(endpoints)
if not lbaas_state:
lbaas_state = obj_lbaas.LBaaSState()
@ -209,7 +209,7 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
# failing items, or validate configuration) to prevent annotation
# being out of sync with the actual Neutron state.
try:
self._set_lbaas_state(endpoints, lbaas_state)
utils.set_lbaas_state(endpoints, lbaas_state)
except k_exc.K8sResourceNotFound:
# Note(yboaron) It's impossible to store neutron resources
# in K8S object since object was deleted. In that case
@ -220,7 +220,7 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
def on_deleted(self, endpoints, lbaas_state=None):
if lbaas_state is None:
lbaas_state = self._get_lbaas_state(endpoints)
lbaas_state = utils.get_lbaas_state(endpoints)
if not lbaas_state:
return
# NOTE(ivc): deleting pool deletes its members
@ -606,30 +606,3 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
obj = obj_lbaas.LBaaSServiceSpec.obj_from_primitive(obj_dict)
LOG.debug("Got LBaaSServiceSpec from annotation: %r", obj)
return obj
def _set_lbaas_state(self, endpoints, lbaas_state):
# TODO(ivc): extract annotation interactions
if lbaas_state is None:
LOG.debug("Removing LBaaSState annotation: %r", lbaas_state)
annotation = None
else:
lbaas_state.obj_reset_changes(recursive=True)
LOG.debug("Setting LBaaSState annotation: %r", lbaas_state)
annotation = jsonutils.dumps(lbaas_state.obj_to_primitive(),
sort_keys=True)
k8s = clients.get_kubernetes_client()
k8s.annotate(endpoints['metadata']['selfLink'],
{k_const.K8S_ANNOTATION_LBAAS_STATE: annotation},
resource_version=endpoints['metadata']['resourceVersion'])
def _get_lbaas_state(self, endpoints):
# TODO(ivc): same as '_set_lbaas_state'
try:
annotations = endpoints['metadata']['annotations']
annotation = annotations[k_const.K8S_ANNOTATION_LBAAS_STATE]
except KeyError:
return None
obj_dict = jsonutils.loads(annotation)
obj = obj_lbaas.LBaaSState.obj_from_primitive(obj_dict)
LOG.debug("Got LBaaSState from annotation: %r", obj)
return obj

40
kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py
View File

@ -441,7 +441,9 @@ class TestLoadBalancerHandler(test_base.TestCase):
self.assertEqual(mock.sentinel.drv_lb_ip, handler._drv_service_pub_ip)
self.assertEqual('ovn', handler._lb_provider)
def test_on_present(self):
@mock.patch('kuryr_kubernetes.utils.set_lbaas_state')
@mock.patch('kuryr_kubernetes.utils.get_lbaas_state')
def test_on_present(self, m_get_lbaas_state, m_set_lbaas_state):
lbaas_spec = mock.sentinel.lbaas_spec
lbaas_spec.type = 'DummyType'
lbaas_spec.lb_ip = "1.2.3.4"
@ -461,7 +463,7 @@ class TestLoadBalancerHandler(test_base.TestCase):
m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
m_handler._get_lbaas_spec.return_value = lbaas_spec
m_handler._should_ignore.return_value = False
m_handler._get_lbaas_state.return_value = lbaas_state
m_get_lbaas_state.return_value = lbaas_state
m_handler._sync_lbaas_members.return_value = True
m_handler._drv_service_pub_ip = m_drv_service_pub_ip
@ -469,10 +471,10 @@ class TestLoadBalancerHandler(test_base.TestCase):
m_handler._get_lbaas_spec.assert_called_once_with(endpoints)
m_handler._should_ignore.assert_called_once_with(endpoints, lbaas_spec)
m_handler._get_lbaas_state.assert_called_once_with(endpoints)
m_get_lbaas_state.assert_called_once_with(endpoints)
m_handler._sync_lbaas_members.assert_called_once_with(
endpoints, lbaas_state, lbaas_spec)
m_handler._set_lbaas_state.assert_called_once_with(
m_set_lbaas_state.assert_called_once_with(
endpoints, lbaas_state)
m_handler._update_lb_status.assert_not_called()
@ -483,7 +485,10 @@ class TestLoadBalancerHandler(test_base.TestCase):
lbaas_state.service_pub_ip_info = None
return True
def test_on_present_loadbalancer_service(self):
@mock.patch('kuryr_kubernetes.utils.set_lbaas_state')
@mock.patch('kuryr_kubernetes.utils.get_lbaas_state')
def test_on_present_loadbalancer_service(self, m_get_lbaas_state,
m_set_lbaas_state):
lbaas_spec = mock.sentinel.lbaas_spec
lbaas_spec.type = 'LoadBalancer'
lbaas_spec.lb_ip = "1.2.3.4"
@ -508,7 +513,7 @@ class TestLoadBalancerHandler(test_base.TestCase):
m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
m_handler._get_lbaas_spec.return_value = lbaas_spec
m_handler._should_ignore.return_value = False
m_handler._get_lbaas_state.return_value = lbaas_state
m_get_lbaas_state.return_value = lbaas_state
m_handler._sync_lbaas_members = self._fake_sync_lbaas_members
m_handler._drv_service_pub_ip = m_drv_service_pub_ip
@ -516,12 +521,15 @@ class TestLoadBalancerHandler(test_base.TestCase):
m_handler._get_lbaas_spec.assert_called_once_with(endpoints)
m_handler._should_ignore.assert_called_once_with(endpoints, lbaas_spec)
m_handler._get_lbaas_state.assert_called_once_with(endpoints)
m_handler._set_lbaas_state.assert_called_once_with(
m_get_lbaas_state.assert_called_once_with(endpoints)
m_set_lbaas_state.assert_called_once_with(
endpoints, lbaas_state)
m_handler._update_lb_status.assert_called()
def test_on_present_rollback(self):
@mock.patch('kuryr_kubernetes.utils.set_lbaas_state')
@mock.patch('kuryr_kubernetes.utils.get_lbaas_state')
def test_on_present_rollback(self, m_get_lbaas_state,
m_set_lbaas_state):
lbaas_spec = mock.sentinel.lbaas_spec
lbaas_spec.type = 'ClusterIp'
lbaas_spec.lb_ip = '1.2.3.4'
@ -540,26 +548,28 @@ class TestLoadBalancerHandler(test_base.TestCase):
m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
m_handler._get_lbaas_spec.return_value = lbaas_spec
m_handler._should_ignore.return_value = False
m_handler._get_lbaas_state.return_value = lbaas_state
m_get_lbaas_state.return_value = lbaas_state
m_handler._sync_lbaas_members.return_value = True
m_handler._set_lbaas_state.side_effect = (
m_set_lbaas_state.side_effect = (
k_exc.K8sResourceNotFound('ep'))
m_handler._drv_service_pub_ip = m_drv_service_pub_ip
h_lbaas.LoadBalancerHandler.on_present(m_handler, endpoints)
m_handler._get_lbaas_spec.assert_called_once_with(endpoints)
m_handler._should_ignore.assert_called_once_with(endpoints, lbaas_spec)
m_handler._get_lbaas_state.assert_called_once_with(endpoints)
m_get_lbaas_state.assert_called_once_with(endpoints)
m_handler._sync_lbaas_members.assert_called_once_with(
endpoints, lbaas_state, lbaas_spec)
m_handler._set_lbaas_state.assert_called_once_with(
m_set_lbaas_state.assert_called_once_with(
endpoints, lbaas_state)
m_handler.on_deleted.assert_called_once_with(
endpoints, lbaas_state)
@mock.patch('kuryr_kubernetes.utils.get_lbaas_state')
@mock.patch('kuryr_kubernetes.objects.lbaas'
'.LBaaSServiceSpec')
def test_on_cascade_deleted_lb_service(self, m_svc_spec_ctor):
def test_on_cascade_deleted_lb_service(self, m_svc_spec_ctor,
m_get_lbaas_state):
endpoints = mock.sentinel.endpoints
empty_spec = mock.sentinel.empty_spec
lbaas_state = mock.Mock()
@ -568,7 +578,7 @@ class TestLoadBalancerHandler(test_base.TestCase):
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_get_lbaas_state.return_value = lbaas_state
m_handler._drv_lbaas = mock.Mock()
m_handler._drv_service_pub_ip = mock.Mock()

28
kuryr_kubernetes/utils.py
View File

@ -252,6 +252,34 @@ def set_lbaas_spec(service, lbaas_spec):
resource_version=service['metadata']['resourceVersion'])
def get_lbaas_state(endpoint):
try:
annotations = endpoint['metadata']['annotations']
annotation = annotations[constants.K8S_ANNOTATION_LBAAS_STATE]
except KeyError:
return None
obj_dict = jsonutils.loads(annotation)
obj = obj_lbaas.LBaaSState.obj_from_primitive(obj_dict)
LOG.debug("Got LBaaSState from annotation: %r", obj)
return obj
def set_lbaas_state(endpoints, lbaas_state):
# TODO(ivc): extract annotation interactions
if lbaas_state is None:
LOG.debug("Removing LBaaSState annotation: %r", lbaas_state)
annotation = None
else:
lbaas_state.obj_reset_changes(recursive=True)
LOG.debug("Setting LBaaSState annotation: %r", lbaas_state)
annotation = jsonutils.dumps(lbaas_state.obj_to_primitive(),
sort_keys=True)
k8s = clients.get_kubernetes_client()
k8s.annotate(endpoints['metadata']['selfLink'],
{constants.K8S_ANNOTATION_LBAAS_STATE: annotation},
resource_version=endpoints['metadata']['resourceVersion'])
def get_endpoints_link(service):
svc_link = service['metadata']['selfLink']
link_parts = svc_link.split('/')