Browse Source

Add support for svc with text targetPorts

This patch adds support for services that define the targetPort
with text (port name), pointing to the same port number as the defined
exposed port on the svc.

Closes-Bug: 1818969
Change-Id: I7f957d292f7c4a43b759292e5bd04c4db704c4c4
Luis Tomas Bolivar 1 month ago
parent
commit
dfa9a392f1

+ 19
- 44
kuryr_kubernetes/controller/handlers/lbaas.py View File

@@ -118,33 +118,7 @@ class LBaaSSpecHandler(k8s_base.ResourceEventHandler):
118 118
 
119 119
     def _has_lbaas_spec_changes(self, service, lbaas_spec):
120 120
         return (self._has_ip_changes(service, lbaas_spec) or
121
-                self._has_port_changes(service, lbaas_spec))
122
-
123
-    def _get_service_ports(self, service):
124
-        return [{'name': port.get('name'),
125
-                 'protocol': port.get('protocol', 'TCP'),
126
-                 'port': port['port'],
127
-                 'targetPort': port['targetPort']}
128
-                for port in service['spec']['ports']]
129
-
130
-    def _has_port_changes(self, service, lbaas_spec):
131
-        link = service['metadata']['selfLink']
132
-
133
-        fields = obj_lbaas.LBaaSPortSpec.fields
134
-        svc_port_set = {tuple(port[attr] for attr in fields)
135
-                        for port in self._get_service_ports(service)}
136
-
137
-        spec_port_set = {tuple(getattr(port, attr)
138
-                         for attr in fields
139
-                         if port.obj_attr_is_set(attr))
140
-                         for port in lbaas_spec.ports}
141
-
142
-        if svc_port_set != spec_port_set:
143
-            LOG.debug("LBaaS spec ports %(spec_ports)s != %(svc_ports)s "
144
-                      "for %(link)s" % {'spec_ports': spec_port_set,
145
-                                        'svc_ports': svc_port_set,
146
-                                        'link': link})
147
-        return svc_port_set != spec_port_set
121
+                utils.has_port_changes(service, lbaas_spec))
148 122
 
149 123
     def _has_ip_changes(self, service, lbaas_spec):
150 124
         link = service['metadata']['selfLink']
@@ -166,7 +140,7 @@ class LBaaSSpecHandler(k8s_base.ResourceEventHandler):
166 140
 
167 141
     def _generate_lbaas_port_specs(self, service):
168 142
         return [obj_lbaas.LBaaSPortSpec(**port)
169
-                for port in self._get_service_ports(service)]
143
+                for port in utils.get_service_ports(service)]
170 144
 
171 145
 
172 146
 class LoadBalancerHandler(k8s_base.ResourceEventHandler):
@@ -262,24 +236,25 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
262 236
                                      obj_lbaas.LBaaSServiceSpec())
263 237
 
264 238
     def _should_ignore(self, endpoints, lbaas_spec):
239
+        # NOTE(ltomasbo): we must wait until service handler has annotated the
240
+        # endpoints to process them. Thus, if annotations are not updated to
241
+        # match the endpoints information, we should skip the event
265 242
         return not(lbaas_spec and
266 243
                    self._has_pods(endpoints) and
267
-                   self._is_lbaas_spec_in_sync(endpoints, lbaas_spec))
268
-
269
-    def _is_lbaas_spec_in_sync(self, endpoints, lbaas_spec):
270
-        ports = lbaas_spec.ports
271
-        ep_ports = list(set((port.get('name'), port.get('port'))
272
-                            if ports[0].obj_attr_is_set('targetPort')
273
-                            else port.get('name')
274
-                            for subset in endpoints.get('subsets', [])
275
-                            for port in subset.get('ports', [])))
276
-
277
-        spec_ports = [(port.name, port.targetPort)
278
-                      if port.obj_attr_is_set('targetPort')
279
-                      else port.name
280
-                      for port in ports]
281
-
282
-        return sorted(ep_ports) == sorted(spec_ports)
244
+                   self._svc_handler_annotations_updated(endpoints,
245
+                                                         lbaas_spec))
246
+
247
+    def _svc_handler_annotations_updated(self, endpoints, lbaas_spec):
248
+        svc_link = self._get_service_link(endpoints)
249
+        k8s = clients.get_kubernetes_client()
250
+        service = k8s.get(svc_link)
251
+        if utils.has_port_changes(service, lbaas_spec):
252
+            # NOTE(ltomasbo): Ensuring lbaas_spec annotated on the endpoints
253
+            # is in sync with the service status, i.e., upon a service
254
+            # modification it will ensure endpoint modifications are not
255
+            # handled until the service handler has performed its annotations
256
+            return False
257
+        return True
283 258
 
284 259
     def _has_pods(self, endpoints):
