[Maas-devel] State machine and design reviews, was [jason.hobbs at canonical.com: [Merge] lp:~jason-hobbs/maas/expose-enable-disk-erasing-option into lp:maas]
Christian Robottom Reis
kiko at canonical.com
Mon Sep 29 20:11:57 UTC 2014
I made a comment on this MP which I wanted to note more widely:
In the future, changes to the state machine require explicit
discussion and review before we commit to them. The exposure of
complex state machines often kills good software -- we're not going
to make that mistake in MAAS.
I'd like us to have a process where a lightweight discussion and
sign-off happens with the core team on infrastructural changes. We used
to have pre-implementation calls for features in Launchpad that we could
reuse, or perhaps you want to suggest your favorite approach, I'm happy
either way, and I don't want to do any hard enforcement of this at this
time.
But let's ensure that as new features are mooted and designs started,
that we have clarity on the design and fix it before the code is sealed.
Think about this as you work on new features -- communication is key to
the success of any project assembled by a team, and software even more
so (and Fred Brooks be the man).
Thanks,
----- Forwarded message from Jason Hobbs <jason.hobbs at canonical.com> -----
Date: Thu, 25 Sep 2014 22:09:23 -0000
From: Jason Hobbs <jason.hobbs at canonical.com>
To: mp+236039 at code.launchpad.net
Subject: [Merge] lp:~jason-hobbs/maas/expose-enable-disk-erasing-option into lp:maas
X-Launchpad-Message-Rationale: Reviewer @maas-maintainers
X-Launchpad-Notification-Type: code-review
X-Launchpad-Branch: ~jason-hobbs/maas/expose-enable-disk-erasing-option
X-Launchpad-Project: maas
X-Launchpad-Hash: 3f95510c33553bf1cac8d8113198718fe47c7cfe
X-Spambayes-Classification: ham; 0.00
Jason Hobbs has proposed merging lp:~jason-hobbs/maas/expose-enable-disk-erasing-option into lp:maas.
Commit message:
Add a new config option - 'enable_disk_erasing_on_release'.
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~jason-hobbs/maas/expose-enable-disk-erasing-option/+merge/236039
You can enable disk erasing on release now via the API, but not via the Web UI.
--
https://code.launchpad.net/~jason-hobbs/maas/expose-enable-disk-erasing-option/+merge/236039
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jason-hobbs/maas/expose-enable-disk-erasing-option into lp:maas.
=== modified file 'src/maasserver/api/nodes.py'
--- src/maasserver/api/nodes.py 2014-09-22 05:33:12 +0000
+++ src/maasserver/api/nodes.py 2014-09-25 21:59:28 +0000
@@ -324,7 +324,7 @@
# postcondition is achieved, so call this success.
pass
elif node.status in RELEASABLE_STATUSES:
- node.release()
+ node.release_or_erase()
else:
raise NodeStateViolation(
"Node cannot be released in its current state ('%s')."
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2014-09-23 14:16:33 +0000
+++ src/maasserver/forms.py 2014-09-25 21:59:28 +0000
@@ -26,6 +26,7 @@
"get_node_edit_form",
"get_node_create_form",
"list_all_usable_architectures",
+ "DiskErasingOnReleaseForm",
"MAASAndNetworkForm",
"MACAddressForm",
"NetworkConnectMACsForm",
@@ -1085,6 +1086,12 @@
enable_third_party_drivers = get_config_field('enable_third_party_drivers')
+class DiskErasingOnReleaseForm(ConfigForm):
+ """Settings page, Disk Erasing on Release section."""
+ enable_disk_erasing_on_release = get_config_field(
+ 'enable_disk_erasing_on_release')
+
+
class CommissioningForm(ConfigForm):
"""Settings page, Commissioning section."""
check_compatibility = get_config_field('check_compatibility')
=== modified file 'src/maasserver/forms_settings.py'
--- src/maasserver/forms_settings.py 2014-09-22 14:47:49 +0000
+++ src/maasserver/forms_settings.py 2014-09-25 21:59:28 +0000
@@ -257,6 +257,15 @@
"using KMS activation.)")
}
},
+ 'enable_disk_erasing_on_release': {
+ 'default': False,
+ 'form': forms.BooleanField,
+ 'form_kwargs': {
+ 'required': False,
+ 'label': (
+ "Erase nodes' disks prior to releasing.")
+ }
+ },
}
=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py 2014-07-03 07:01:15 +0000
+++ src/maasserver/models/config.py 2014-09-25 21:59:28 +0000
@@ -58,6 +58,7 @@
'rpc_region_certificate': None,
# Third Party
'enable_third_party_drivers': True,
+ 'enable_disk_erasing_on_release': False,
## /settings
}
=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py 2014-09-25 18:08:05 +0000
+++ src/maasserver/models/node.py 2014-09-25 21:59:28 +0000
@@ -1240,6 +1240,17 @@
self.license_key = ''
self.save()
+ def release_or_erase(self):
+ """Either release the node or erase the node then release it, depending
+ on settings."""
+ erase_on_release = Config.objects.get_config(
+ 'enable_disk_erasing_on_release')
+ if erase_on_release:
+ self.start_disk_erasing(self.owner)
+ return
+
+ self.release()
+
def set_netboot(self, on=True):
"""Set netboot on or off."""
maaslog.debug("%s: Turning on netboot for node", self.hostname)
=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py 2014-09-25 17:25:08 +0000
+++ src/maasserver/models/tests/test_node.py 2014-09-25 21:59:28 +0000
@@ -80,6 +80,7 @@
from maastesting.matchers import (
MockAnyCall,
MockCalledOnceWith,
+ MockNotCalled,
)
from maastesting.testcase import MAASTestCase
from metadataserver.enum import RESULT_TYPE
@@ -1695,6 +1696,28 @@
node.set_netboot(False)
self.assertFalse(node.netboot)
+ def test_release_or_erase_erases_when_enabled(self):
+ owner = factory.make_User()
+ node = factory.make_Node(status=NODE_STATUS.ALLOCATED, owner=owner)
+ Config.objects.set_config(
+ 'enable_disk_erasing_on_release', True)
+ erase_mock = self.patch_autospec(node, 'start_disk_erasing')
+ release_mock = self.patch_autospec(node, 'release')
+ node.release_or_erase()
+ self.assertThat(erase_mock, MockCalledOnceWith(owner))
+ self.assertThat(release_mock, MockNotCalled())
+
+ def test_release_or_erase_releases_when_disabled(self):
+ owner = factory.make_User()
+ node = factory.make_Node(status=NODE_STATUS.ALLOCATED, owner=owner)
+ Config.objects.set_config(
+ 'enable_disk_erasing_on_release', False)
+ erase_mock = self.patch_autospec(node, 'start_disk_erasing')
+ release_mock = self.patch_autospec(node, 'release')
+ node.release_or_erase()
+ self.assertThat(release_mock, MockCalledOnceWith())
+ self.assertThat(erase_mock, MockNotCalled())
+
class NodeManagerTest_StartNodes(MAASServerTestCase):
=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py 2014-09-25 17:25:08 +0000
+++ src/maasserver/node_action.py 2014-09-25 21:59:28 +0000
@@ -330,7 +330,7 @@
def execute(self, allow_redirect=True):
"""See `NodeAction.execute`."""
- self.node.release()
+ self.node.release_or_erase()
return dedent("""\
This node is no longer allocated to you.
""")
----- End forwarded message -----
--
Christian Robottom Reis | [+1] 612 888 4935 | http://launchpad.net/~kiko
Canonical VP Hyperscale | [+55 16] 9 9112 6430 | http://async.com.br/~kiko
More information about the Maas-devel
mailing list