Browse Source

Revert "Cleanup down ports"

The port filter for DOWN port seems to have no effect. It actually
deleted *all* ports in the tenant.

This reverts commit cdd60504ec.

Change-Id: I48c1430bb768903af467cace1a720e45ecc8e98f
Tobias Henkel 5 months ago
parent
commit
7e1b8a7261
No account linked to committer's email address

+ 0
- 6
doc/source/operation.rst View File

@@ -329,12 +329,6 @@ Nodepool launcher
329 329
    * ready
330 330
    * used
331 331
 
332
-.. zuul:stat:: nodepool.provider.<provider>.downPorts
333
-   :type: counter
334
-
335
-   Number of ports in the DOWN state that have been removed automatically
336
-   in the cleanup resources phase of the OpenStack driver.
337
-
338 332
 .. zuul:stat:: nodepool.provider.<provider>.nodes.<state>
339 333
    :type: gauge
340 334
 

+ 0
- 21
nodepool/driver/fake/provider.py View File

@@ -31,7 +31,6 @@ class Dummy(object):
31 31
     INSTANCE = 'Instance'
32 32
     FLAVOR = 'Flavor'
33 33
     LOCATION = 'Server.Location'
34
-    PORT = 'Port'
35 34
 
36 35
     def __init__(self, kind, **kw):
37 36
         self.__kind = kind
@@ -105,12 +104,6 @@ class FakeOpenStackCloud(object):
105 104
         self._server_list = []
106 105
         self.max_cores, self.max_instances, self.max_ram = FakeOpenStackCloud.\
107 106
             _get_quota()
108
-        self._down_ports = [
109
-            Dummy(Dummy.PORT, id='1a', status='DOWN',
110
-                  device_owner="compute:nova"),
111
-            Dummy(Dummy.PORT, id='2b', status='DOWN',
112
-                  device_owner=None),
113
-        ]
114 107
 
115 108
     def _get(self, name_or_id, instance_list):
116 109
         self.log.debug("Get %s in %s" % (name_or_id, repr(instance_list)))
@@ -288,20 +281,6 @@ class FakeOpenStackCloud(object):
288 281
             total_ram_used=8192 * len(self._server_list)
289 282
         )
290 283
 
291
-    def list_ports(self, filters=None):
292
-        if filters and filters.get('status') == 'DOWN':
293
-            return self._down_ports
294
-        return []
295
-
296
-    def delete_port(self, port_id):
297
-        tmp_ports = []
298
-        for port in self._down_ports:
299
-            if port.id != port_id:
300
-                tmp_ports.append(port)
301
-            else:
302
-                self.log.debug("Deleted port ID: %s", port_id)
303
-        self._down_ports = tmp_ports
304
-
305 284
 
306 285
 class FakeUploadFailCloud(FakeOpenStackCloud):
307 286
     log = logging.getLogger("nodepool.FakeUploadFailCloud")

+ 1
- 78
nodepool/driver/openstack/provider.py View File

@@ -26,7 +26,6 @@ from nodepool.driver import Provider
26 26
 from nodepool.driver.utils import QuotaInformation
27 27
 from nodepool.nodeutils import iterate_timeout
28 28
 from nodepool.task_manager import TaskManager
29
-from nodepool import stats
30 29
 from nodepool import version
31 30
 from nodepool import zk
32 31
 
@@ -51,10 +50,6 @@ class OpenStackProvider(Provider):
51 50
         self._taskmanager = None
52 51
         self._current_nodepool_quota = None
53 52
         self._zk = None
54
-        self._down_ports = set()
55
-        self._last_port_cleanup = None
56
-        self._port_cleanup_interval_secs = 180
57
-        self._statsd = stats.get_client()
58 53
 
59 54
     def start(self, zk_conn):
60 55
         if self._use_taskmanager:
@@ -422,21 +417,6 @@ class OpenStackProvider(Provider):
422 417
             **meta)
423 418
         return image.id
424 419
 
425
-    def listPorts(self, status=None):
426
-        '''
427
-        List known ports.
428
-
429
-        :param str status: A valid port status. E.g., 'ACTIVE' or 'DOWN'.
430
-        '''
431
-        if status:
432
-            ports = self._client.list_ports(filters={'status': status})
433
-        else:
434
-            ports = self._client.list_ports()
435
-        return ports
436
-
437
-    def deletePort(self, port_id):
438
-        self._client.delete_port(port_id)
439
-
440 420
     def listImages(self):
441 421
         return self._client.list_images()
442 422
 
@@ -464,7 +444,7 @@ class OpenStackProvider(Provider):
464 444
         self.log.debug('Deleting server %s' % server_id)