285 260
         ep_subsets = endpoints.get('subsets', [])

+ 1
- 1
kuryr_kubernetes/objects/lbaas.py View File

@@ -128,7 +128,7 @@ class LBaaSPortSpec(k_obj.KuryrK8sObjectBase):
128 128
         'name': obj_fields.StringField(nullable=True),
129 129
         'protocol': obj_fields.StringField(),
130 130
         'port': obj_fields.IntegerField(),
131
-        'targetPort': obj_fields.IntegerField(),
131
+        'targetPort': obj_fields.StringField(),
132 132
     }
133 133
 
134 134
 

+ 13
- 78
kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py View File

@@ -193,7 +193,8 @@ class TestLBaaSSpecHandler(test_base.TestCase):
193 193
         m_drv_sg.get_security_groups.assert_called_once_with(
194 194
             service, project_id)
195 195
 
196
-    def test_has_lbaas_spec_changes(self):
196
+    @mock.patch('kuryr_kubernetes.utils.has_port_changes')
197
+    def test_has_lbaas_spec_changes(self, m_port_changes):
197 198
         m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
198 199
         service = mock.sentinel.service
199 200
         lbaas_spec = mock.sentinel.lbaas_spec
@@ -201,65 +202,11 @@ class TestLBaaSSpecHandler(test_base.TestCase):
201 202
         for has_ip_changes in (True, False):
202 203
             for has_port_changes in (True, False):
203 204
                 m_handler._has_ip_changes.return_value = has_ip_changes
204
-                m_handler._has_port_changes.return_value = has_port_changes
205
+                m_port_changes.return_value = has_port_changes
205 206
                 ret = h_lbaas.LBaaSSpecHandler._has_lbaas_spec_changes(
206 207
                     m_handler, service, lbaas_spec)
207 208
                 self.assertEqual(has_ip_changes or has_port_changes, ret)
208 209
 
209
-    def test_get_service_ports(self):
210
-        m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
211
-        service = {'spec': {'ports': [
212
-            {'port': 1, 'targetPort': 1},
213
-            {'port': 2, 'name': 'X', 'protocol': 'UDP', 'targetPort': 2}
214
-        ]}}
215
-        expected_ret = [
216
-            {'port': 1, 'name': None, 'protocol': 'TCP', 'targetPort': 1},
217
-            {'port': 2, 'name': 'X', 'protocol': 'UDP', 'targetPort': 2}]
218
-
219
-        ret = h_lbaas.LBaaSSpecHandler._get_service_ports(m_handler, service)
220
-        self.assertEqual(expected_ret, ret)
221
-
222
-    def test_has_port_changes(self):
223
-        m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
224
-        m_service = mock.MagicMock()
225
-        m_handler._get_service_ports.return_value = [
226
-            {'port': 1, 'name': 'X', 'protocol': 'TCP', 'targetPort': 1},
227
-        ]
228
-
229
-        m_lbaas_spec = mock.MagicMock()
230
-        m_lbaas_spec.ports = [
231
-            obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1,
232
-                                    targetPort=1),
233
-            obj_lbaas.LBaaSPortSpec(name='Y', protocol='TCP', port=2,
234
-                                    targetPort=2),
235
-        ]
236
-
237
-        ret = h_lbaas.LBaaSSpecHandler._has_port_changes(
238
-            m_handler, m_service, m_lbaas_spec)
239
-
240
-        self.assertTrue(ret)
241
-
242
-    def test_has_port_changes__no_changes(self):
243
-        m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
244
-        m_service = mock.MagicMock()
245
-        m_handler._get_service_ports.return_value = [
246
-            {'port': 1, 'name': 'X', 'protocol': 'TCP', 'targetPort': 1},
247
-            {'port': 2, 'name': 'Y', 'protocol': 'TCP', 'targetPort': 2}
248
-        ]
249
-
250
-        m_lbaas_spec = mock.MagicMock()
251
-        m_lbaas_spec.ports = [
252
-            obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1,
253
-                                    targetPort=1),
254
-            obj_lbaas.LBaaSPortSpec(name='Y', protocol='TCP', port=2,
255
-                                    targetPort=2),
256
-        ]
257
-
258
-        ret = h_lbaas.LBaaSSpecHandler._has_port_changes(
259
-            m_handler, m_service, m_lbaas_spec)
260
-
261
-        self.assertFalse(ret)
262
-
263 210
     def test_has_ip_changes(self):
264 211
         m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
265 212
         m_service = mock.MagicMock()
@@ -302,9 +249,10 @@ class TestLBaaSSpecHandler(test_base.TestCase):
302 249
             m_handler, m_service, m_lbaas_spec)
303 250
         self.assertFalse(ret)
304 251
 
