From 552cc28f7fcda35b181fdf6d4c77960bab35d92e Mon Sep 17 00:00:00 2001
From: Tobias Henkel <tobias.henkel@bmw.de>
Date: Fri, 12 Oct 2018 14:00:52 +0200
Subject: [PATCH] Use quota handling code for min-ready declines

We currently check the current pool nodes against max_servers in order
to decline min-ready request when we're at quota. This is older than
the quota handling where max-servers was the only limiting thing. Now
we have real quota handling we should defer this min-ready check to
the point where we know that we're at quota (independent of self
defined pool quota or real cloud quota).

Change-Id: Ic36ecfaf441436db6699297ef459c586f14c2dfc
---
 nodepool/driver/__init__.py | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/nodepool/driver/__init__.py b/nodepool/driver/__init__.py
index 635d695b0..ba0281b16 100644
--- a/nodepool/driver/__init__.py
+++ b/nodepool/driver/__init__.py
@@ -413,7 +413,10 @@ class NodeRequestHandler(NodeRequestHandlerNotifications,
         diff = requested_types - saved_types
         needed_types = list(diff.elements())
 
-        ready_nodes = self.zk.getReadyNodesOfTypes(needed_types)
+        if self.request.reuse:
+            ready_nodes = self.zk.getReadyNodesOfTypes(needed_types)
+        else:
+            ready_nodes = []
 
         for ntype in needed_types:
             # First try to grab from the list of already available nodes.
@@ -455,6 +458,20 @@ class NodeRequestHandler(NodeRequestHandlerNotifications,
                 # If we calculate that we're at capacity, pause until nodes
                 # are released by Zuul and removed by the DeletedNodeWorker.
                 if not self.hasRemainingQuota(ntype):
+
+                    if self.request.requestor == "NodePool:min-ready":
+                        # The point of the min-ready nodes is to have nodes on
+                        # standby for future requests. When at capacity, it
+                        # doesn't make sense to wait for and use resources to
+                        # speculatively create a node. Decline this so someone
+                        # else with capacity can take it.
+                        self.log.debug(
+                            "Declining node request %s because provider cannot"
+                            " satisfy min-ready")
+                        self.decline_request()
+                        self._declinedHandlerCleanup()
+                        return
+
                     self.log.info(
                         "Not enough quota remaining to satisfy request %s",
                         self.request.id)
@@ -531,19 +548,6 @@ class NodeRequestHandler(NodeRequestHandlerNotifications,
         # This way we could give another (free) provider the chance to take
         # this request earlier.
 
-        # For min-ready requests, which do not re-use READY nodes, let's
-        # decline if this provider is already at capacity. Otherwise, we
-        # could end up wedged until another request frees up a node.
-        if self.pool.max_servers is not None and \
-           self.request.requestor == "NodePool:min-ready":
-            current_count = self.zk.countPoolNodes(self.provider.name,
-                                                   self.pool.name)
-            # Use >= because dynamic config changes to max-servers can leave
-            # us with more than max-servers.
-            # TODO: handle this with the quota code
-            if current_count >= self.pool.max_servers:
-                declined_reasons.append("provider cannot satisfy min-ready")
-
         if declined_reasons:
             self.log.debug("Declining node request %s because %s",
                            self.request.id, ', '.join(declined_reasons))