Fixes to drone_manager behavior.

Fixed bug where a drone that a user is not allowed to access may get a
job scheduled on it anyways if all drones are over capacity.

Modified capacity computation to handle drones with the same ratio of
running-to-max processes, but different total max processes.

Signed-off-by: James Ren <jamesren@google.com>


git-svn-id: http://test.kernel.org/svn/autotest/trunk@4345 592f7852-d20e-0410-864c-8624ca9c26a4
diff --git a/scheduler/drone_manager.py b/scheduler/drone_manager.py
index 65551be..b2dace4 100644
--- a/scheduler/drone_manager.py
+++ b/scheduler/drone_manager.py
@@ -423,8 +423,10 @@
         if not usable_drone_wrappers:
             # all drones disabled or inaccessible
             return 0
-        return max(wrapper.drone.max_processes - wrapper.drone.active_processes
-                   for wrapper in usable_drone_wrappers)
+        runnable_processes = [
+                wrapper.drone.max_processes - wrapper.drone.active_processes
+                for wrapper in usable_drone_wrappers]
+        return max([0] + runnable_processes)
 
 
     def _least_loaded_drone(self, drones):
@@ -439,12 +441,14 @@
         # cycle through drones is order of increasing used capacity until
         # we find one that can handle these processes
         checked_drones = []
+        usable_drones = []
         drone_to_use = None
         while self._drone_queue:
             drone = heapq.heappop(self._drone_queue).drone
             checked_drones.append(drone)
             if not drone.usable_by(username):
                 continue
+            usable_drones.append(drone)
             if drone.active_processes + num_processes <= drone.max_processes:
                 drone_to_use = drone
                 break
@@ -453,10 +457,10 @@
             drone_summary = ','.join('%s %s/%s' % (drone.hostname,
                                                    drone.active_processes,
                                                    drone.max_processes)
-                                     for drone in checked_drones)
-            logging.error('No drone has capacity to handle %d processes (%s)',
-                          num_processes, drone_summary)
-            drone_to_use = self._least_loaded_drone(checked_drones)
+                                     for drone in usable_drones)
+            logging.error('No drone has capacity to handle %d processes (%s) '
+                          'for user %s', num_processes, drone_summary, username)
+            drone_to_use = self._least_loaded_drone(usable_drones)
 
         # refill _drone_queue
         for drone in checked_drones:
diff --git a/scheduler/drone_manager_unittest.py b/scheduler/drone_manager_unittest.py
index 161b5c5..bd078c1 100755
--- a/scheduler/drone_manager_unittest.py
+++ b/scheduler/drone_manager_unittest.py
@@ -121,6 +121,12 @@
         self.assertEquals(drone.name, 1)
 
 
+    def test_choose_drone_for_execution_all_full_same_percentage_capacity(self):
+        drone = self._test_choose_drone_for_execution_helper([(5, 3), (10, 6)],
+                                                             1)
+        self.assertEquals(drone.name, 1)
+
+
     def test_user_restrictions(self):
         # this drone is restricted to a different user
         self.manager._enqueue_drone(MockDrone(1, max_processes=10,
@@ -136,6 +142,22 @@
         self.assertEquals(drone.name, 2)
 
 
+    def test_user_restrictions_with_full_drone(self):
+        # this drone is restricted to a different user
+        self.manager._enqueue_drone(MockDrone(1, max_processes=10,
+                                              allowed_users=['fakeuser']))
+        # this drone is allowed but is full
+        self.manager._enqueue_drone(MockDrone(2, active_processes=3,
+                                              max_processes=2,
+                                              allowed_users=[self._USERNAME]))
+
+        self.assertEquals(0,
+                          self.manager.max_runnable_processes(self._USERNAME))
+        drone = self.manager._choose_drone_for_execution(
+                1, username=self._USERNAME)
+        self.assertEquals(drone.name, 2)
+
+
     def test_initialize(self):
         results_hostname = 'results_repo'
         results_install_dir = '/results/install'
diff --git a/scheduler/drones.py b/scheduler/drones.py
index b32f7e5..85a5ee2 100644
--- a/scheduler/drones.py
+++ b/scheduler/drones.py
@@ -27,9 +27,20 @@
 
 
     def used_capacity(self):
+        """Gets the capacity used by this drone
+
+        Returns a tuple of (percentage_full, -max_capacity). This is to aid
+        direct comparisons, so that a 0/10 drone is considered less heavily
+        loaded than a 0/2 drone.
+
+        This value should never be used directly. It should only be used in
+        direct comparisons using the basic comparison operators, or using the
+        cmp() function.
+        """
         if self.max_processes == 0:
-            return 1.0
-        return float(self.active_processes) / self.max_processes
+            return (1.0, 0)
+        return (float(self.active_processes) / self.max_processes,
+                -self.max_processes)
 
 
     def usable_by(self, user):