465 445
         self.deleteServer(server_id)
466 446
 
467
-    def cleanupLeakedInstances(self):
447
+    def cleanupLeakedResources(self):
468 448
         '''
469 449
         Delete any leaked server instances.
470 450
 
@@ -512,63 +492,6 @@ class OpenStackProvider(Provider):
512 492
                 node.state = zk.DELETING
513 493
                 self._zk.storeNode(node)
514 494
 
515
-    def filterComputePorts(self, ports):
516
-        '''
517
-        Return a list of compute ports (or no device owner).
518
-
519
-        We are not interested in ports for routers or DHCP.
520
-        '''
521
-        ret = []
522
-        for p in ports:
523
-            if p.device_owner is None or p.device_owner.startswith("compute:"):
524
-                ret.append(p)
525
-        return ret
526
-
527
-    def cleanupLeakedPorts(self):
528
-        if not self._last_port_cleanup:
529
-            self._last_port_cleanup = time.monotonic()
530
-            ports = self.listPorts(status='DOWN')
531
-            ports = self.filterComputePorts(ports)
532
-            self._down_ports = set([p.id for p in ports])
533
-            return
534
-
535
-        # Return if not enough time has passed between cleanup
536
-        last_check_in_secs = int(time.monotonic() - self._last_port_cleanup)
537
-        if last_check_in_secs <= self._port_cleanup_interval_secs:
538
-            return
539
-
540
-        ports = self.listPorts(status='DOWN')
541
-        ports = self.filterComputePorts(ports)
542
-        current_set = set([p.id for p in ports])
543
-        remove_set = current_set & self._down_ports
544
-
545
-        removed_count = 0
546
-        for port_id in remove_set:
547
-            try:
548
-                self.deletePort(port_id)
549
-            except Exception:
550
-                self.log.exception("Exception deleting port %s in %s:",
551
-                                   port_id, self.provider.name)
552
-            else:
553
-                removed_count += 1
554
-                self.log.debug("Removed DOWN port %s in %s",
555
-                               port_id, self.provider.name)
556
-
557
-        if self._statsd and removed_count:
558
-            key = 'nodepool.provider.%s.downPorts' % (self.provider.name)
559
-            self._statsd.incr(key, removed_count)
560
-
561
-        self._last_port_cleanup = time.monotonic()
562
-
563
-        # Rely on OpenStack to tell us the down ports rather than doing our
564
-        # own set adjustment.
565
-        ports = self.listPorts(status='DOWN')
566
-        ports = self.filterComputePorts(ports)
567
-        self._down_ports = set([p.id for p in ports])
568
-
569
-    def cleanupLeakedResources(self):
570
-        self.cleanupLeakedInstances()
571
-        self.cleanupLeakedPorts()
572 495
         if self.provider.clean_floating_ips:
573 496
             self._client.delete_unattached_floating_ips()
574 497
 

+ 0
- 21
nodepool/tests/test_launcher.py View File

@@ -1773,24 +1773,3 @@ class TestLauncher(tests.DBTestCase):
1773 1773
 
1774 1774
         req3 = self.waitForNodeRequest(req3)
1775 1775
         self.assertEqual(req3.state, zk.FAILED)
1776
-
1777
-    def test_leaked_port_cleanup(self):
1778
-        configfile = self.setup_config('node.yaml')
1779
-        self.useBuilder(configfile)
1780
-        pool = self.useNodepool(configfile, watermark_sleep=1)
1781
-        pool.cleanup_interval = 1
1782
-        pool.start()
1783
-        self.waitForNodes('fake-label')
1784
-
1785
-        manager = pool.getProviderManager('fake-provider')
1786
-        down_ports = manager.listPorts(status='DOWN')
1787
-        self.assertEqual(2, len(down_ports))
1788
-        self.log.debug("Down ports: %s", down_ports)
1789
-
1790
-        # Change the port cleanup interval to happen quicker
1791
-        manager._port_cleanup_interval_secs = 2
1792
-        while manager.listPorts(status='DOWN'):
1793
-            time.sleep(1)
1794
-
1795
-        self.assertReportedStat('nodepool.provider.fake-provider.downPorts',
1796
-                                value='2', kind='c')

+ 0
- 7
releasenotes/notes/port-cleanup-667d476437f03358.yaml View File

@@ -1,7 +0,0 @@
1
----
2
-features:
3
-  - |
4
-    Added a new routine to the OpenStack driver cleanup resources phase that
5
-    will remove any ports reported to be in the DOWN state. Ports will have
6
-    to be seen as DOWN for at least three minutes before they will be removed.
7
-    The number of ports removed will be reported to statsd.

Loading…
Cancel
Save