Browse Source

Merge "Add support for svc with text targetPorts"

Zuul 1 month ago
parent
commit
76212810d3

+ 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):
@@ -257,24 +231,25 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
257 231
                 lbaas_state.service_pub_ip_info)
258 232
 
259 233
     def _should_ignore(self, endpoints, lbaas_spec):
234
+        # NOTE(ltomasbo): we must wait until service handler has annotated the
235
+        # endpoints to process them. Thus, if annotations are not updated to
236
+        # match the endpoints information, we should skip the event
260 237
         return not(lbaas_spec and
261 238
                    self._has_pods(endpoints) and
262
-                   self._is_lbaas_spec_in_sync(endpoints, lbaas_spec))
263
-
264
-    def _is_lbaas_spec_in_sync(self, endpoints, lbaas_spec):
265
-        ports = lbaas_spec.ports
266
-        ep_ports = list(set((port.get('name'), port.get('port'))
267
-                            if ports[0].obj_attr_is_set('targetPort')
268
-                            else port.get('name')
269
-                            for subset in endpoints.get('subsets', [])
270
-                            for port in subset.get('ports', [])))
271
-
272
-        spec_ports = [(port.name, port.targetPort)
273
-                      if port.obj_attr_is_set('targetPort')
274
-                      else port.name
275
-                      for port in ports]
276
-
277
-        return sorted(ep_ports) == sorted(spec_ports)
239
+                   self._svc_handler_annotations_updated(endpoints,
240
+                                                         lbaas_spec))
241
+
242
+    def _svc_handler_annotations_updated(self, endpoints, lbaas_spec):
243
+        svc_link = self._get_service_link(endpoints)
244
+        k8s = clients.get_kubernetes_client()
245
+        service = k8s.get(svc_link)
246
+        if utils.has_port_changes(service, lbaas_spec):
247
+            # NOTE(ltomasbo): Ensuring lbaas_spec annotated on the endpoints
248
+            # is in sync with the service status, i.e., upon a service
249
+            # modification it will ensure endpoint modifications are not
250
+            # handled until the service handler has performed its annotations
251
+            return False
252
+        return True
278 253
 
279 254
     def _has_pods(self, endpoints):
280 255
         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):
@@ -637,30 +586,16 @@ class TestLoadBalancerHandler(test_base.TestCase):
637 586
         # REVISIT(ivc): ddt?
638 587
         m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
639 588
         m_handler._has_pods.return_value = True
640
-        m_handler._is_lbaas_spec_in_sync.return_value = True
589
+        m_handler._svc_handler_annotations_updated.return_value = True
641 590
 
642 591
         ret = h_lbaas.LoadBalancerHandler._should_ignore(
643 592
             m_handler, endpoints, lbaas_spec)
644 593
         self.assertEqual(False, ret)
645 594
 
646 595
         m_handler._has_pods.assert_called_once_with(endpoints)
647
-        m_handler._is_lbaas_spec_in_sync.assert_called_once_with(
596
+        m_handler._svc_handler_annotations_updated.assert_called_once_with(
648 597
             endpoints, lbaas_spec)
649 598
 
650
-    def test_is_lbaas_spec_in_sync(self):
651
-        names = ['a', 'b', 'c']
652
-        endpoints = {'subsets': [{'ports': [{'name': n, 'port': 1}
653
-                     for n in names]}]}
654
-        lbaas_spec = obj_lbaas.LBaaSServiceSpec(ports=[
655
-            obj_lbaas.LBaaSPortSpec(name=n, targetPort=1)
656
-            for n in reversed(names)])
657
-
658
-        m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
659
-        ret = h_lbaas.LoadBalancerHandler._is_lbaas_spec_in_sync(
660
-            m_handler, endpoints, lbaas_spec)
661
-
662
-        self.assertEqual(True, ret)
663
-
664 599
     def test_has_pods(self):
665 600
         # REVISIT(ivc): ddt?
666 601
         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