Reuse node ids

Squashed commit of the following:

commit 21b0a49
Author: Mich <fmlheureux@datacratic.com>
Date:   Mon Jun 29 09:36:39 2015 -0400

    Covers count = 0 case + test

commit 1b62eda
Author: Mich <fmlheureux@datacratic.com>
Date:   Mon Jun 29 09:31:50 2015 -0400

    Rm debug print leftover

commit eaba0c2
Author: Mich <fmlheureux@datacratic.com>
Date:   Fri Jun 26 16:29:26 2015 -0400

    Reuse node ids

Author: Mich

starcluster/cluster.py

@@ -1213,20 +1213,29 @@ def create_nodes(self, aliases, image_id=None, instance_type=None,
             log.info(str(resv), extra=dict(__raw__=True))
         return resvs
 
-    def _get_next_node_num(self):
+    @classmethod
+    def get_free_ids_among_nodes(cls, count, nodes):
+        result = []
+        if count == 0:
+            return result
+        ids = [int(re.search('node(\d{3})', n.alias).group(1)) for n in nodes]
+        remaining = count
+        for i in xrange(1, 1000):
+            if i not in ids:
+                result.append(i)
+                remaining -= 1
+            if remaining == 0:
+                break
+        assert len(result) == count
+        return result
+
+    def _get_free_node_nums(self, count):
+        """
+        Returns unused node ids
+        """
         nodes = self._nodes_in_states(['pending', 'running'])
         nodes = filter(lambda x: not x.is_master(), nodes)
-        highest = 0
-        for n in nodes:
-            match = re.search('node(\d{3})', n.alias)
-            try:
-                _possible_highest = match.group(1)
-            except AttributeError:
-                continue
-            highest = max(int(_possible_highest), highest)
-        next = int(highest) + 1
-        log.debug("Highest node number is %d. choosing %d." % (highest, next))
-        return next
+        return self.get_free_ids_among_nodes(count, nodes)
 
     def add_node(self, alias=None, no_create=False, image_id=None,
                  instance_type=None, zone=None, placement_group=None,
@@ -1256,8 +1265,7 @@ def add_nodes(self, num_nodes, aliases=None, image_id=None,
         running_pending = self._nodes_in_states(['pending', 'running'])
         aliases = aliases or []
         if not aliases:
-            next_node_id = self._get_next_node_num()
-            for i in range(next_node_id, next_node_id + num_nodes):
+            for i in self._get_free_node_nums(num_nodes):
                 alias = self._make_alias(i)
                 aliases.append(alias)
         assert len(aliases) == num_nodes
@@ -2098,7 +2106,8 @@ def _recover_duplicate_aliases(self):
                 if node.alias != alias:
                     continue
                 if node.private_ip_address != ip:
-                    new_alias = self._make_alias(self._get_next_node_num())
+                    new_alias = self._make_alias(
+                        self._get_free_node_nums(1)[0])
                     log.info("Renaming {} from {} to {}"
                              .format(node, node.alias, new_alias))
                     node.rename(new_alias)

starcluster/tests/test_generic.py

@@ -20,6 +20,7 @@
 
 from starcluster import tests
 from starcluster.node import Node
+from starcluster.cluster import Cluster
 
 
 class FooNode(Node):
@@ -51,3 +52,22 @@ def test_filter_etc_hosts_lines(self):
         assert len(rejected) == 2
         assert rejected[0] == lines[1]
         assert rejected[1] == lines[2]
+
+    def test_get_free_node_nums(self):
+        res = Cluster.get_free_ids_among_nodes(0, [])
+        assert res == []
+
+        node001 = FooNode("node001", "1.2.3.4")
+        res = Cluster.get_free_ids_among_nodes(1, [node001])
+        assert res == [2]
+
+        res = Cluster.get_free_ids_among_nodes(3, [node001])
+        assert res == [2, 3, 4]
+
+        node003 = FooNode("node003", "1.2.3.4")
+        node005 = FooNode("node005", "1.2.3.4")
+        node006 = FooNode("node006", "1.2.3.4")
+        node106 = FooNode("node106", "1.2.3.4")
+        res = Cluster.get_free_ids_among_nodes(5, [node001, node003, node005,
+                                                   node006, node106])
+        assert res == [2, 4, 7, 8, 9]