Merge ~adam-collard/maas:vmfs7-validate-boot-disk-size into maas:master

Proposed by Adam Collard
Status: Merged
Approved by: Adam Collard
Approved revision: f99c94443bbbb3ccaf4416d28d89f38da4fd5b66
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~adam-collard/maas:vmfs7-validate-boot-disk-size
Merge into: maas:master
Diff against target: 167 lines (+35/-16)
2 files modified
src/maasserver/storage_layouts.py (+21/-3)
src/maasserver/tests/test_storage_layouts.py (+14/-13)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Björn Tillenius Approve
Review via email: mp+407034@code.launchpad.net

Commit message

LP:1931736 Fix validation of boot disk for VMFS6 and 7

Validate VMFS7 requiring a boot disk of at least 32Gb, VMFS6 of 10Gb.

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b vmfs7-validate-boot-disk-size lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 232e54943c7d7d3eb80c077fe3c68e9506c3df6f

review: Approve
Revision history for this message
Björn Tillenius (bjornt) wrote :

Looks good, but considering we use GB for disks, maybe we should use 1000**3 instead of 1024**3?

review: Approve
7267039... by Adam Collard

Typo fixes

02aa030... by Adam Collard

Redundant specificity with assertDictEqual

e546136... by Adam Collard

Move error to "boot_size" instead of "size" to clarify

f99c944... by Adam Collard

Remove redundant setup

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b vmfs7-validate-boot-disk-size lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10660/console
COMMIT: f99c94443bbbb3ccaf4416d28d89f38da4fd5b66

review: Needs Fixing
Revision history for this message
Adam Collard (adam-collard) wrote :

