summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTobias Henkel <tobias.henkel@bmw.de>2018-10-30 12:58:40 +0100
committerTobias Henkel <tobias.henkel@bmw.de>2018-10-30 13:13:43 +0100
commit7e1b8a7261d31208d853ecfa255da648ba54b656 (patch)
tree32d3dc59f32498b51b31d0f7854af67ab1b8d82c
parent5c87fdb0469402cd93562b216e41df17809e3c2a (diff)
Revert "Cleanup down ports"3.3.1
The port filter for DOWN port seems to have no effect. It actually deleted *all* ports in the tenant. This reverts commit cdd60504ecdfb97a3f85dffdb7ad18aedbf2021d. Change-Id: I48c1430bb768903af467cace1a720e45ecc8e98f
Notes
Notes (review): Code-Review+2: David Shrewsbury <shrewsbury.dave@gmail.com> Code-Review+2: Monty Taylor <mordred@inaugust.com> Workflow+1: Monty Taylor <mordred@inaugust.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Tue, 30 Oct 2018 14:04:59 +0000 Reviewed-on: https://review.openstack.org/614198 Project: openstack-infra/nodepool Branch: refs/heads/master
-rw-r--r--doc/source/operation.rst6
-rw-r--r--nodepool/driver/fake/provider.py21
-rwxr-xr-xnodepool/driver/openstack/provider.py79
-rw-r--r--nodepool/tests/test_launcher.py21
-rw-r--r--releasenotes/notes/port-cleanup-667d476437f03358.yaml7
5 files changed, 1 insertions, 133 deletions
diff --git a/doc/source/operation.rst b/doc/source/operation.rst
index 470e9f9..80d96cf 100644
--- a/doc/source/operation.rst
+++ b/doc/source/operation.rst
@@ -329,12 +329,6 @@ Nodepool launcher
329 * ready 329 * ready
330 * used 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.. zuul:stat:: nodepool.provider.<provider>.nodes.<state> 332.. zuul:stat:: nodepool.provider.<provider>.nodes.<state>
339 :type: gauge 333 :type: gauge
340 334
diff --git a/nodepool/driver/fake/provider.py b/nodepool/driver/fake/provider.py
index ed83be6..6e6fdcd 100644
--- a/nodepool/driver/fake/provider.py
+++ b/nodepool/driver/fake/provider.py
@@ -31,7 +31,6 @@ class Dummy(object):
31 INSTANCE = 'Instance' 31 INSTANCE = 'Instance'
32 FLAVOR = 'Flavor' 32 FLAVOR = 'Flavor'
33 LOCATION = 'Server.Location' 33 LOCATION = 'Server.Location'
34 PORT = 'Port'
35 34
36 def __init__(self, kind, **kw): 35 def __init__(self, kind, **kw):
37 self.__kind = kind 36 self.__kind = kind
@@ -105,12 +104,6 @@ class FakeOpenStackCloud(object):
105 self._server_list = [] 104 self._server_list = []
106 self.max_cores, self.max_instances, self.max_ram = FakeOpenStackCloud.\ 105 self.max_cores, self.max_instances, self.max_ram = FakeOpenStackCloud.\
107 _get_quota() 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 def _get(self, name_or_id, instance_list): 108 def _get(self, name_or_id, instance_list):
116 self.log.debug("Get %s in %s" % (name_or_id, repr(instance_list))) 109 self.log.debug("Get %s in %s" % (name_or_id, repr(instance_list)))
@@ -288,20 +281,6 @@ class FakeOpenStackCloud(object):
288 total_ram_used=8192 * len(self._server_list) 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
306class FakeUploadFailCloud(FakeOpenStackCloud): 285class FakeUploadFailCloud(FakeOpenStackCloud):
307 log = logging.getLogger("nodepool.FakeUploadFailCloud") 286 log = logging.getLogger("nodepool.FakeUploadFailCloud")
diff --git a/nodepool/driver/openstack/provider.py b/nodepool/driver/openstack/provider.py
index de479fc..d202f9f 100755
--- a/nodepool/driver/openstack/provider.py
+++ b/nodepool/driver/openstack/provider.py
@@ -26,7 +26,6 @@ from nodepool.driver import Provider
26from nodepool.driver.utils import QuotaInformation 26from nodepool.driver.utils import QuotaInformation
27from nodepool.nodeutils import iterate_timeout 27from nodepool.nodeutils import iterate_timeout
28from nodepool.task_manager import TaskManager 28from nodepool.task_manager import TaskManager
29from nodepool import stats
30from nodepool import version 29from nodepool import version
31from nodepool import zk 30from nodepool import zk
32 31
@@ -51,10 +50,6 @@ class OpenStackProvider(Provider):
51 self._taskmanager = None 50 self._taskmanager = None
52 self._current_nodepool_quota = None 51 self._current_nodepool_quota = None
53 self._zk = None 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 def start(self, zk_conn): 54 def start(self, zk_conn):
60 if self._use_taskmanager: 55 if self._use_taskmanager:
@@ -422,21 +417,6 @@ class OpenStackProvider(Provider):
422 **meta) 417 **meta)
423 return image.id 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 def listImages(self): 420 def listImages(self):
441 return self._client.list_images() 421 return self._client.list_images()
442 422
@@ -464,7 +444,7 @@ class OpenStackProvider(Provider):
464 self.log.debug('Deleting server %s' % server_id) 444 self.log.debug('Deleting server %s' % server_id)
465 self.deleteServer(server_id) 445 self.deleteServer(server_id)
466 446
467 def cleanupLeakedInstances(self): 447 def cleanupLeakedResources(self):
468 ''' 448 '''
469 Delete any leaked server instances. 449 Delete any leaked server instances.
470 450
@@ -512,63 +492,6 @@ class OpenStackProvider(Provider):
512 node.state = zk.DELETING 492 node.state = zk.DELETING
513 self._zk.storeNode(node) 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 if self.provider.clean_floating_ips: 495 if self.provider.clean_floating_ips:
573 self._client.delete_unattached_floating_ips() 496 self._client.delete_unattached_floating_ips()
574 497
diff --git a/nodepool/tests/test_launcher.py b/nodepool/tests/test_launcher.py
index 1406f48..27c3ec3 100644
--- a/nodepool/tests/test_launcher.py
+++ b/nodepool/tests/test_launcher.py
@@ -1773,24 +1773,3 @@ class TestLauncher(tests.DBTestCase):
1773 1773
1774 req3 = self.waitForNodeRequest(req3) 1774 req3 = self.waitForNodeRequest(req3)
1775 self.assertEqual(req3.state, zk.FAILED) 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')
diff --git a/releasenotes/notes/port-cleanup-667d476437f03358.yaml b/releasenotes/notes/port-cleanup-667d476437f03358.yaml
deleted file mode 100644
index e89e375..0000000
--- a/releasenotes/notes/port-cleanup-667d476437f03358.yaml
+++ /dev/null
@@ -1,7 +0,0 @@
1---
2features:
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.