Merge "Add port-cleanup-interval config option"

doc/source/configuration.rst

@@ -577,6 +577,15 @@ Selecting the OpenStack driver adds the following options to the
      the OpenStack project and will attempt to clean unattached
      floating ips that may have leaked around restarts.
 
+  .. attr:: port-cleanup-interval
+     :type: int seconds
+     :default: 600
+
+     If greater than 0, nodepool will assume it is the only user of the
+     OpenStack project and will attempt to clean ports in `DOWN` state after
+     `cleanup-interval` has passed. This value can be reduced if the instance
+     spawn time on the provider is reliably quicker.
+
   .. attr:: diskimages
      :type: list
 

nodepool/driver/openstack/config.py

@@ -230,6 +230,7 @@ class OpenStackProviderConfig(ProviderConfig):
         self.boot_timeout = None
         self.launch_timeout = None
         self.clean_floating_ips = None
+        self.port_cleanup_interval = None
         self.diskimages = {}
         self.cloud_images = {}
         self.hostname_format = None
@@ -246,6 +247,8 @@ class OpenStackProviderConfig(ProviderConfig):
                     other.boot_timeout == self.boot_timeout and
                     other.launch_timeout == self.launch_timeout and
                     other.clean_floating_ips == self.clean_floating_ips and
+                    other.port_cleanup_interval ==
+                    self.port_cleanup_interval and
                     other.diskimages == self.diskimages and
                     other.cloud_images == self.cloud_images)
         return False
@@ -277,6 +280,10 @@ class OpenStackProviderConfig(ProviderConfig):
         self.launch_timeout = self.provider.get('launch-timeout', 3600)
         self.launch_retries = self.provider.get('launch-retries', 3)
         self.clean_floating_ips = self.provider.get('clean-floating-ips')
+        self.port_cleanup_interval = self.provider.get(
+            'port-cleanup-interval',
+            600
+        )
         self.hostname_format = self.provider.get(
             'hostname-format',
             '{label.name}-{provider.name}-{node.id}'
@@ -414,6 +421,7 @@ class OpenStackProviderConfig(ProviderConfig):
             'hostname-format': str,
             'image-name-format': str,
             'clean-floating-ips': bool,
+            'port-cleanup-interval': int,
             'pools': [pool],
             'diskimages': [provider_diskimage],
             'cloud-images': [provider_cloud_images],

nodepool/driver/openstack/provider.py

@@ -51,10 +51,6 @@ class OpenStackProvider(Provider):
         self._zk = None
         self._down_ports = set()
         self._last_port_cleanup = None
-        # Set this long enough to avoid deleting a port which simply
-        # hasn't yet been attached to an instance which is being
-        # created.
-        self._port_cleanup_interval_secs = 600
         self._statsd = stats.get_client()
 
     def start(self, zk_conn):
@@ -554,7 +550,7 @@ class OpenStackProvider(Provider):
 
         # Return if not enough time has passed between cleanup
         last_check_in_secs = int(time.monotonic() - self._last_port_cleanup)
-        if last_check_in_secs <= self._port_cleanup_interval_secs:
+        if last_check_in_secs <= self.provider.port_cleanup_interval:
             return
 
         ports = self.listPorts(status='DOWN')
@@ -588,7 +584,8 @@ class OpenStackProvider(Provider):
 
     def cleanupLeakedResources(self):
         self.cleanupLeakedInstances()
-        self.cleanupLeakedPorts()
+        if self.provider.port_cleanup_interval:
+            self.cleanupLeakedPorts()
         if self.provider.clean_floating_ips:
             self._client.delete_unattached_floating_ips()
 

nodepool/tests/fixtures/cleanup-port.yaml

@@ -0,0 +1,54 @@
+elements-dir: .
+images-dir: '{images_dir}'
+build-log-dir: '{build_log_dir}'
+build-log-retention: 1
+
+zookeeper-servers:
+  - host: {zookeeper_host}
+    port: {zookeeper_port}
+    chroot: {zookeeper_chroot}
+
+labels:
+  - name: fake-label
+    min-ready: 1
+
+providers:
+  - name: fake-provider
+    cloud: fake
+    driver: fake
+    region-name: fake-region
+    rate: 0.0001
+    port-cleanup-interval: 2
+    diskimages:
+      - name: fake-image
+        meta:
+          key: value
+          key2: value
+    pools:
+      - name: main
+        max-servers: 96
+        node-attributes:
+          key1: value1
+          key2: value2
+        availability-zones:
+          - az1
+        networks:
+          - net-name
+        labels:
+          - name: fake-label
+            diskimage: fake-image
+            min-ram: 8192
+            flavor-name: 'Fake'
+
+diskimages:
+  - name: fake-image
+    elements:
+      - fedora
+      - vm
+    release: 21
+    dib-cmd: nodepool/tests/fake-image-create
+    env-vars:
+      TMPDIR: /opt/dib_tmp
+      DIB_IMAGE_CACHE: /opt/dib_cache
+      DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
+      BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2

nodepool/tests/fixtures/config_validate/good.yaml

@@ -33,6 +33,7 @@ providers:
     boot-timeout: 120
     max-concurrency: 10
     launch-retries: 3
+    port-cleanup-interval: 600
     rate: 1
     diskimages:
       - name: trusty
@@ -75,6 +76,7 @@ providers:
     region-name: 'chocolate'
     boot-timeout: 120
     rate: 0.001
+    port-cleanup-interval: 0
     diskimages:
       - name: trusty
         pause: False

nodepool/tests/unit/test_launcher.py

@@ -2109,10 +2109,29 @@ class TestLauncher(tests.DBTestCase):
         self.assertEqual(2, len(down_ports))
         self.log.debug("Down ports: %s", down_ports)
 
-        # Change the port cleanup interval to happen quicker
-        manager._port_cleanup_interval_secs = 2
-        while manager.listPorts(status='DOWN'):
-            time.sleep(1)
+        # Second config decreases cleanup interval to 2 seconds
+        self.replace_config(configfile, 'cleanup-port.yaml')
+        oldmanager = manager
+        manager = pool.getProviderManager('fake-provider')
+        for _ in iterate_timeout(10, Exception, 'assert config updated'):
+            try:
+                self.assertNotEqual(manager, oldmanager)
+                break
+            except AssertionError:
+                # config still hasn't updated, retry
+                manager = pool.getProviderManager('fake-provider')
+        # Reset the client as a new fake client will have been
+        # created.
+        manager.resetClient()
+
+        for _ in iterate_timeout(4, Exception, 'assert ports are cleaned'):
+            try:
+                down_ports = manager.listPorts(status='DOWN')
+                self.assertEqual(0, len(down_ports))
+                break
+            except AssertionError:
+                # ports not cleaned up yet, retry
+                pass
 
         self.assertReportedStat('nodepool.provider.fake-provider.downPorts',
                                 value='2', kind='c')