Merge lp:~gz/juju-ci-tools/substrate_maas_refac into lp:juju-ci-tools

Proposed by Martin Packman
Status: Merged
Merged at revision: 1475
Proposed branch: lp:~gz/juju-ci-tools/substrate_maas_refac
Merge into: lp:juju-ci-tools
Diff against target: 200 lines (+47/-43)
2 files modified
substrate.py (+14/-11)
tests/test_substrate.py (+33/-32)
To merge this branch: bzr merge lp:~gz/juju-ci-tools/substrate_maas_refac
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+297783@code.launchpad.net

Description of the change

Changes to maas substrate implementation

As part of adding a bunch more maas substrate commands, I ended up changing some details of how the existing code worked that meant updating the existing tests. This branch is just the existing code with those changes to separate out the new additions from the implementation changes.

Note that login/logout still dump the maas output straight to stdout (and our ci logs), but other commands now consume the output given by maas and parse it as json. Commands that want to output something for the ci logs should include logging of those maas return values.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'substrate.py'
--- substrate.py 2016-05-31 23:12:41 +0000
+++ substrate.py 2016-06-17 15:01:00 +0000
@@ -414,33 +414,36 @@
414 self.url = urlparse.urljoin(url, self._API_PATH)414 self.url = urlparse.urljoin(url, self._API_PATH)
415 self.oauth = oauth415 self.oauth = oauth
416416
417 def _maas(self, *args):
418 """Call maas api with given arguments and parse json result."""
419 output = subprocess.check_output(('maas',) + args)
420 return json.loads(output)
421
417 def login(self):422 def login(self):
418 """Login with the maas cli."""423 """Login with the maas cli."""
419 subprocess.check_call(424 subprocess.check_call([
420 ['maas', 'login', self.profile, self.url, self.oauth])425 'maas', 'login', self.profile, self.url, self.oauth])
421426
422 def logout(self):427 def logout(self):
423 """Logout with the maas cli."""428 """Logout with the maas cli."""
424 subprocess.check_call(429 subprocess.check_call(['maas', 'logout', self.profile])
425 ['maas', 'logout', self.profile])
426430
427 def _machine_release_args(self, machine_id):431 def _machine_release_args(self, machine_id):
428 return ['maas', self.profile, 'machine', 'release', machine_id]432 return (self.profile, 'machine', 'release', machine_id)
429433
430 def terminate_instances(self, instance_ids):434 def terminate_instances(self, instance_ids):
431 """Terminate the specified instances."""435 """Terminate the specified instances."""
432 for instance in instance_ids:436 for instance in instance_ids:
433 maas_system_id = instance.split('/')[5]437 maas_system_id = instance.split('/')[5]
434 log.info('Deleting %s.' % instance)438 log.info('Deleting %s.' % instance)
435 subprocess.check_call(self._machine_release_args(maas_system_id))439 self._maas(*self._machine_release_args(maas_system_id))
436440
437 def _list_allocated_args(self):441 def _list_allocated_args(self):
438 return ['maas', self.profile, 'machines', 'list-allocated']442 return (self.profile, 'machines', 'list-allocated')
439443
440 def get_allocated_nodes(self):444 def get_allocated_nodes(self):
441 """Return a dict of allocated nodes with the hostname as keys."""445 """Return a dict of allocated nodes with the hostname as keys."""
442 data = subprocess.check_output(self._list_allocated_args())446 nodes = self._maas(*self._list_allocated_args())
443 nodes = json.loads(data)
444 allocated = {node['hostname']: node for node in nodes}447 allocated = {node['hostname']: node for node in nodes}
445 return allocated448 return allocated
446449
@@ -462,10 +465,10 @@
462 _API_PATH = 'api/1.0/'465 _API_PATH = 'api/1.0/'
463466
464 def _list_allocated_args(self):467 def _list_allocated_args(self):
465 return ['maas', self.profile, 'nodes', 'list-allocated']468 return (self.profile, 'nodes', 'list-allocated')
466469
467 def _machine_release_args(self, machine_id):470 def _machine_release_args(self, machine_id):
468 return ['maas', self.profile, 'node', 'release', machine_id]471 return (self.profile, 'node', 'release', machine_id)
469472
470473
471@contextmanager474@contextmanager
472475
=== modified file 'tests/test_substrate.py'
--- tests/test_substrate.py 2016-05-31 23:12:41 +0000
+++ tests/test_substrate.py 2016-06-17 15:01:00 +0000
@@ -171,18 +171,17 @@
171171
172 def test_terminate_maas(self):172 def test_terminate_maas(self):
173 env = get_maas_env()173 env = get_maas_env()
174 with patch('subprocess.check_call') as cc_mock:174 with patch('subprocess.check_call', autospec=True) as cc_mock:
175 terminate_instances(env, ['/A/B/C/D/node-3d/'])175 with patch('subprocess.check_output', autospec=True,
176 expected = (176 return_value='{}') as co_mock:
177 ['maas', 'login', 'mas', 'http://10.0.10.10/MAAS/api/2.0/',177 terminate_instances(env, ['/A/B/C/D/node-3d/'])
178 'a:password:string'],178 self.assertEquals(cc_mock.call_args_list, [
179 )179 call(['maas', 'login', 'mas', 'http://10.0.10.10/MAAS/api/2.0/',
180 self.assertEqual(expected, cc_mock.call_args_list[0][0])180 'a:password:string']),
181 expected = (['maas', 'mas', 'machine', 'release', 'node-3d'],)181 call(['maas', 'logout', 'mas']),
182 self.assertEqual(expected, cc_mock.call_args_list[1][0])182 ])
183 expected = (['maas', 'logout', 'mas'],)183 co_mock.assert_called_once_with(
184 self.assertEqual(expected, cc_mock.call_args_list[2][0])184 ('maas', 'mas', 'machine', 'release', 'node-3d'))
185 self.assertEqual(3, len(cc_mock.call_args_list))
186 self.assertEqual(185 self.assertEqual(
187 self.log_stream.getvalue(),186 self.log_stream.getvalue(),
188 'INFO Deleting /A/B/C/D/node-3d/.\n')187 'INFO Deleting /A/B/C/D/node-3d/.\n')
@@ -763,17 +762,18 @@
763 account.logout()762 account.logout()
764 cc_mock.assert_called_once_with(['maas', 'logout', 'mas'])763 cc_mock.assert_called_once_with(['maas', 'logout', 'mas'])
765764
766 @patch('subprocess.check_call', autospec=True)765 def test_terminate_instances(self):
767 def test_terminate_instances(self, cc_mock):
768 config = get_maas_env().config766 config = get_maas_env().config
769 account = MAASAccount(767 account = MAASAccount(
770 config['name'], config['maas-server'], config['maas-oauth'])768 config['name'], config['maas-server'], config['maas-oauth'])
771 instance_ids = ['/A/B/C/D/node-1d/', '/A/B/C/D/node-2d/']769 instance_ids = ['/A/B/C/D/node-1d/', '/A/B/C/D/node-2d/']
772 account.terminate_instances(instance_ids)770 with patch('subprocess.check_output', autospec=True,
773 cc_mock.assert_any_call(771 return_value='{}') as co_mock:
774 ['maas', 'mas', 'machine', 'release', 'node-1d'])772 account.terminate_instances(instance_ids)
775 cc_mock.assert_called_with(773 co_mock.assert_any_call(
776 ['maas', 'mas', 'machine', 'release', 'node-2d'])774 ('maas', 'mas', 'machine', 'release', 'node-1d'))
775 co_mock.assert_called_with(
776 ('maas', 'mas', 'machine', 'release', 'node-2d'))
777777
778 def test_get_allocated_nodes(self):778 def test_get_allocated_nodes(self):
779 config = get_maas_env().config779 config = get_maas_env().config
@@ -785,7 +785,7 @@
785 return_value=allocated_nodes_string) as co_mock:785 return_value=allocated_nodes_string) as co_mock:
786 allocated = account.get_allocated_nodes()786 allocated = account.get_allocated_nodes()
787 co_mock.assert_called_once_with(787 co_mock.assert_called_once_with(
788 ['maas', 'mas', 'machines', 'list-allocated'])788 ('maas', 'mas', 'machines', 'list-allocated'))
789 self.assertEqual(node, allocated['maas-node-1.maas'])789 self.assertEqual(node, allocated['maas-node-1.maas'])
790790
791 def test_get_allocated_ips(self):791 def test_get_allocated_ips(self):
@@ -798,7 +798,7 @@
798 return_value=allocated_nodes_string) as co_mock:798 return_value=allocated_nodes_string) as co_mock:
799 ips = account.get_allocated_ips()799 ips = account.get_allocated_ips()
800 co_mock.assert_called_once_with(800 co_mock.assert_called_once_with(
801 ['maas', 'mas', 'machines', 'list-allocated'])801 ('maas', 'mas', 'machines', 'list-allocated'))
802 self.assertEqual('10.0.30.165', ips['maas-node-1.maas'])802 self.assertEqual('10.0.30.165', ips['maas-node-1.maas'])
803803
804 def test_get_allocated_ips_empty(self):804 def test_get_allocated_ips_empty(self):
@@ -812,7 +812,7 @@
812 return_value=allocated_nodes_string) as co_mock:812 return_value=allocated_nodes_string) as co_mock:
813 ips = account.get_allocated_ips()813 ips = account.get_allocated_ips()
814 co_mock.assert_called_once_with(814 co_mock.assert_called_once_with(
815 ['maas', 'mas', 'machines', 'list-allocated'])815 ('maas', 'mas', 'machines', 'list-allocated'))
816 self.assertEqual({}, ips)816 self.assertEqual({}, ips)
817817
818818
@@ -836,17 +836,18 @@
836 account.logout()836 account.logout()
837 cc_mock.assert_called_once_with(['maas', 'logout', 'mas'])837 cc_mock.assert_called_once_with(['maas', 'logout', 'mas'])
838838
839 @patch('subprocess.check_call', autospec=True)839 def test_terminate_instances(self):
840 def test_terminate_instances(self, cc_mock):
841 config = get_maas_env().config840 config = get_maas_env().config
842 account = MAAS1Account(841 account = MAAS1Account(
843 config['name'], config['maas-server'], config['maas-oauth'])842 config['name'], config['maas-server'], config['maas-oauth'])
844 instance_ids = ['/A/B/C/D/node-1d/', '/A/B/C/D/node-2d/']843 instance_ids = ['/A/B/C/D/node-1d/', '/A/B/C/D/node-2d/']
845 account.terminate_instances(instance_ids)844 with patch('subprocess.check_output', autospec=True,
846 cc_mock.assert_any_call(845 return_value='{}') as co_mock:
847 ['maas', 'mas', 'node', 'release', 'node-1d'])846 account.terminate_instances(instance_ids)
848 cc_mock.assert_called_with(847 co_mock.assert_any_call(
849 ['maas', 'mas', 'node', 'release', 'node-2d'])848 ('maas', 'mas', 'node', 'release', 'node-1d'))
849 co_mock.assert_called_with(
850 ('maas', 'mas', 'node', 'release', 'node-2d'))
850851
851 def test_get_allocated_nodes(self):852 def test_get_allocated_nodes(self):
852 config = get_maas_env().config853 config = get_maas_env().config
@@ -858,7 +859,7 @@
858 return_value=allocated_nodes_string) as co_mock:859 return_value=allocated_nodes_string) as co_mock:
859 allocated = account.get_allocated_nodes()860 allocated = account.get_allocated_nodes()
860 co_mock.assert_called_once_with(861 co_mock.assert_called_once_with(
861 ['maas', 'mas', 'nodes', 'list-allocated'])862 ('maas', 'mas', 'nodes', 'list-allocated'))
862 self.assertEqual(node, allocated['maas-node-1.maas'])863 self.assertEqual(node, allocated['maas-node-1.maas'])
863864
864 def test_get_allocated_ips(self):865 def test_get_allocated_ips(self):
@@ -871,7 +872,7 @@
871 return_value=allocated_nodes_string) as co_mock:872 return_value=allocated_nodes_string) as co_mock:
872 ips = account.get_allocated_ips()873 ips = account.get_allocated_ips()
873 co_mock.assert_called_once_with(874 co_mock.assert_called_once_with(
874 ['maas', 'mas', 'nodes', 'list-allocated'])875 ('maas', 'mas', 'nodes', 'list-allocated'))
875 self.assertEqual('10.0.30.165', ips['maas-node-1.maas'])876 self.assertEqual('10.0.30.165', ips['maas-node-1.maas'])
876877
877 def test_get_allocated_ips_empty(self):878 def test_get_allocated_ips_empty(self):
@@ -885,7 +886,7 @@
885 return_value=allocated_nodes_string) as co_mock:886 return_value=allocated_nodes_string) as co_mock:
886 ips = account.get_allocated_ips()887 ips = account.get_allocated_ips()
887 co_mock.assert_called_once_with(888 co_mock.assert_called_once_with(
888 ['maas', 'mas', 'nodes', 'list-allocated'])889 ('maas', 'mas', 'nodes', 'list-allocated'))
889 self.assertEqual({}, ips)890 self.assertEqual({}, ips)
890891
891892

Subscribers

People subscribed via source and target branches