Merge lp:~ltrager/maas/fix_exception into lp:maas/trunk

Proposed by Lee Trager on 2016-05-11
Status: Merged
Approved by: Lee Trager on 2016-05-11
Approved revision: 5014
Merged at revision: 5015
Proposed branch: lp:~ltrager/maas/fix_exception
Merge into: lp:maas/trunk
Diff against target: 288 lines (+71/-73)
6 files modified
src/maasserver/api/devices.py (+1/-1)
src/maasserver/api/machines.py (+14/-1)
src/maasserver/api/tests/test_devices.py (+1/-1)
src/maasserver/api/tests/test_machine.py (+50/-13)
src/maasserver/models/node.py (+3/-20)
src/maasserver/models/tests/test_node.py (+2/-37)
To merge this branch: bzr merge lp:~ltrager/maas/fix_exception
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) Approve on 2016-05-11
Blake Rouse 2016-05-11 Approve on 2016-05-11
Review via email: mp+294421@code.launchpad.net

Commit message

In restore methods use assert on the model and raise NodeStateViolation on the API

Description of the change

As per the discussion in the stand up this turns the validation in set_initial_networking to be an assert which will be logged no matter where it is called from. The API now checks the node state and raises a NodeStateViolation if the state is not READY.

To post a comment you must log in.
Blake Rouse (blake-rouse) wrote :

Looks good. Need to fix one of the API validation messages and add a message to the assert as well.

review: Approve
Andres Rodriguez (andreserl) wrote :

Lee, before you alnd this, pelase attach this barnch to https://bugs.launchpad.net/maas/+bug/1580405 and fix the bug title and description to reflect this bugfix.

review: Approve
lp:~ltrager/maas/fix_exception updated on 2016-05-11
5014. By Lee Trager on 2016-05-11

