openstack
/
kuryr-kubernetes
Code Issues Proposed changes

Fix SG rules on targetPort update 

After applying a Network Policy and updating an existent Service so that
all 'targetPorts' are allowed by the policy, the SG rules are not being
created with the required 'remote_ip_prefix'. Also, when the service is
again updated with a 'targetPort' that is not allowed by the policy the
respective SG rule is not deleted.
This commit fixes the issue by associating 'targetPort' field to the 
'LBaaSPortSpec' versioned object, which allows Kuryr to accounts for
changes in not only 'name', 'port' and 'protocol' Kubernetes services'
fields, but also 'targetPorts'. In addition, the LBaaS SG from the
LBaaS state annotation is updated to match the SG stated in the
LBaaS spec annotation, which has the updated SG to be applied.

Closes-Bug: #1814920
Change-Id: Ifcdd1889a813c1eb078064facfb2ede83a179887
This commit is contained in:
Maysa Macedo 2019-02-05 20:49:08 +00:00 committed by Maysa de Macedo Souza
parent 0437fac8a4
commit 9c2fcbc3e3
4 changed files with 55 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -122,7 +122,8 @@ class LBaaSSpecHandler(k8s_base.ResourceEventHandler):
def _get_service_ports(self, service): def _get_service_ports(self, service):
return [{'name': port.get('name'), return [{'name': port.get('name'),
'protocol': port.get('protocol', 'TCP'), 'protocol': port.get('protocol', 'TCP'),
'port': port['port']} 'port': port['port'],
'targetPort': port['targetPort']}
for port in service['spec']['ports']] for port in service['spec']['ports']]
def _has_port_changes(self, service, lbaas_spec): def _has_port_changes(self, service, lbaas_spec):
@ -131,7 +132,10 @@ class LBaaSSpecHandler(k8s_base.ResourceEventHandler):
fields = obj_lbaas.LBaaSPortSpec.fields fields = obj_lbaas.LBaaSPortSpec.fields
svc_port_set = {tuple(port[attr] for attr in fields) svc_port_set = {tuple(port[attr] for attr in fields)
for port in self._get_service_ports(service)} for port in self._get_service_ports(service)}
spec_port_set = {tuple(getattr(port, attr) for attr in fields)
spec_port_set = {tuple(getattr(port, attr)
for attr in fields
if port.obj_attr_is_set(attr))
for port in lbaas_spec.ports} for port in lbaas_spec.ports}
if svc_port_set != spec_port_set: if svc_port_set != spec_port_set:
@ -311,11 +315,17 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
self._is_lbaas_spec_in_sync(endpoints, lbaas_spec)) self._is_lbaas_spec_in_sync(endpoints, lbaas_spec))
def _is_lbaas_spec_in_sync(self, endpoints, lbaas_spec): def _is_lbaas_spec_in_sync(self, endpoints, lbaas_spec):
# REVISIT(ivc): consider other options instead of using 'name' ports = lbaas_spec.ports
ep_ports = list(set(port.get('name') ep_ports = list(set((port.get('name'), port.get('port'))
if ports[0].obj_attr_is_set('targetPort')
else port.get('name')
for subset in endpoints.get('subsets', []) for subset in endpoints.get('subsets', [])
for port in subset.get('ports', []))) for port in subset.get('ports', [])))
spec_ports = [port.name for port in lbaas_spec.ports]
spec_ports = [(port.name, port.targetPort)
if port.obj_attr_is_set('targetPort')
else port.name
for port in ports]
return sorted(ep_ports) == sorted(spec_ports) return sorted(ep_ports) == sorted(spec_ports)
@ -352,6 +362,7 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
pool_by_lsnr_port = {(lsnr_by_id[p.listener_id].protocol, pool_by_lsnr_port = {(lsnr_by_id[p.listener_id].protocol,
lsnr_by_id[p.listener_id].port): p lsnr_by_id[p.listener_id].port): p
for p in lbaas_state.pools} for p in lbaas_state.pools}
# NOTE(yboaron): Since LBaaSv2 doesn't support UDP load balancing, # NOTE(yboaron): Since LBaaSv2 doesn't support UDP load balancing,
# the LBaaS driver will return 'None' in case of UDP port # the LBaaS driver will return 'None' in case of UDP port
# listener creation. # listener creation.
@ -387,6 +398,7 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
except KeyError: except KeyError:
LOG.debug("No pool found for port: %r", port_name) LOG.debug("No pool found for port: %r", port_name)
continue continue
if (target_ip, target_port, pool.id) in current_targets: if (target_ip, target_port, pool.id) in current_targets:
continue continue
# TODO(apuimedo): Do not pass subnet_id at all when in # TODO(apuimedo): Do not pass subnet_id at all when in
@ -410,6 +422,7 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
listener_port = lsnr_by_id[pool.listener_id].port listener_port = lsnr_by_id[pool.listener_id].port
else: else:
listener_port = None listener_port = None
member = self._drv_lbaas.ensure_member( member = self._drv_lbaas.ensure_member(
loadbalancer=lbaas_state.loadbalancer, loadbalancer=lbaas_state.loadbalancer,
pool=pool, pool=pool,
@ -636,6 +649,15 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
lbaas_state.service_pub_ip_info = None lbaas_state.service_pub_ip_info = None
changed = True changed = True
default_sgs = config.CONF.neutron_defaults.pod_security_groups
lbaas_spec_sgs = lbaas_spec.security_groups_ids
if lb.security_groups and lb.security_groups != lbaas_spec_sgs:
sgs = [lb_sg for lb_sg in lb.security_groups
if lb_sg not in default_sgs]
if lbaas_spec_sgs != default_sgs:
sgs.extend(lbaas_spec_sgs)
lb.security_groups = sgs
lbaas_state.loadbalancer = lb lbaas_state.loadbalancer = lb
return changed return changed

5
kuryr_kubernetes/objects/lbaas.py
View File

@ -120,12 +120,15 @@ class LBaaSState(k_obj.KuryrK8sObjectBase):
@obj_base.VersionedObjectRegistry.register @obj_base.VersionedObjectRegistry.register
class LBaaSPortSpec(k_obj.KuryrK8sObjectBase): class LBaaSPortSpec(k_obj.KuryrK8sObjectBase):
VERSION = '1.0' VERSION = '1.1'
# Version 1.0: Initial version
# Version 1.1: Added targetPort field.
fields = { fields = {
'name': obj_fields.StringField(nullable=True), 'name': obj_fields.StringField(nullable=True),
'protocol': obj_fields.StringField(), 'protocol': obj_fields.StringField(),
'port': obj_fields.IntegerField(), 'port': obj_fields.IntegerField(),
'targetPort': obj_fields.IntegerField(),
} }

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

@ -201,12 +201,12 @@ class TestLBaaSSpecHandler(test_base.TestCase):
def test_get_service_ports(self): def test_get_service_ports(self):
m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler) m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
service = {'spec': {'ports': [ service = {'spec': {'ports': [
{'port': 1}, {'port': 1, 'targetPort': 1},
{'port': 2, 'name': 'X', 'protocol': 'UDP'} {'port': 2, 'name': 'X', 'protocol': 'UDP', 'targetPort': 2}
]}} ]}}
expected_ret = [ expected_ret = [
{'port': 1, 'name': None, 'protocol': 'TCP'}, {'port': 1, 'name': None, 'protocol': 'TCP', 'targetPort': 1},
{'port': 2, 'name': 'X', 'protocol': 'UDP'}] {'port': 2, 'name': 'X', 'protocol': 'UDP', 'targetPort': 2}]
ret = h_lbaas.LBaaSSpecHandler._get_service_ports(m_handler, service) ret = h_lbaas.LBaaSSpecHandler._get_service_ports(m_handler, service)
self.assertEqual(expected_ret, ret) self.assertEqual(expected_ret, ret)
@ -215,13 +215,15 @@ class TestLBaaSSpecHandler(test_base.TestCase):
m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler) m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
m_service = mock.MagicMock() m_service = mock.MagicMock()
m_handler._get_service_ports.return_value = [ m_handler._get_service_ports.return_value = [
{'port': 1, 'name': 'X', 'protocol': 'TCP'}, {'port': 1, 'name': 'X', 'protocol': 'TCP', 'targetPort': 1},
] ]
m_lbaas_spec = mock.MagicMock() m_lbaas_spec = mock.MagicMock()
m_lbaas_spec.ports = [ m_lbaas_spec.ports = [
obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1), obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1,
obj_lbaas.LBaaSPortSpec(name='Y', protocol='TCP', port=2), targetPort=1),
obj_lbaas.LBaaSPortSpec(name='Y', protocol='TCP', port=2,
targetPort=2),
] ]
ret = h_lbaas.LBaaSSpecHandler._has_port_changes( ret = h_lbaas.LBaaSSpecHandler._has_port_changes(
@ -233,14 +235,16 @@ class TestLBaaSSpecHandler(test_base.TestCase):
m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler) m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
m_service = mock.MagicMock() m_service = mock.MagicMock()
m_handler._get_service_ports.return_value = [ m_handler._get_service_ports.return_value = [
{'port': 1, 'name': 'X', 'protocol': 'TCP'}, {'port': 1, 'name': 'X', 'protocol': 'TCP', 'targetPort': 1},
{'port': 2, 'name': 'Y', 'protocol': 'TCP'} {'port': 2, 'name': 'Y', 'protocol': 'TCP', 'targetPort': 2}
] ]
m_lbaas_spec = mock.MagicMock() m_lbaas_spec = mock.MagicMock()
m_lbaas_spec.ports = [ m_lbaas_spec.ports = [
obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1), obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1,
obj_lbaas.LBaaSPortSpec(name='Y', protocol='TCP', port=2), targetPort=1),
obj_lbaas.LBaaSPortSpec(name='Y', protocol='TCP', port=2,
targetPort=2),
] ]
ret = h_lbaas.LBaaSSpecHandler._has_port_changes( ret = h_lbaas.LBaaSSpecHandler._has_port_changes(
@ -673,9 +677,11 @@ class TestLoadBalancerHandler(test_base.TestCase):
def test_is_lbaas_spec_in_sync(self): def test_is_lbaas_spec_in_sync(self):
names = ['a', 'b', 'c'] names = ['a', 'b', 'c']
endpoints = {'subsets': [{'ports': [{'name': n} for n in names]}]} endpoints = {'subsets': [{'ports': [{'name': n, 'port': 1}
for n in names]}]}
lbaas_spec = obj_lbaas.LBaaSServiceSpec(ports=[ lbaas_spec = obj_lbaas.LBaaSServiceSpec(ports=[
obj_lbaas.LBaaSPortSpec(name=n) for n in reversed(names)]) obj_lbaas.LBaaSPortSpec(name=n, targetPort=1)
for n in reversed(names)])
m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler) m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
ret = h_lbaas.LoadBalancerHandler._is_lbaas_spec_in_sync( ret = h_lbaas.LoadBalancerHandler._is_lbaas_spec_in_sync(
@ -748,10 +754,11 @@ class TestLoadBalancerHandler(test_base.TestCase):
ip=vip, ip=vip,
project_id=project_id, project_id=project_id,
subnet_id=subnet_id, subnet_id=subnet_id,
ports=[obj_lbaas.LBaaSPortSpec(name=str(port), ports=[obj_lbaas.LBaaSPortSpec(name=str(t[0]),
protocol=prot, protocol=prot,
port=port) port=t[0],
for port in set(t[0] for t in targets.values())], targetPort=t[1])
for t in targets.values()],
type=lbaas_type) type=lbaas_type)
def _generate_endpoints(self, targets): def _generate_endpoints(self, targets):

2
kuryr_kubernetes/tests/unit/test_object.py
View File

@ -28,7 +28,7 @@ object_data = {
'LBaaSLoadBalancer': '1.3-8bc0a9bdbd160da67572aa38784378d1', 'LBaaSLoadBalancer': '1.3-8bc0a9bdbd160da67572aa38784378d1',
'LBaaSMember': '1.0-a770c6884c27d6d8c21186b27d0e2ccb', 'LBaaSMember': '1.0-a770c6884c27d6d8c21186b27d0e2ccb',
'LBaaSPool': '1.1-6e77370d7632a902445444249eb77b01', 'LBaaSPool': '1.1-6e77370d7632a902445444249eb77b01',
'LBaaSPortSpec': '1.0-51dfa3436bec32db3614720056fcc83f', 'LBaaSPortSpec': '1.1-fcfa2fd07f4bc5619b96fa41bcdf6e23',
'LBaaSPubIp': '1.0-83992edec2c60fb4ab8998ea42a4ff74', 'LBaaSPubIp': '1.0-83992edec2c60fb4ab8998ea42a4ff74',
'LBaaSRouteNotifEntry': '1.0-dd2f2be956f68814b1f47cb13483a885', 'LBaaSRouteNotifEntry': '1.0-dd2f2be956f68814b1f47cb13483a885',
'LBaaSRouteNotifier': '1.0-f0bfd8e772434abe7557930d7e0180c1', 'LBaaSRouteNotifier': '1.0-f0bfd8e772434abe7557930d7e0180c1',