[ClusterLabs] Rework function next_nodeid.

[Andrei Maruha]

Hi,
Attached patch contains a little bit reworked function next_nodeid, 
because some times it is not clear why node gets new id during rejoin 
operation. Ex:
1. cluster with one node, id is 1;
2. join another node, joined node gets  id 2;
3. delete the node with id 1 from the cluster;
4. join this node again and ... joined node gets id 3.

With this fix node id for the first node after rejoin will be set again 
to 1.

Best Regards,
Andrei Maruha

-------------- next part --------------
From 56d99aa764abb2af8d638425b10a1e493d935e4b Mon Sep 17 00:00:00 2001
From: Andrei Maruha <Andrei_Maruha at epam.com>
Date: Wed, 6 Apr 2016 12:33:27 +0300
Subject: low: corosync: Don't take next node id based on max value, if some
 smaller node id is free.

Do not assign node id equals to 'maxid + 1' in case if some node was
removed and free node id can be reused.

diff --git a/crmsh/corosync.py b/crmsh/corosync.py
index e9950b8..6401f52 100644
--- a/crmsh/corosync.py
+++ b/crmsh/corosync.py
@@ -327,11 +327,16 @@ def diff_configuration(nodes, checksum=False):
         utils.remote_diff(local_path, nodes)
 
 
-def next_nodeid(parser):
+def get_free_nodeid(parser):
     ids = parser.get_all('nodelist.node.nodeid')
     if not ids:
         return 1
-    return max([int(i) for i in ids]) + 1
+    ids = [int(i) for i in ids]
+    max_id = max(ids) + 1
+    for i in xrange(1, max_id):
+        if i not in ids:
+            return i
+    return max_id
 
 
 def get_ip(node):
@@ -399,7 +404,7 @@ def add_node(addr, name=None):
     p = Parser(f)
 
     node_addr = addr
-    node_id = next_nodeid(p)
+    node_id = get_free_nodeid(p)
     node_name = name
     node_value = (make_value('nodelist.node.ring0_addr', node_addr) +
                   make_value('nodelist.node.nodeid', str(node_id)))
diff --git a/test/unittests/test_corosync.py b/test/unittests/test_corosync.py
index db8dd8c..d2a25b6 100644
--- a/test/unittests/test_corosync.py
+++ b/test/unittests/test_corosync.py
@@ -5,6 +5,7 @@
 
 import os
 import unittest
+import mock
 from crmsh import corosync
 from crmsh.corosync import Parser, make_section, make_value
 
@@ -67,7 +68,7 @@ class TestCorosyncParser(unittest.TestCase):
         p.add('nodelist',
               make_section('nodelist.node',
                            make_value('nodelist.node.ring0_addr', '10.10.10.10') +
-                           make_value('nodelist.node.nodeid', str(corosync.next_nodeid(p)))))
+                           make_value('nodelist.node.nodeid', str(corosync.get_free_nodeid(p)))))
         _valid(p)
         self.assertEqual(p.count('nodelist.node'), 6)
         self.assertEqual(p.get_all('nodelist.node.nodeid'),
@@ -75,11 +76,11 @@ class TestCorosyncParser(unittest.TestCase):
 
     def test_add_node_no_nodelist(self):
         "test checks that if there is no nodelist, no node is added"
-        from crmsh.corosync import make_section, make_value, next_nodeid
+        from crmsh.corosync import make_section, make_value, get_free_nodeid
 
         p = Parser(F1)
         _valid(p)
-        nid = next_nodeid(p)
+        nid = get_free_nodeid(p)
         self.assertEqual(p.count('nodelist.node'), nid - 1)
         p.add('nodelist',
               make_section('nodelist.node',
@@ -89,11 +90,11 @@ class TestCorosyncParser(unittest.TestCase):
         self.assertEqual(p.count('nodelist.node'), nid - 1)
 
     def test_add_node_nodelist(self):
-        from crmsh.corosync import make_section, make_value, next_nodeid
+        from crmsh.corosync import make_section, make_value, get_free_nodeid
 
         p = Parser(F2)
         _valid(p)
-        nid = next_nodeid(p)
+        nid = get_free_nodeid(p)
         c = p.count('nodelist.node')
         p.add('nodelist',
               make_section('nodelist.node',
@@ -101,7 +102,7 @@ class TestCorosyncParser(unittest.TestCase):
                            make_value('nodelist.node.nodeid', str(nid))))
         _valid(p)
         self.assertEqual(p.count('nodelist.node'), c + 1)
-        self.assertEqual(next_nodeid(p), nid + 1)
+        self.assertEqual(get_free_nodeid(p), nid + 1)
 
     def test_remove_node(self):
         p = Parser(F2)
@@ -118,5 +119,14 @@ class TestCorosyncParser(unittest.TestCase):
         _valid(p)
         self.assertEqual(p.count('service.ver'), 1)
 
+    def test_get_free_nodeid(self):
+        mock_parser = mock.Mock()
+        mock_parser.get_all.return_value = ['2','5']
+        self.assertEqual(corosync.get_free_nodeid(mock_parser), 1)
+        mock_parser.get_all.return_value = ['1','2','5']
+        self.assertEqual(corosync.get_free_nodeid(mock_parser), 3)
+        mock_parser.get_all.return_value = ['1','2','3']
+        self.assertEqual(corosync.get_free_nodeid(mock_parser), 4)
+
 if __name__ == '__main__':
     unittest.main()