jenkins: !test

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b vmfs7-validate-boot-disk-size lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: f99c94443bbbb3ccaf4416d28d89f38da4fd5b66

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/storage_layouts.py b/src/maasserver/storage_layouts.py
2index e06da76..f3ca2ac 100644
3--- a/src/maasserver/storage_layouts.py
4+++ b/src/maasserver/storage_layouts.py
5@@ -62,7 +62,7 @@ class StorageLayoutBase(Form):
6 def _load_physical_block_devices(self):
7 """Load all the `PhysicalBlockDevice`'s for node."""
8 # The websocket prefetches node.blockdevice_set, creating a queryset
9- # on node.physicalblockdevice_set adds addtional queries.
10+ # on node.physicalblockdevice_set adds additional queries.
11 physical_bds = []
12 for bd in self.node.blockdevice_set.all():
13 try:
14@@ -813,10 +813,15 @@ class VMFS6StorageLayout(StorageLayoutBase):
15 {"size": 2560 * 1024 ** 2},
16 ]
17
18+ def _clean_boot_disk(self):
19+ if self.boot_disk.size < (10 * 1024 ** 3):
20+ set_form_error(
21+ self, "boot_size", "Boot disk must be at least 10Gb."
22+ )
23+
24 def clean(self):
25 cleaned_data = super().clean()
26- if self.boot_disk.size < 1024 ** 3:
27- set_form_error(self, "size", "Boot disk must be at least 10G.")
28+ self._clean_boot_disk()
29 return cleaned_data
30
31 def configure_storage(self, allow_fallback):
32@@ -915,6 +920,19 @@ class VMFS7StorageLayout(VMFS6StorageLayout):
33 {"size": 0},
34 ]
35
36+ def _clean_boot_disk(self):
37+ """https://docs.vmware.com/en/VMware-vSphere/7.0/com.vmware.esxi.install.doc/GUID-DEB8086A-306B-4239-BF76-E354679202FC.html
38+
39+ ESXi 7.0 requires a boot disk of at least 32 GB of persistent
40+ storage such as HDD, SSD, or NVMe. Use USB, SD and non-USB
41+ flash media devices only for ESXi boot bank partitions. A boot
42+ device must not be shared between ESXi hosts.
43+ """
44+ if self.boot_disk.size < (32 * 1024 ** 3):
45+ set_form_error(
46+ self, "boot_size", "Boot disk must be at least 32Gb."
47+ )
48+
49 def configure_storage(self, allow_fallback):
50 super().configure_storage(allow_fallback)
51 return "VMFS7"
52diff --git a/src/maasserver/tests/test_storage_layouts.py b/src/maasserver/tests/test_storage_layouts.py
53index c85425b..b17f14e 100644
54--- a/src/maasserver/tests/test_storage_layouts.py
55+++ b/src/maasserver/tests/test_storage_layouts.py
56@@ -242,7 +242,7 @@ class TestStorageLayoutBase(MAASServerTestCase):
57 error.message_dict,
58 )
59
60- def test_raises_error_when_value_to_low_for_boot_disk(self):
61+ def test_raises_error_when_value_too_low_for_boot_disk(self):
62 node = make_Node_with_uefi_boot_method()
63 factory.make_PhysicalBlockDevice(node=node, size=LARGE_BLOCK_DEVICE)
64 layout = StorageLayoutBase(
65@@ -259,7 +259,7 @@ class TestStorageLayoutBase(MAASServerTestCase):
66 error.message_dict,
67 )
68
69- def test_raises_error_when_percentage_to_high_for_boot_disk(self):
70+ def test_raises_error_when_percentage_too_high_for_boot_disk(self):
71 node = make_Node_with_uefi_boot_method()
72 boot_disk = factory.make_PhysicalBlockDevice(
73 node=node, size=LARGE_BLOCK_DEVICE
74@@ -1853,7 +1853,7 @@ class TestVMFS6StorageLayout(MAASServerTestCase):
75 layout = VMFS6StorageLayout(node)
76 self.assertEqual("VMFS6", layout.configure())
77 pt = node.boot_disk.get_partitiontable()
78- self.assertDictEqual(
79+ self.assertEqual(
80 {
81 "%s-part1" % node.boot_disk.name: 3 * 1024 ** 2,
82 "%s-part2" % node.boot_disk.name: 4 * 1024 ** 3,
83@@ -1881,12 +1881,13 @@ class TestVMFS6StorageLayout(MAASServerTestCase):
84 def test_clean_validates_min_size(self):
85 node = factory.make_Node(with_boot_disk=False)
86 node.boot_disk = factory.make_PhysicalBlockDevice(
87- node=node, size=1024 ** 3 - 1
88+ node=node, size=10 * 1024 ** 3 - 1
89 )
90 layout = VMFS6StorageLayout(node)
91 error = self.assertRaises(StorageLayoutFieldsError, layout.configure)
92 self.assertEqual(
93- {"size": ["Boot disk must be at least 10G."]}, error.message_dict
94+ {"boot_size": ["Boot disk must be at least 10Gb."]},
95+ error.message_dict,
96 )
97
98 def test_accepts_root_device_param(self):
99@@ -1901,7 +1902,7 @@ class TestVMFS6StorageLayout(MAASServerTestCase):
100 layout = VMFS6StorageLayout(node, {"root_device": root_disk.id})
101 self.assertEqual("VMFS6", layout.configure())
102 pt = root_disk.get_partitiontable()
103- self.assertDictEqual(
104+ self.assertEqual(
105 {
106 "%s-part1" % root_disk.name: 3 * 1024 ** 2,
107 "%s-part2" % root_disk.name: 4 * 1024 ** 3,
108@@ -1934,7 +1935,7 @@ class TestVMFS6StorageLayout(MAASServerTestCase):
109 layout = VMFS6StorageLayout(node, {"root_size": 10 * 1024 ** 3})
110 self.assertEqual("VMFS6", layout.configure())
111 pt = node.boot_disk.get_partitiontable()
112- self.assertDictEqual(
113+ self.assertEqual(
114 {
115 "%s-part1" % node.boot_disk.name: 3 * 1024 ** 2,
116 "%s-part2" % node.boot_disk.name: 4 * 1024 ** 3,
117@@ -1987,7 +1988,6 @@ class TestVMFS6StorageLayout(MAASServerTestCase):
118 class TestVMFS7StorageLayout(MAASServerTestCase):
119 def test_init_sets_up_all_fields(self):
120 node = factory.make_Node(with_boot_disk=False)
121- factory.make_PhysicalBlockDevice(node=node, size=LARGE_BLOCK_DEVICE)
122 layout = VMFS7StorageLayout(node)
123 self.assertItemsEqual(
124 ["root_device", "root_size", "boot_size"], layout.fields.keys()
125@@ -2001,7 +2001,7 @@ class TestVMFS7StorageLayout(MAASServerTestCase):
126 layout = VMFS7StorageLayout(node)
127 self.assertEqual("VMFS7", layout.configure())
128 pt = node.boot_disk.get_partitiontable()
129- self.assertDictEqual(
130+ self.assertEqual(
131 {
132 "%s-part1" % node.boot_disk.name: 105 * 1024 ** 2,
133 "%s-part5" % node.boot_disk.name: 1074 * 1024 ** 2,
134@@ -2023,12 +2023,13 @@ class TestVMFS7StorageLayout(MAASServerTestCase):
135 def test_clean_validates_min_size(self):
136 node = factory.make_Node(with_boot_disk=False)
137 node.boot_disk = factory.make_PhysicalBlockDevice(
138- node=node, size=1024 ** 3 - 1
139+ node=node, size=32 * 1024 ** 3 - 1
140 )
141 layout = VMFS7StorageLayout(node)
142 error = self.assertRaises(StorageLayoutFieldsError, layout.configure)
143 self.assertEqual(
144- {"size": ["Boot disk must be at least 10G."]}, error.message_dict
145+ {"boot_size": ["Boot disk must be at least 32Gb."]},
146+ error.message_dict,
147 )
148
149 def test_accepts_root_device_param(self):
150@@ -2043,7 +2044,7 @@ class TestVMFS7StorageLayout(MAASServerTestCase):
151 layout = VMFS7StorageLayout(node, {"root_device": root_disk.id})
152 self.assertEqual("VMFS7", layout.configure())
153 pt = root_disk.get_partitiontable()
154- self.assertDictEqual(
155+ self.assertEqual(
156 {
157 "%s-part1" % root_disk.name: 105 * 1024 ** 2,
158 "%s-part5" % root_disk.name: 1074 * 1024 ** 2,
159@@ -2070,7 +2071,7 @@ class TestVMFS7StorageLayout(MAASServerTestCase):
160 layout = VMFS7StorageLayout(node, {"root_size": 10 * 1024 ** 3})
161 self.assertEqual("VMFS7", layout.configure())
162 pt = node.boot_disk.get_partitiontable()
163- self.assertDictEqual(
164+ self.assertEqual(
165 {
166 "%s-part1" % node.boot_disk.name: 105 * 1024 ** 2,
167 "%s-part5" % node.boot_disk.name: 1074 * 1024 ** 2,

Subscribers

People subscribed via source and target branches