305
-    def test_generate_lbaas_port_specs(self):
252
+    @mock.patch('kuryr_kubernetes.utils.get_service_ports')
253
+    def test_generate_lbaas_port_specs(self, m_get_service_ports):
306 254
         m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
307
-        m_handler._get_service_ports.return_value = [
255
+        m_get_service_ports.return_value = [
308 256
             {'port': 1, 'name': 'X', 'protocol': 'TCP'},
309 257
             {'port': 2, 'name': 'Y', 'protocol': 'TCP'}
310 258
         ]
@@ -316,12 +264,13 @@ class TestLBaaSSpecHandler(test_base.TestCase):
316 264
         ret = h_lbaas.LBaaSSpecHandler._generate_lbaas_port_specs(
317 265
             m_handler, mock.sentinel.service)
318 266
         self.assertEqual(expected_ports, ret)
319
-        m_handler._get_service_ports.assert_called_once_with(
267
+        m_get_service_ports.assert_called_once_with(
320 268
             mock.sentinel.service)
321 269
 
322
-    def test_generate_lbaas_port_specs_udp(self):
270
+    @mock.patch('kuryr_kubernetes.utils.get_service_ports')
271
+    def test_generate_lbaas_port_specs_udp(self, m_get_service_ports):
323 272
         m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler)
324
-        m_handler._get_service_ports.return_value = [
273
+        m_get_service_ports.return_value = [
325 274
             {'port': 1, 'name': 'X', 'protocol': 'TCP'},
326 275
             {'port': 2, 'name': 'Y', 'protocol': 'UDP'}
327 276
         ]
@@ -333,7 +282,7 @@ class TestLBaaSSpecHandler(test_base.TestCase):
333 282
         ret = h_lbaas.LBaaSSpecHandler._generate_lbaas_port_specs(
334 283
             m_handler, mock.sentinel.service)
335 284
         self.assertEqual(expected_ports, ret)
336
-        m_handler._get_service_ports.assert_called_once_with(
285
+        m_get_service_ports.assert_called_once_with(
337 286
             mock.sentinel.service)
338 287
 
339 288
     def test_set_lbaas_spec(self):
@@ -657,30 +606,16 @@ class TestLoadBalancerHandler(test_base.TestCase):
657 606
         # REVISIT(ivc): ddt?
658 607
         m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
659 608
         m_handler._has_pods.return_value = True
660
-        m_handler._is_lbaas_spec_in_sync.return_value = True
609
+        m_handler._svc_handler_annotations_updated.return_value = True
661 610
 
662 611
         ret = h_lbaas.LoadBalancerHandler._should_ignore(
663 612
             m_handler, endpoints, lbaas_spec)
664 613
         self.assertEqual(False, ret)
665 614
 
666 615
         m_handler._has_pods.assert_called_once_with(endpoints)
667
-        m_handler._is_lbaas_spec_in_sync.assert_called_once_with(
616
+        m_handler._svc_handler_annotations_updated.assert_called_once_with(
668 617
             endpoints, lbaas_spec)
669 618
 
670
-    def test_is_lbaas_spec_in_sync(self):
671
-        names = ['a', 'b', 'c']
672
-        endpoints = {'subsets': [{'ports': [{'name': n, 'port': 1}
673
-                     for n in names]}]}
674
-        lbaas_spec = obj_lbaas.LBaaSServiceSpec(ports=[
675
-            obj_lbaas.LBaaSPortSpec(name=n, targetPort=1)
676
-            for n in reversed(names)])
677
-
678
-        m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
679
-        ret = h_lbaas.LoadBalancerHandler._is_lbaas_spec_in_sync(
680
-            m_handler, endpoints, lbaas_spec)
681
-
682
-        self.assertEqual(True, ret)
683
-
684 619
     def test_has_pods(self):
685 620
         # REVISIT(ivc): ddt?
686 621
         endpoints = {'subsets': [

+ 1
- 1
kuryr_kubernetes/tests/unit/test_object.py View File

@@ -28,7 +28,7 @@ object_data = {
28 28
     'LBaaSLoadBalancer': '1.3-8bc0a9bdbd160da67572aa38784378d1',
29 29
     'LBaaSMember': '1.0-a770c6884c27d6d8c21186b27d0e2ccb',
30 30
     'LBaaSPool': '1.1-6e77370d7632a902445444249eb77b01',
31
-    'LBaaSPortSpec': '1.1-fcfa2fd07f4bc5619b96fa41bcdf6e23',
31
+    'LBaaSPortSpec': '1.1-1b307f34630617086c7af70f2cb8b215',
32 32
     'LBaaSPubIp': '1.0-83992edec2c60fb4ab8998ea42a4ff74',
33 33
     'LBaaSRouteNotifEntry': '1.0-dd2f2be956f68814b1f47cb13483a885',
34 34
     'LBaaSRouteNotifier': '1.0-f0bfd8e772434abe7557930d7e0180c1',

+ 51
- 0
kuryr_kubernetes/tests/unit/test_utils.py View File

@@ -18,6 +18,7 @@ from oslo_config import cfg
18 18
 
19 19
 from kuryr_kubernetes import constants as k_const
20 20
 from kuryr_kubernetes import exceptions as k_exc
21
+from kuryr_kubernetes.objects import lbaas as obj_lbaas
21 22
 from kuryr_kubernetes.objects import vif
22 23
 from kuryr_kubernetes.tests import base as test_base
23 24
 from kuryr_kubernetes.tests.unit import kuryr_fixtures as k_fix
@@ -164,3 +165,53 @@ class TestUtils(test_base.TestCase):
164 165
         ret = utils.get_endpoints_link(service)
165 166
         expected_link = "/api/v1/namespaces/default/endpoints/test"
166 167
         self.assertEqual(expected_link, ret)
168
+
169
+    def test_get_service_ports(self):
170
+        service = {'spec': {'ports': [
171
+            {'port': 1, 'targetPort': 1},
172
+            {'port': 2, 'name': 'X', 'protocol': 'UDP', 'targetPort': 2}
173
+        ]}}
174
+        expected_ret = [
175
+            {'port': 1, 'name': None, 'protocol': 'TCP', 'targetPort': '1'},
176
+            {'port': 2, 'name': 'X', 'protocol': 'UDP', 'targetPort': '2'}]
177
+
178
+        ret = utils.get_service_ports(service)
179
+        self.assertEqual(expected_ret, ret)
180
+
181
+    @mock.patch('kuryr_kubernetes.utils.get_service_ports')
182
+    def test_has_port_changes(self, m_get_service_ports):
183
+        service = mock.MagicMock()
184
+        m_get_service_ports.return_value = [
185
+            {'port': 1, 'name': 'X', 'protocol': 'TCP', 'targetPort': 1},
186
+        ]
187
+
188
+        lbaas_spec = mock.MagicMock()
189
+        lbaas_spec.ports = [
190
+            obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1,
191
+                                    targetPort=1),
192
+            obj_lbaas.LBaaSPortSpec(name='Y', protocol='TCP', port=2,
193
+                                    targetPort=2),
194
+        ]
195
+
196
+        ret = utils.has_port_changes(service, lbaas_spec)
197
+        self.assertTrue(ret)
198
+
199
+    @mock.patch('kuryr_kubernetes.utils.get_service_ports')
200
+    def test_has_port_changes__no_changes(self, m_get_service_ports):
201
+        service = mock.MagicMock()
202
+        m_get_service_ports.return_value = [
203
+            {'port': 1, 'name': 'X', 'protocol': 'TCP', 'targetPort': '1'},
204
+            {'port': 2, 'name': 'Y', 'protocol': 'TCP', 'targetPort': '2'}
205
+        ]
206
+
207
+        lbaas_spec = mock.MagicMock()
208
+        lbaas_spec.ports = [
209
+            obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1,
210
+                                    targetPort=1),
211
+            obj_lbaas.LBaaSPortSpec(name='Y', protocol='TCP', port=2,
212
+                                    targetPort=2),
213
+        ]
214
+
215
+        ret = utils.has_port_changes(service, lbaas_spec)
216
+
217
+        self.assertFalse(ret)

+ 28
- 0
kuryr_kubernetes/utils.py View File

@@ -263,3 +263,31 @@ def get_endpoints_link(service):
263 263
     link_parts[-2] = 'endpoints'
264 264
 
265 265
     return "/".join(link_parts)
266
+
267
+
268
+def has_port_changes(service, lbaas_spec):
269
+    link = service['metadata']['selfLink']
270
+
271
+    fields = obj_lbaas.LBaaSPortSpec.fields
272
+    svc_port_set = {tuple(port[attr] for attr in fields)
273
+                    for port in get_service_ports(service)}
274
+
275
+    spec_port_set = {tuple(getattr(port, attr)
276
+                     for attr in fields
277
+                     if port.obj_attr_is_set(attr))
278
+                     for port in lbaas_spec.ports}
279
+
280
+    if svc_port_set != spec_port_set:
281
+        LOG.debug("LBaaS spec ports %(spec_ports)s != %(svc_ports)s "
282
+                  "for %(link)s" % {'spec_ports': spec_port_set,
283
+                                    'svc_ports': svc_port_set,
284
+                                    'link': link})
285
+    return svc_port_set != spec_port_set
286
+
287
+
288
+def get_service_ports(service):
289
+    return [{'name': port.get('name'),
290
+             'protocol': port.get('protocol', 'TCP'),
291
+             'port': port['port'],
292
+             'targetPort': str(port['targetPort'])}
293
+            for port in service['spec']['ports']]

Loading…
Cancel
Save