Make blake_r corrections

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/maasserver/api/devices.py'
--- src/maasserver/api/devices.py 2016-05-04 21:14:04 +0000
+++ src/maasserver/api/devices.py 2016-05-11 21:24:10 +0000
@@ -141,7 +141,7 @@
141 device = self.model.objects.get_node_or_404(141 device = self.model.objects.get_node_or_404(
142 system_id=system_id, user=request.user,142 system_id=system_id, user=request.user,
143 perm=NODE_PERMISSION.ADMIN)143 perm=NODE_PERMISSION.ADMIN)
144 device.restore_default_configuration()144 device.set_initial_networking_configuration()
145 return reload_object(device)145 return reload_object(device)
146146
147 @classmethod147 @classmethod
148148
=== modified file 'src/maasserver/api/machines.py'
--- src/maasserver/api/machines.py 2016-05-09 20:13:38 +0000
+++ src/maasserver/api/machines.py 2016-05-11 21:24:10 +0000
@@ -595,6 +595,10 @@
595 machine = self.model.objects.get_node_or_404(595 machine = self.model.objects.get_node_or_404(
596 system_id=system_id, user=request.user,596 system_id=system_id, user=request.user,
597 perm=NODE_PERMISSION.ADMIN)597 perm=NODE_PERMISSION.ADMIN)
598 if machine.status != NODE_STATUS.READY:
599 raise NodeStateViolation(
600 "Machine must be in a ready state to restore networking "
601 "configuration")
598 machine.set_initial_networking_configuration()602 machine.set_initial_networking_configuration()
599 return reload_object(machine)603 return reload_object(machine)
600604
@@ -608,6 +612,10 @@
608 machine = self.model.objects.get_node_or_404(612 machine = self.model.objects.get_node_or_404(
609 system_id=system_id, user=request.user,613 system_id=system_id, user=request.user,
610 perm=NODE_PERMISSION.ADMIN)614 perm=NODE_PERMISSION.ADMIN)
615 if machine.status != NODE_STATUS.READY:
616 raise NodeStateViolation(
617 "Machine must be in a ready state to restore storage "
618 "configuration.")
611 machine.set_default_storage_layout()619 machine.set_default_storage_layout()
612 return reload_object(machine)620 return reload_object(machine)
613621
@@ -621,7 +629,12 @@
621 machine = self.model.objects.get_node_or_404(629 machine = self.model.objects.get_node_or_404(
622 system_id=system_id, user=request.user,630 system_id=system_id, user=request.user,
623 perm=NODE_PERMISSION.ADMIN)631 perm=NODE_PERMISSION.ADMIN)
624 machine.restore_default_configuration()632 if machine.status != NODE_STATUS.READY:
633 raise NodeStateViolation(
634 "Machine must be in a ready state to restore default "
635 "networking and storage configuration.")
636 machine.set_default_storage_layout()
637 machine.set_initial_networking_configuration()
625 return reload_object(machine)638 return reload_object(machine)
626639
627 @operation(idempotent=False)640 @operation(idempotent=False)
628641
=== modified file 'src/maasserver/api/tests/test_devices.py'
--- src/maasserver/api/tests/test_devices.py 2016-05-04 21:14:04 +0000
+++ src/maasserver/api/tests/test_devices.py 2016-05-11 21:24:10 +0000
@@ -376,7 +376,7 @@
376 self.become_admin()376 self.become_admin()
377 device = factory.make_Device()377 device = factory.make_Device()
378 mock_set_initial_networking_config = self.patch(378 mock_set_initial_networking_config = self.patch(
379 node_module.Device, 'restore_default_configuration')379 node_module.Device, 'set_initial_networking_configuration')
380 response = self.client.post(380 response = self.client.post(
381 get_device_uri(device),381 get_device_uri(device),
382 {'op': 'restore_default_configuration'})382 {'op': 'restore_default_configuration'})
383383
=== modified file 'src/maasserver/api/tests/test_machine.py'
--- src/maasserver/api/tests/test_machine.py 2016-05-09 23:54:19 +0000
+++ src/maasserver/api/tests/test_machine.py 2016-05-11 21:24:10 +0000
@@ -60,6 +60,7 @@
60 HasLength,60 HasLength,
61 MockCalledOnce,61 MockCalledOnce,
62 MockCalledOnceWith,62 MockCalledOnceWith,
63 MockNotCalled,
63)64)
64from metadataserver.models import (65from metadataserver.models import (
65 NodeKey,66 NodeKey,
@@ -1894,7 +1895,7 @@
1894 """Get the API URI for `machine`."""1895 """Get the API URI for `machine`."""
1895 return reverse('machine_handler', args=[machine.system_id])1896 return reverse('machine_handler', args=[machine.system_id])
18961897
1897 def test_(self):1898 def test_restore_networking_configuration(self):
1898 self.become_admin()1899 self.become_admin()
1899 machine = factory.make_Machine(status=NODE_STATUS.READY)1900 machine = factory.make_Machine(status=NODE_STATUS.READY)
1900 mock_set_initial_networking_config = self.patch(1901 mock_set_initial_networking_config = self.patch(
@@ -1915,6 +1916,17 @@
1915 self.assertEqual(1916 self.assertEqual(
1916 http.client.FORBIDDEN, response.status_code, response.content)1917 http.client.FORBIDDEN, response.status_code, response.content)
19171918
1919 def test_restore_networking_configuration_checks_machine_status(self):
1920 self.become_admin()
1921 machine = factory.make_Machine(status=NODE_STATUS.DEPLOYED)
1922 mock_set_initial_networking_config = self.patch(
1923 node_module.Machine, 'set_initial_networking_configuration')
1924 response = self.client.post(
1925 self.get_machine_uri(machine),
1926 {'op': 'restore_networking_configuration'})
1927 self.assertEqual(http.client.CONFLICT, response.status_code)
1928 self.assertThat(mock_set_initial_networking_config, MockNotCalled())
1929
19181930
1919class TestRestoreStorageConfiguration(APITestCase):1931class TestRestoreStorageConfiguration(APITestCase):
1920 """Tests for1932 """Tests for
@@ -1945,6 +1957,17 @@
1945 self.assertEqual(1957 self.assertEqual(
1946 http.client.FORBIDDEN, response.status_code, response.content)1958 http.client.FORBIDDEN, response.status_code, response.content)
19471959
1960 def test_restore_storage_configuration_checks_machine_status(self):
1961 self.become_admin()
1962 machine = factory.make_Machine(status=NODE_STATUS.DEPLOYED)
1963 mock_set_default_storage_layout = self.patch(
1964 node_module.Machine, 'set_default_storage_layout')
1965 response = self.client.post(
1966 self.get_machine_uri(machine),
1967 {'op': 'restore_storage_configuration'})
1968 self.assertEqual(http.client.CONFLICT, response.status_code)
1969 self.assertThat(mock_set_default_storage_layout, MockNotCalled())
1970
19481971
1949class TestRestoreDefaultConfiguration(APITestCase):1972class TestRestoreDefaultConfiguration(APITestCase):
1950 """Tests for1973 """Tests for
@@ -1957,23 +1980,37 @@
1957 def test_restore_default_configuration(self):1980 def test_restore_default_configuration(self):
1958 self.become_admin()1981 self.become_admin()
1959 machine = factory.make_Machine(status=NODE_STATUS.READY)1982 machine = factory.make_Machine(status=NODE_STATUS.READY)
1983 mock_set_default_storage_layout = self.patch(
1984 node_module.Machine, 'set_default_storage_layout')
1985 mock_set_initial_networking_config = self.patch(
1986 node_module.Machine, 'set_initial_networking_configuration')
1987 response = self.client.post(
1988 self.get_machine_uri(machine),
1989 {'op': 'restore_default_configuration'})
1990 self.assertEqual(http.client.OK, response.status_code)
1991 self.assertEqual(
1992 machine.system_id, json_load_bytes(response.content)['system_id'])
1993 self.assertThat(mock_set_default_storage_layout, MockCalledOnce())
1994 self.assertThat(mock_set_initial_networking_config, MockCalledOnce())
1995
1996 def test_restore_default_configuration_requires_admin(self):
1997 machine = factory.make_Machine()
1998 response = self.client.post(
1999 self.get_machine_uri(machine),
2000 {'op': 'restore_default_configuration'})
2001 self.assertEqual(
2002 http.client.FORBIDDEN, response.status_code, response.content)
2003
2004 def test_restore_default_configuration_checks_machine_status(self):
2005 self.become_admin()
2006 machine = factory.make_Machine(status=NODE_STATUS.DEPLOYED)
1960 mock_restore_default_configuration = self.patch(2007 mock_restore_default_configuration = self.patch(
1961 node_module.Machine, 'restore_default_configuration')2008 node_module.Machine, 'restore_default_configuration')
1962 response = self.client.post(2009 response = self.client.post(
1963 self.get_machine_uri(machine),2010 self.get_machine_uri(machine),
1964 {'op': 'restore_default_configuration'})2011 {'op': 'restore_default_configuration'})
1965 self.assertEqual(http.client.OK, response.status_code)2012 self.assertEqual(http.client.CONFLICT, response.status_code)
1966 self.assertEqual(2013 self.assertThat(mock_restore_default_configuration, MockNotCalled())
1967 machine.system_id, json_load_bytes(response.content)['system_id'])
1968 self.assertThat(mock_restore_default_configuration, MockCalledOnce())
1969
1970 def test_restore_default_configuration_requires_admin(self):
1971 machine = factory.make_Machine()
1972 response = self.client.post(
1973 self.get_machine_uri(machine),
1974 {'op': 'restore_default_configuration'})
1975 self.assertEqual(
1976 http.client.FORBIDDEN, response.status_code, response.content)
19772014
19782015
1979class TestMarkBroken(APITestCase):2016class TestMarkBroken(APITestCase):
19802017
=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py 2016-05-11 02:26:59 +0000
+++ src/maasserver/models/node.py 2016-05-11 21:24:10 +0000
@@ -2424,11 +2424,9 @@
2424 # No interfaces on the node. Nothing to do.2424 # No interfaces on the node. Nothing to do.
2425 return2425 return
24262426
2427 if self.node_type == NODE_TYPE.MACHINE and self.status in [2427 assert \
2428 NODE_STATUS.DEPLOYING, NODE_STATUS.DEPLOYED]:2428 self.status not in [NODE_STATUS.DEPLOYING, NODE_STATUS.DEPLOYED], \
2429 raise ValidationError(2429 'Node cannot be in a deploying state when configuring network'
2430 "Machine must be in a new, ready, allocated, or failed "
2431 "deployment state to be reset.")
24322430
2433 # Set AUTO mode on the boot interface.2431 # Set AUTO mode on the boot interface.
2434 auto_set = False2432 auto_set = False
@@ -3073,17 +3071,6 @@
3073 super(Machine, self).__init__(3071 super(Machine, self).__init__(
3074 node_type=NODE_TYPE.MACHINE, *args, **kwargs)3072 node_type=NODE_TYPE.MACHINE, *args, **kwargs)
30753073
3076 def restore_default_configuration(self):
3077 """Restores a machine's configuration to default settings."""
3078 if self.status not in [
3079 NODE_STATUS.NEW, NODE_STATUS.READY, NODE_STATUS.ALLOCATED,
3080 NODE_STATUS.FAILED_DEPLOYMENT]:
3081 raise ValidationError(
3082 "Machine must be in a new, ready, allocated, or failed "
3083 "deployment state to restore default configuration.")
3084 self.set_default_storage_layout()
3085 self.set_initial_networking_configuration()
3086
30873074
3088class Controller(Node):3075class Controller(Node):
3089 """A node which is either a rack or region controller."""3076 """A node which is either a rack or region controller."""
@@ -3825,10 +3812,6 @@
3825 super(Device, self).__init__(3812 super(Device, self).__init__(
3826 node_type=NODE_TYPE.DEVICE, *args, **kwargs)3813 node_type=NODE_TYPE.DEVICE, *args, **kwargs)
38273814
3828 def restore_default_configuration(self):
3829 """Restores a device's configuration to default settings."""
3830 self.set_initial_networking_configuration()
3831
3832 def clean_architecture(self, prev):3815 def clean_architecture(self, prev):
3833 # Devices aren't required to have a defined architecture3816 # Devices aren't required to have a defined architecture
3834 pass3817 pass
38353818
=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py 2016-05-11 02:26:59 +0000
+++ src/maasserver/models/tests/test_node.py 2016-05-11 21:24:10 +0000
@@ -296,30 +296,6 @@
296 list(Machine.objects.get_available_machines_for_acquisition(user)))296 list(Machine.objects.get_available_machines_for_acquisition(user)))
297297
298298
299class TestMachine(MAASServerTestCase):
300 def test_restore_default_configuration(self):
301 machine = factory.make_Machine(status=NODE_STATUS.NEW)
302 mock_set_default_storage_layout = self.patch(
303 machine, 'set_default_storage_layout')
304 mock_set_initial_networking_configuration = self.patch(
305 machine, 'set_initial_networking_configuration')
306 machine.restore_default_configuration()
307 self.assertThat(mock_set_default_storage_layout, MockCalledOnce())
308 self.assertThat(
309 mock_set_initial_networking_configuration, MockCalledOnce())
310
311 def test_restore_default_configuration_error_wrong_status(self):
312 machine = factory.make_Machine_with_Interface_on_Subnet(
313 status=factory.pick_choice(
314 NODE_STATUS_CHOICES,
315 but_not=[
316 NODE_STATUS.NEW, NODE_STATUS.READY, NODE_STATUS.ALLOCATED,
317 NODE_STATUS.FAILED_DEPLOYMENT])
318 )
319 self.assertRaises(
320 ValidationError, machine.restore_default_configuration)
321
322
323class TestControllerManager(MAASServerTestCase):299class TestControllerManager(MAASServerTestCase):
324 def test_controller_lists_node_type_rack_and_region(self):300 def test_controller_lists_node_type_rack_and_region(self):
325 racks_and_regions = set()301 racks_and_regions = set()
@@ -508,17 +484,6 @@
508 self.assertEqual('', device.architecture)484 self.assertEqual('', device.architecture)
509485
510486
511class TestDevice(MAASServerTestCase):
512
513 def test_restore_default_configuration(self):
514 device = factory.make_Device()
515 mock_set_initial_networking_configuration = self.patch(
516 device, 'set_initial_networking_configuration')
517 device.restore_default_configuration()
518 self.assertThat(
519 mock_set_initial_networking_configuration, MockCalledOnce())
520
521
522class TestNode(MAASServerTestCase):487class TestNode(MAASServerTestCase):
523488
524 def setUp(self):489 def setUp(self):
@@ -3648,12 +3613,12 @@
3648 alloc_type=IPADDRESS_TYPE.AUTO).first()3613 alloc_type=IPADDRESS_TYPE.AUTO).first()
3649 self.assertIsNone(auto_ip)3614 self.assertIsNone(auto_ip)
36503615
3651 def test_set_initial_net_config_rasies_validation_error_wrong_status(self):3616 def test_set_initial_net_config_asserts_proper_status(self):
3652 machine = factory.make_Machine_with_Interface_on_Subnet(3617 machine = factory.make_Machine_with_Interface_on_Subnet(
3653 status=random.choice([NODE_STATUS.DEPLOYING, NODE_STATUS.DEPLOYED])3618 status=random.choice([NODE_STATUS.DEPLOYING, NODE_STATUS.DEPLOYED])
3654 )3619 )
3655 self.assertRaises(3620 self.assertRaises(
3656 ValidationError, machine.set_initial_networking_configuration)3621 AssertionError, machine.set_initial_networking_configuration)
36573622
3658 def test_set_initial_networking_configuration_auto_on_boot_nic(self):3623 def test_set_initial_networking_configuration_auto_on_boot_nic(self):
3659 node = factory.make_Node_with_Interface_on_Subnet()3624 node = factory.make_Node_with_Interface_on_Subnet()