Andrei, On 06/04/16 14:39 +0300, Andrei Maruha wrote: > Attached patch contains a little bit reworked function next_nodeid> > > [...]
there are two better aligned channels to propose patches (ordered by preference, at least judging based on https://github.com/ClusterLabs/crmsh/pulls?q=is%3Apr+is%3Aclosed): 1. pull request against https://github.com/ClusterLabs/crmsh 2. patch (per git conventions, which is abided here) sent to [email protected] ML, with appropriately labeled topic so that the respective upstream folks can hook on easily (in this case, the prefix should contain "crmsh"; see how I modified the topic, along with cross-posting to the correct list) There is a reason behind having two mailing lists, different audience being the most prominent one (true, devels will likely read both, but the rest of "users" would be better off without such a traffic) Thanks for understanding. -- Jan > From 56d99aa764abb2af8d638425b10a1e493d935e4b Mon Sep 17 00:00:00 2001 > From: Andrei Maruha <[email protected]> > 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()
pgp2AKex2clSh.pgp
Description: PGP signature
_______________________________________________ Developers mailing list [email protected] http://clusterlabs.org/mailman/listinfo/developers
