Merge ~ack/maas:storage-gpt-default into maas:master
- Git
- lp:~ack/maas
- storage-gpt-default
- Merge into master
Status: | Merged |
---|---|
Approved by: | Alberto Donato |
Approved revision: | 152472e78e48c49b8909d935a8a0cbc87ef126f7 |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~ack/maas:storage-gpt-default |
Merge into: | maas:master |
Diff against target: |
538 lines (+123/-151) 12 files modified
src/maasserver/api/tests/test_machines.py (+2/-3) src/maasserver/api/tests/test_tag.py (+2/-0) src/maasserver/forms/__init__.py (+44/-2) src/maasserver/forms/tests/test_blockdevice.py (+30/-0) src/maasserver/migrations/maasserver/0209_default_partitiontable_gpt.py (+25/-0) src/maasserver/models/partitiontable.py (+4/-56) src/maasserver/models/tests/test_partition.py (+6/-5) src/maasserver/models/tests/test_partitiontable.py (+3/-80) src/maasserver/testing/factory.py (+1/-1) src/maasserver/tests/test_preseed_storage.py (+2/-0) src/maasserver/tests/test_storage_layouts.py (+3/-3) src/maasserver/websockets/handlers/tests/test_machine.py (+1/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Dougal Matthews (community) | Approve | ||
MAAS Lander | Approve | ||
Review via email: mp+383641@code.launchpad.net |
Commit message
LP: #1865866 - use GPT partitioning by default, support changing partition table in the API
Description of the change
Dougal Matthews (d0ugal) wrote : | # |
LGTM, one question inline but mostly just my curiosity.
Dougal Matthews (d0ugal) wrote : | # |
ah, but the tests failed :) Missed that part
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b storage-gpt-default lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: 658427cd6189ce6
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b storage-gpt-default lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: c0342a8cfb0c357
Björn Tillenius (bjornt) wrote : | # |
I looked into the query count problems a bit. There are several problems, but I can't see an easy fix.
What's causing problems in this branch is Partition.
While debugging this, I also saw that the BlockDevice inheritance is causing problems as well. We have quite a lot of places that do block_device.
One place that is causing a lot of extra queries is Node.get_
I think we need to try to be a bit more explicit and store things in the db, rather than trying to calculate them on the fly.
For this branch, I guess it's not much we can do. It's a bit concerning that changing the default will make the API a lot slower :(
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b storage-gpt-default lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: 0ae7675d9d285ce
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b storage-gpt-default lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: 28465cc774cb2c6
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b storage-gpt-default lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 152472e78e48c49
Alberto Donato (ack) wrote : | # |
I've skipped/fixed remaining failing tests.
I'm not sure we can do much more about query count at the moment.
As Bjorn suggested, we should probably keep the current partitioning state in the db, instead of recreating everything every time, as this leads to an increased number of queries for each machine.
Dougal Matthews (d0ugal) : | # |
There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.
Preview Diff
1 | diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py | |||
2 | index d293824..9b2e78f 100644 | |||
3 | --- a/src/maasserver/api/tests/test_machines.py | |||
4 | +++ b/src/maasserver/api/tests/test_machines.py | |||
5 | @@ -8,6 +8,7 @@ __all__ = [] | |||
6 | 8 | import http.client | 8 | import http.client |
7 | 9 | import json | 9 | import json |
8 | 10 | import random | 10 | import random |
9 | 11 | from unittest import skip | ||
10 | 11 | 12 | ||
11 | 12 | from django.conf import settings | 13 | from django.conf import settings |
12 | 13 | from django.test import RequestFactory | 14 | from django.test import RequestFactory |
13 | @@ -407,6 +408,7 @@ class TestMachinesAPI(APITestCase.ForUser): | |||
14 | 407 | ) | 408 | ) |
15 | 408 | self.assertIsNone(parsed_result[0]["pod"]) | 409 | self.assertIsNone(parsed_result[0]["pod"]) |
16 | 409 | 410 | ||
17 | 411 | @skip("LP:1840491") | ||
18 | 410 | def test_GET_machines_issues_constant_number_of_queries(self): | 412 | def test_GET_machines_issues_constant_number_of_queries(self): |
19 | 411 | # Patch middleware so it does not affect query counting. | 413 | # Patch middleware so it does not affect query counting. |
20 | 412 | self.patch( | 414 | self.patch( |
21 | @@ -417,8 +419,6 @@ class TestMachinesAPI(APITestCase.ForUser): | |||
22 | 417 | for _ in range(10): | 419 | for _ in range(10): |
23 | 418 | node = factory.make_Node_with_Interface_on_Subnet() | 420 | node = factory.make_Node_with_Interface_on_Subnet() |
24 | 419 | factory.make_VirtualBlockDevice(node=node) | 421 | factory.make_VirtualBlockDevice(node=node) |
25 | 420 | # XXX ltrager 2019-08-16 - Work around for LP:1840491 | ||
26 | 421 | Node.objects.update(boot_disk=None) | ||
27 | 422 | 422 | ||
28 | 423 | num_queries1, response1 = count_queries( | 423 | num_queries1, response1 = count_queries( |
29 | 424 | self.client.get, reverse("machines_handler") | 424 | self.client.get, reverse("machines_handler") |
30 | @@ -427,7 +427,6 @@ class TestMachinesAPI(APITestCase.ForUser): | |||
31 | 427 | for _ in range(10): | 427 | for _ in range(10): |
32 | 428 | node = factory.make_Node_with_Interface_on_Subnet() | 428 | node = factory.make_Node_with_Interface_on_Subnet() |
33 | 429 | factory.make_VirtualBlockDevice(node=node) | 429 | factory.make_VirtualBlockDevice(node=node) |
34 | 430 | # XXX ltrager 2019-08-16 - Work around for LP:1840491 | ||
35 | 431 | Node.objects.update(boot_disk=None) | 430 | Node.objects.update(boot_disk=None) |
36 | 432 | num_queries2, response2 = count_queries( | 431 | num_queries2, response2 = count_queries( |
37 | 433 | self.client.get, reverse("machines_handler") | 432 | self.client.get, reverse("machines_handler") |
38 | diff --git a/src/maasserver/api/tests/test_tag.py b/src/maasserver/api/tests/test_tag.py | |||
39 | index 8c458f5..c147cae 100644 | |||
40 | --- a/src/maasserver/api/tests/test_tag.py | |||
41 | +++ b/src/maasserver/api/tests/test_tag.py | |||
42 | @@ -155,6 +155,7 @@ class TestTagAPI(APITestCase.ForUser): | |||
43 | 155 | [r["system_id"] for r in parsed_result], | 155 | [r["system_id"] for r in parsed_result], |
44 | 156 | ) | 156 | ) |
45 | 157 | 157 | ||
46 | 158 | @skip("LP:1840491") | ||
47 | 158 | def test_GET_nodes_query_count(self): | 159 | def test_GET_nodes_query_count(self): |
48 | 159 | # Patch middleware so it does not affect query counting. | 160 | # Patch middleware so it does not affect query counting. |
49 | 160 | self.patch( | 161 | self.patch( |
50 | @@ -233,6 +234,7 @@ class TestTagAPI(APITestCase.ForUser): | |||
51 | 233 | [machine.system_id], [r["system_id"] for r in parsed_result] | 234 | [machine.system_id], [r["system_id"] for r in parsed_result] |
52 | 234 | ) | 235 | ) |
53 | 235 | 236 | ||
54 | 237 | @skip("LP:1840491") | ||
55 | 236 | def test_GET_machines_query_count(self): | 238 | def test_GET_machines_query_count(self): |
56 | 237 | # Patch middleware so it does not affect query counting. | 239 | # Patch middleware so it does not affect query counting. |
57 | 238 | self.patch( | 240 | self.patch( |
58 | diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py | |||
59 | index a68511f..62e1ecd 100644 | |||
60 | --- a/src/maasserver/forms/__init__.py | |||
61 | +++ b/src/maasserver/forms/__init__.py | |||
62 | @@ -152,6 +152,7 @@ from maasserver.models import ( | |||
63 | 152 | ) | 152 | ) |
64 | 153 | from maasserver.models.blockdevice import MIN_BLOCK_DEVICE_SIZE | 153 | from maasserver.models.blockdevice import MIN_BLOCK_DEVICE_SIZE |
65 | 154 | from maasserver.models.partition import MIN_PARTITION_SIZE | 154 | from maasserver.models.partition import MIN_PARTITION_SIZE |
66 | 155 | from maasserver.models.partitiontable import PARTITION_TABLE_TYPE_CHOICES | ||
67 | 155 | from maasserver.permissions import NodePermission, ResourcePoolPermission | 156 | from maasserver.permissions import NodePermission, ResourcePoolPermission |
68 | 156 | from maasserver.storage_layouts import VMFS6StorageLayout | 157 | from maasserver.storage_layouts import VMFS6StorageLayout |
69 | 157 | from maasserver.utils.converters import machine_readable_bytes | 158 | from maasserver.utils.converters import machine_readable_bytes |
70 | @@ -2869,6 +2870,30 @@ class NUMANodeFormMixin: | |||
71 | 2869 | return self.cleaned_data["numa_node"] | 2870 | return self.cleaned_data["numa_node"] |
72 | 2870 | 2871 | ||
73 | 2871 | 2872 | ||
74 | 2873 | class UpdateBlockDevicePartitionTableTypeFormMixin: | ||
75 | 2874 | """Mixin form class for updating partition table type for a block device. | ||
76 | 2875 | |||
77 | 2876 | The form using this mixin should define a partition_table_type field and | ||
78 | 2877 | call the save() from this class in its own save. | ||
79 | 2878 | |||
80 | 2879 | """ | ||
81 | 2880 | |||
82 | 2881 | def clean_partition_table_type(self): | ||
83 | 2882 | table_type = self.cleaned_data.get("partition_table_type") | ||
84 | 2883 | if not table_type: | ||
85 | 2884 | return | ||
86 | 2885 | if not self.instance.get_partitiontable(): | ||
87 | 2886 | raise ValidationError("Block device has no partition table") | ||
88 | 2887 | return table_type | ||
89 | 2888 | |||
90 | 2889 | def save(self): | ||
91 | 2890 | table_type = self.cleaned_data.get("partition_table_type") | ||
92 | 2891 | if table_type: | ||
93 | 2892 | part_table = self.instance.get_partitiontable() | ||
94 | 2893 | part_table.table_type = table_type | ||
95 | 2894 | part_table.save() | ||
96 | 2895 | |||
97 | 2896 | |||
98 | 2872 | class CreatePhysicalBlockDeviceForm(MAASModelForm, NUMANodeFormMixin): | 2897 | class CreatePhysicalBlockDeviceForm(MAASModelForm, NUMANodeFormMixin): |
99 | 2873 | """For creating physical block device.""" | 2898 | """For creating physical block device.""" |
100 | 2874 | 2899 | ||
101 | @@ -2903,7 +2928,11 @@ class CreatePhysicalBlockDeviceForm(MAASModelForm, NUMANodeFormMixin): | |||
102 | 2903 | return block_device | 2928 | return block_device |
103 | 2904 | 2929 | ||
104 | 2905 | 2930 | ||
106 | 2906 | class UpdatePhysicalBlockDeviceForm(MAASModelForm, NUMANodeFormMixin): | 2931 | class UpdatePhysicalBlockDeviceForm( |
107 | 2932 | MAASModelForm, | ||
108 | 2933 | NUMANodeFormMixin, | ||
109 | 2934 | UpdateBlockDevicePartitionTableTypeFormMixin, | ||
110 | 2935 | ): | ||
111 | 2907 | """For updating physical block device.""" | 2936 | """For updating physical block device.""" |
112 | 2908 | 2937 | ||
113 | 2909 | name = forms.CharField(required=False) | 2938 | name = forms.CharField(required=False) |
114 | @@ -2913,6 +2942,9 @@ class UpdatePhysicalBlockDeviceForm(MAASModelForm, NUMANodeFormMixin): | |||
115 | 2913 | numa_node = forms.IntegerField( | 2942 | numa_node = forms.IntegerField( |
116 | 2914 | required=False, initial=0, min_value=0, label="NUMA node" | 2943 | required=False, initial=0, min_value=0, label="NUMA node" |
117 | 2915 | ) | 2944 | ) |
118 | 2945 | partition_table_type = forms.ChoiceField( | ||
119 | 2946 | required=False, choices=PARTITION_TABLE_TYPE_CHOICES | ||
120 | 2947 | ) | ||
121 | 2916 | 2948 | ||
122 | 2917 | class Meta: | 2949 | class Meta: |
123 | 2918 | model = PhysicalBlockDevice | 2950 | model = PhysicalBlockDevice |
124 | @@ -2935,6 +2967,11 @@ class UpdatePhysicalBlockDeviceForm(MAASModelForm, NUMANodeFormMixin): | |||
125 | 2935 | # needed by NUMANodeForm | 2967 | # needed by NUMANodeForm |
126 | 2936 | return self.instance.node | 2968 | return self.instance.node |
127 | 2937 | 2969 | ||
128 | 2970 | def save(self): | ||
129 | 2971 | block_device = super().save() | ||
130 | 2972 | UpdateBlockDevicePartitionTableTypeFormMixin.save(self) | ||
131 | 2973 | return block_device | ||
132 | 2974 | |||
133 | 2938 | 2975 | ||
134 | 2939 | class UpdateDeployedPhysicalBlockDeviceForm(MAASModelForm): | 2976 | class UpdateDeployedPhysicalBlockDeviceForm(MAASModelForm): |
135 | 2940 | """For updating physical block device on deployed machine.""" | 2977 | """For updating physical block device on deployed machine.""" |
136 | @@ -2947,12 +2984,17 @@ class UpdateDeployedPhysicalBlockDeviceForm(MAASModelForm): | |||
137 | 2947 | fields = ["name", "model", "serial", "id_path"] | 2984 | fields = ["name", "model", "serial", "id_path"] |
138 | 2948 | 2985 | ||
139 | 2949 | 2986 | ||
141 | 2950 | class UpdateVirtualBlockDeviceForm(MAASModelForm): | 2987 | class UpdateVirtualBlockDeviceForm( |
142 | 2988 | MAASModelForm, UpdateBlockDevicePartitionTableTypeFormMixin | ||
143 | 2989 | ): | ||
144 | 2951 | """For updating virtual block device.""" | 2990 | """For updating virtual block device.""" |
145 | 2952 | 2991 | ||
146 | 2953 | name = forms.CharField(required=False) | 2992 | name = forms.CharField(required=False) |
147 | 2954 | uuid = UUID4Field(required=False) | 2993 | uuid = UUID4Field(required=False) |
148 | 2955 | size = BytesField(required=False) | 2994 | size = BytesField(required=False) |
149 | 2995 | partition_table_type = forms.ChoiceField( | ||
150 | 2996 | required=False, choices=PARTITION_TABLE_TYPE_CHOICES | ||
151 | 2997 | ) | ||
152 | 2956 | 2998 | ||
153 | 2957 | class Meta: | 2999 | class Meta: |
154 | 2958 | model = VirtualBlockDevice | 3000 | model = VirtualBlockDevice |
155 | diff --git a/src/maasserver/forms/tests/test_blockdevice.py b/src/maasserver/forms/tests/test_blockdevice.py | |||
156 | index 0526b10..43c235a 100644 | |||
157 | --- a/src/maasserver/forms/tests/test_blockdevice.py | |||
158 | +++ b/src/maasserver/forms/tests/test_blockdevice.py | |||
159 | @@ -22,6 +22,7 @@ from maasserver.forms import ( | |||
160 | 22 | from maasserver.models import Filesystem | 22 | from maasserver.models import Filesystem |
161 | 23 | from maasserver.models.blockdevice import MIN_BLOCK_DEVICE_SIZE | 23 | from maasserver.models.blockdevice import MIN_BLOCK_DEVICE_SIZE |
162 | 24 | from maasserver.models.partition import PARTITION_ALIGNMENT_SIZE | 24 | from maasserver.models.partition import PARTITION_ALIGNMENT_SIZE |
163 | 25 | from maasserver.models.partitiontable import PARTITION_TABLE_TYPE | ||
164 | 25 | from maasserver.testing.factory import factory | 26 | from maasserver.testing.factory import factory |
165 | 26 | from maasserver.testing.testcase import MAASServerTestCase | 27 | from maasserver.testing.testcase import MAASServerTestCase |
166 | 27 | from maasserver.utils.converters import round_size_to_nearest_block | 28 | from maasserver.utils.converters import round_size_to_nearest_block |
167 | @@ -351,6 +352,35 @@ class TestUpdatePhysicalBlockDeviceForm(MAASServerTestCase): | |||
168 | 351 | block_device = form.save() | 352 | block_device = form.save() |
169 | 352 | self.assertEqual(block_device.numa_node, numa_node) | 353 | self.assertEqual(block_device.numa_node, numa_node) |
170 | 353 | 354 | ||
171 | 355 | def test_udpate_partitiontable_type(self): | ||
172 | 356 | block_device = factory.make_PhysicalBlockDevice() | ||
173 | 357 | factory.make_PartitionTable( | ||
174 | 358 | table_type=PARTITION_TABLE_TYPE.GPT, block_device=block_device | ||
175 | 359 | ) | ||
176 | 360 | form = UpdatePhysicalBlockDeviceForm( | ||
177 | 361 | instance=block_device, | ||
178 | 362 | data={"partition_table_type": PARTITION_TABLE_TYPE.MBR}, | ||
179 | 363 | ) | ||
180 | 364 | self.assertTrue(form.is_valid(), form.errors) | ||
181 | 365 | block_device = form.save() | ||
182 | 366 | self.assertEqual( | ||
183 | 367 | block_device.get_partitiontable().table_type, | ||
184 | 368 | PARTITION_TABLE_TYPE.MBR, | ||
185 | 369 | ) | ||
186 | 370 | |||
187 | 371 | def test_udpate_partitiontable_type_no_table(self): | ||
188 | 372 | block_device = factory.make_PhysicalBlockDevice() | ||
189 | 373 | self.assertIsNone(block_device.get_partitiontable()) | ||
190 | 374 | form = UpdatePhysicalBlockDeviceForm( | ||
191 | 375 | instance=block_device, | ||
192 | 376 | data={"partition_table_type": PARTITION_TABLE_TYPE.MBR}, | ||
193 | 377 | ) | ||
194 | 378 | self.assertFalse(form.is_valid()) | ||
195 | 379 | self.assertEqual( | ||
196 | 380 | form.errors["partition_table_type"], | ||
197 | 381 | ["Block device has no partition table"], | ||
198 | 382 | ) | ||
199 | 383 | |||
200 | 354 | 384 | ||
201 | 355 | class TestUpdateDeployedPhysicalBlockDeviceForm(MAASServerTestCase): | 385 | class TestUpdateDeployedPhysicalBlockDeviceForm(MAASServerTestCase): |
202 | 356 | def test_requires_no_fields(self): | 386 | def test_requires_no_fields(self): |
203 | diff --git a/src/maasserver/migrations/maasserver/0209_default_partitiontable_gpt.py b/src/maasserver/migrations/maasserver/0209_default_partitiontable_gpt.py | |||
204 | 357 | new file mode 100644 | 387 | new file mode 100644 |
205 | index 0000000..3a153f9 | |||
206 | --- /dev/null | |||
207 | +++ b/src/maasserver/migrations/maasserver/0209_default_partitiontable_gpt.py | |||
208 | @@ -0,0 +1,25 @@ | |||
209 | 1 | # -*- coding: utf-8 -*- | ||
210 | 2 | # Generated by Django 1.11.11 on 2020-05-07 16:08 | ||
211 | 3 | from __future__ import unicode_literals | ||
212 | 4 | |||
213 | 5 | from django.db import migrations, models | ||
214 | 6 | |||
215 | 7 | |||
216 | 8 | class Migration(migrations.Migration): | ||
217 | 9 | |||
218 | 10 | dependencies = [("maasserver", "0208_no_power_query_events")] | ||
219 | 11 | |||
220 | 12 | operations = [ | ||
221 | 13 | migrations.AlterField( | ||
222 | 14 | model_name="partitiontable", | ||
223 | 15 | name="table_type", | ||
224 | 16 | field=models.CharField( | ||
225 | 17 | choices=[ | ||
226 | 18 | ("MBR", "Master boot record"), | ||
227 | 19 | ("GPT", "GUID parition table"), | ||
228 | 20 | ], | ||
229 | 21 | default="GPT", | ||
230 | 22 | max_length=20, | ||
231 | 23 | ), | ||
232 | 24 | ) | ||
233 | 25 | ] | ||
234 | diff --git a/src/maasserver/models/partitiontable.py b/src/maasserver/models/partitiontable.py | |||
235 | index 7bdac7d..b238c4b 100644 | |||
236 | --- a/src/maasserver/models/partitiontable.py | |||
237 | +++ b/src/maasserver/models/partitiontable.py | |||
238 | @@ -41,13 +41,9 @@ PARTITION_TABLE_EXTRA_SPACE = ( | |||
239 | 41 | # on ppc64el and will fail to boot. | 41 | # on ppc64el and will fail to boot. |
240 | 42 | PREP_PARTITION_SIZE = 8 * 1024 * 1024 # 8MiB | 42 | PREP_PARTITION_SIZE = 8 * 1024 * 1024 # 8MiB |
241 | 43 | 43 | ||
242 | 44 | # 2 TiB disks require GPT partition tables so the whole disk can be used. MBR | ||
243 | 45 | # is forced on the boot disk unless the disk is larger than 2TiB. | ||
244 | 46 | GPT_REQUIRED_SIZE = 2 * 1024 * 1024 * 1024 * 1024 | ||
245 | 47 | |||
246 | 48 | # The amount of space required to be reserved for the bios_grub partition. | 44 | # The amount of space required to be reserved for the bios_grub partition. |
247 | 49 | # bios_grub partition is required on amd64 architectures when grub is used | 45 | # bios_grub partition is required on amd64 architectures when grub is used |
249 | 50 | # on the boot disk and the disk is larger than GPT_REQUIRED_SIZE. | 46 | # on the boot disk with GPT. |
250 | 51 | BIOS_GRUB_PARTITION_SIZE = 1 * 1024 * 1024 # 1MiB | 47 | BIOS_GRUB_PARTITION_SIZE = 1 * 1024 * 1024 # 1MiB |
251 | 52 | 48 | ||
252 | 53 | 49 | ||
253 | @@ -62,7 +58,9 @@ class PartitionTable(CleanSave, TimestampedModel): | |||
254 | 62 | """Needed for South to recognize this model.""" | 58 | """Needed for South to recognize this model.""" |
255 | 63 | 59 | ||
256 | 64 | table_type = CharField( | 60 | table_type = CharField( |
258 | 65 | max_length=20, choices=PARTITION_TABLE_TYPE_CHOICES, default=None | 61 | max_length=20, |
259 | 62 | choices=PARTITION_TABLE_TYPE_CHOICES, | ||
260 | 63 | default=PARTITION_TABLE_TYPE.GPT, | ||
261 | 66 | ) | 64 | ) |
262 | 67 | 65 | ||
263 | 68 | block_device = ForeignKey( | 66 | block_device = ForeignKey( |
264 | @@ -95,7 +93,6 @@ class PartitionTable(CleanSave, TimestampedModel): | |||
265 | 95 | elif ( | 93 | elif ( |
266 | 96 | node_arch == "amd64" | 94 | node_arch == "amd64" |
267 | 97 | and self.block_device.node.bios_boot_method != "uefi" | 95 | and self.block_device.node.bios_boot_method != "uefi" |
268 | 98 | and self.block_device.size >= GPT_REQUIRED_SIZE | ||
269 | 99 | ): | 96 | ): |
270 | 100 | extra_space += BIOS_GRUB_PARTITION_SIZE | 97 | extra_space += BIOS_GRUB_PARTITION_SIZE |
271 | 101 | return extra_space | 98 | return extra_space |
272 | @@ -147,55 +144,6 @@ class PartitionTable(CleanSave, TimestampedModel): | |||
273 | 147 | def __str__(self): | 144 | def __str__(self): |
274 | 148 | return "Partition table for {bd}".format(bd=self.block_device) | 145 | return "Partition table for {bd}".format(bd=self.block_device) |
275 | 149 | 146 | ||
276 | 150 | def save(self, *args, **kwargs): | ||
277 | 151 | self._set_and_validate_table_type_for_boot_disk() | ||
278 | 152 | return super(PartitionTable, self).save(*args, **kwargs) | ||
279 | 153 | |||
280 | 154 | def _set_and_validate_table_type_for_boot_disk(self): | ||
281 | 155 | """Validates or set the table type if this partition table is on the | ||
282 | 156 | boot disk for a node.""" | ||
283 | 157 | if ( | ||
284 | 158 | self.block_device is not None | ||
285 | 159 | and self.block_device.node is not None | ||
286 | 160 | ): | ||
287 | 161 | node = self.block_device.node | ||
288 | 162 | boot_disk = node.get_boot_disk() | ||
289 | 163 | # Compare the block_device.id and boot_disk.id because it is | ||
290 | 164 | # possible they are not the same type. One being an instance | ||
291 | 165 | # of PhysicalBlockDevice and the other being just a BlockDevice. | ||
292 | 166 | # Without this comparison the wrong partition table type will be | ||
293 | 167 | # placed on the boot disk. | ||
294 | 168 | if boot_disk is not None and self.block_device.id == boot_disk.id: | ||
295 | 169 | bios_boot_method = node.get_bios_boot_method() | ||
296 | 170 | if bios_boot_method in ["uefi", "powernv", "powerkvm"]: | ||
297 | 171 | # UEFI, PowerNV, or PowerKVM must always use a GPT table. | ||
298 | 172 | if not self.table_type: | ||
299 | 173 | self.table_type = PARTITION_TABLE_TYPE.GPT | ||
300 | 174 | elif self.table_type != PARTITION_TABLE_TYPE.GPT: | ||
301 | 175 | raise ValidationError( | ||
302 | 176 | { | ||
303 | 177 | "table_type": [ | ||
304 | 178 | "Partition table on this node's boot disk " | ||
305 | 179 | "must be using '%s'." | ||
306 | 180 | % (PARTITION_TABLE_TYPE.GPT) | ||
307 | 181 | ] | ||
308 | 182 | } | ||
309 | 183 | ) | ||
310 | 184 | else: | ||
311 | 185 | # Don't even check if its 'pxe', because we always fallback | ||
312 | 186 | # to MBR unless the disk is larger than 2TiB in that case | ||
313 | 187 | # it is GPT. | ||
314 | 188 | disk_size = self.block_device.size | ||
315 | 189 | if not self.table_type: | ||
316 | 190 | if disk_size >= GPT_REQUIRED_SIZE: | ||
317 | 191 | self.table_type = PARTITION_TABLE_TYPE.GPT | ||
318 | 192 | else: | ||
319 | 193 | self.table_type = PARTITION_TABLE_TYPE.MBR | ||
320 | 194 | |||
321 | 195 | # Force GPT for everything else. | ||
322 | 196 | if not self.table_type: | ||
323 | 197 | self.table_type = PARTITION_TABLE_TYPE.GPT | ||
324 | 198 | |||
325 | 199 | def clean(self, *args, **kwargs): | 147 | def clean(self, *args, **kwargs): |
326 | 200 | super(PartitionTable, self).clean(*args, **kwargs) | 148 | super(PartitionTable, self).clean(*args, **kwargs) |
327 | 201 | # Circular imports. | 149 | # Circular imports. |
328 | diff --git a/src/maasserver/models/tests/test_partition.py b/src/maasserver/models/tests/test_partition.py | |||
329 | index 8c466f2..f431393 100644 | |||
330 | --- a/src/maasserver/models/tests/test_partition.py | |||
331 | +++ b/src/maasserver/models/tests/test_partition.py | |||
332 | @@ -418,17 +418,18 @@ class TestPartition(MAASServerTestCase): | |||
333 | 418 | for _ in range(4) | 418 | for _ in range(4) |
334 | 419 | ] | 419 | ] |
335 | 420 | idx = 1 | 420 | idx = 1 |
339 | 421 | for partition in partitions: | 421 | for idx, partition in enumerate(partitions, 1): |
340 | 422 | self.expectThat(idx, Equals(partition.get_partition_number())) | 422 | self.assertEqual(partition.get_partition_number(), idx) |
338 | 423 | idx += 1 | ||
341 | 424 | 423 | ||
343 | 425 | def test_get_partition_number_starting_at_2_for_amd64_not_gpt(self): | 424 | def test_get_partition_number_starting_at_2_for_amd64_not_uefi(self): |
344 | 426 | node = factory.make_Node( | 425 | node = factory.make_Node( |
345 | 427 | bios_boot_method="pxe", architecture="amd64/generic" | 426 | bios_boot_method="pxe", architecture="amd64/generic" |
346 | 428 | ) | 427 | ) |
347 | 429 | block_device = factory.make_PhysicalBlockDevice( | 428 | block_device = factory.make_PhysicalBlockDevice( |
348 | 430 | node=node, | 429 | node=node, |
350 | 431 | size=(MIN_PARTITION_SIZE * 4) + PARTITION_TABLE_EXTRA_SPACE, | 430 | size=(MIN_PARTITION_SIZE * 4) |
351 | 431 | + PARTITION_TABLE_EXTRA_SPACE | ||
352 | 432 | + BIOS_GRUB_PARTITION_SIZE, | ||
353 | 432 | ) | 433 | ) |
354 | 433 | partition_table = factory.make_PartitionTable( | 434 | partition_table = factory.make_PartitionTable( |
355 | 434 | block_device=block_device, table_type=PARTITION_TABLE_TYPE.GPT | 435 | block_device=block_device, table_type=PARTITION_TABLE_TYPE.GPT |
356 | diff --git a/src/maasserver/models/tests/test_partitiontable.py b/src/maasserver/models/tests/test_partitiontable.py | |||
357 | index 699a278..0fb0259 100644 | |||
358 | --- a/src/maasserver/models/tests/test_partitiontable.py | |||
359 | +++ b/src/maasserver/models/tests/test_partitiontable.py | |||
360 | @@ -8,7 +8,7 @@ __all__ = [] | |||
361 | 8 | from django.core.exceptions import ValidationError | 8 | from django.core.exceptions import ValidationError |
362 | 9 | 9 | ||
363 | 10 | from maasserver.enum import FILESYSTEM_GROUP_TYPE, PARTITION_TABLE_TYPE | 10 | from maasserver.enum import FILESYSTEM_GROUP_TYPE, PARTITION_TABLE_TYPE |
365 | 11 | from maasserver.models.blockdevice import BlockDevice, MIN_BLOCK_DEVICE_SIZE | 11 | from maasserver.models.blockdevice import MIN_BLOCK_DEVICE_SIZE |
366 | 12 | from maasserver.models.partition import ( | 12 | from maasserver.models.partition import ( |
367 | 13 | MAX_PARTITION_SIZE_FOR_MBR, | 13 | MAX_PARTITION_SIZE_FOR_MBR, |
368 | 14 | MIN_PARTITION_SIZE, | 14 | MIN_PARTITION_SIZE, |
369 | @@ -224,91 +224,14 @@ class TestPartitionTable(MAASServerTestCase): | |||
370 | 224 | ), | 224 | ), |
371 | 225 | ) | 225 | ) |
372 | 226 | 226 | ||
406 | 227 | def test_save_sets_table_type_to_mbr_for_boot_when_type_miss_match(self): | 227 | def test_save_sets_table_type_to_MBR_for_arm64(self): |
374 | 228 | node = factory.make_Node(with_boot_disk=False, bios_boot_method="pxe") | ||
375 | 229 | boot_disk = factory.make_PhysicalBlockDevice(node=node) | ||
376 | 230 | partition_table = factory.make_PartitionTable( | ||
377 | 231 | block_device=BlockDevice.objects.get(id=boot_disk.id) | ||
378 | 232 | ) | ||
379 | 233 | self.assertEqual(PARTITION_TABLE_TYPE.MBR, partition_table.table_type) | ||
380 | 234 | |||
381 | 235 | def test_save_sets_table_type_to_gpt_for_2tib_boot(self): | ||
382 | 236 | node = factory.make_Node(with_boot_disk=False, bios_boot_method="pxe") | ||
383 | 237 | boot_disk = factory.make_PhysicalBlockDevice( | ||
384 | 238 | node=node, size=2 * 1024 * 1024 * 1024 * 1024 | ||
385 | 239 | ) | ||
386 | 240 | partition_table = factory.make_PartitionTable( | ||
387 | 241 | block_device=BlockDevice.objects.get(id=boot_disk.id) | ||
388 | 242 | ) | ||
389 | 243 | self.assertEqual(PARTITION_TABLE_TYPE.GPT, partition_table.table_type) | ||
390 | 244 | |||
391 | 245 | def test_save_sets_table_type_to_gpt_for_uefi_boot(self): | ||
392 | 246 | node = factory.make_Node(with_boot_disk=False, bios_boot_method="uefi") | ||
393 | 247 | boot_disk = factory.make_PhysicalBlockDevice(node=node) | ||
394 | 248 | partition_table = factory.make_PartitionTable(block_device=boot_disk) | ||
395 | 249 | self.assertEqual(PARTITION_TABLE_TYPE.GPT, partition_table.table_type) | ||
396 | 250 | |||
397 | 251 | def test_save_sets_table_type_to_gpt_for_powernv_boot(self): | ||
398 | 252 | node = factory.make_Node( | ||
399 | 253 | with_boot_disk=False, bios_boot_method="powernv" | ||
400 | 254 | ) | ||
401 | 255 | boot_disk = factory.make_PhysicalBlockDevice(node=node) | ||
402 | 256 | partition_table = factory.make_PartitionTable(block_device=boot_disk) | ||
403 | 257 | self.assertEqual(PARTITION_TABLE_TYPE.GPT, partition_table.table_type) | ||
404 | 258 | |||
405 | 259 | def test_save_sets_table_type_to_gpt_for_powerkvm_boot(self): | ||
407 | 260 | node = factory.make_Node( | 228 | node = factory.make_Node( |
409 | 261 | with_boot_disk=False, bios_boot_method="powerkvm" | 229 | architecture="arm64/generic", with_boot_disk=False |
410 | 262 | ) | 230 | ) |
411 | 263 | boot_disk = factory.make_PhysicalBlockDevice(node=node) | 231 | boot_disk = factory.make_PhysicalBlockDevice(node=node) |
412 | 264 | partition_table = factory.make_PartitionTable(block_device=boot_disk) | 232 | partition_table = factory.make_PartitionTable(block_device=boot_disk) |
413 | 265 | self.assertEqual(PARTITION_TABLE_TYPE.GPT, partition_table.table_type) | 233 | self.assertEqual(PARTITION_TABLE_TYPE.GPT, partition_table.table_type) |
414 | 266 | 234 | ||
415 | 267 | def test_save_sets_table_type_to_gpt_for_none_boot_disk(self): | ||
416 | 268 | node = factory.make_Node(with_boot_disk=False, bios_boot_method="pxe") | ||
417 | 269 | factory.make_PhysicalBlockDevice(node=node) | ||
418 | 270 | other_disk = factory.make_PhysicalBlockDevice(node=node) | ||
419 | 271 | partition_table = factory.make_PartitionTable(block_device=other_disk) | ||
420 | 272 | self.assertEqual(PARTITION_TABLE_TYPE.GPT, partition_table.table_type) | ||
421 | 273 | |||
422 | 274 | def test_save_allows_gpt_on_2tib_boot_disk_pxe(self): | ||
423 | 275 | node = factory.make_Node(with_boot_disk=False, bios_boot_method="pxe") | ||
424 | 276 | boot_disk = factory.make_PhysicalBlockDevice( | ||
425 | 277 | node=node, size=2 * 1024 * 1024 * 1024 * 1024 | ||
426 | 278 | ) | ||
427 | 279 | # ValidationError should not be raised. | ||
428 | 280 | factory.make_PartitionTable( | ||
429 | 281 | table_type=PARTITION_TABLE_TYPE.GPT, block_device=boot_disk | ||
430 | 282 | ) | ||
431 | 283 | |||
432 | 284 | def test_save_force_mbr_on_boot_disk_pxe_force_gpt_on_boot_disk_uefi(self): | ||
433 | 285 | node = factory.make_Node(with_boot_disk=False, bios_boot_method="uefi") | ||
434 | 286 | boot_disk = factory.make_PhysicalBlockDevice(node=node) | ||
435 | 287 | error = self.assertRaises( | ||
436 | 288 | ValidationError, | ||
437 | 289 | factory.make_PartitionTable, | ||
438 | 290 | table_type=PARTITION_TABLE_TYPE.MBR, | ||
439 | 291 | block_device=boot_disk, | ||
440 | 292 | ) | ||
441 | 293 | self.assertEqual( | ||
442 | 294 | { | ||
443 | 295 | "table_type": [ | ||
444 | 296 | "Partition table on this node's boot disk must " | ||
445 | 297 | "be using 'GPT'." | ||
446 | 298 | ] | ||
447 | 299 | }, | ||
448 | 300 | error.message_dict, | ||
449 | 301 | ) | ||
450 | 302 | |||
451 | 303 | def test_save_no_force_on_none_boot_disk(self): | ||
452 | 304 | node = factory.make_Node(bios_boot_method="uefi") | ||
453 | 305 | factory.make_PhysicalBlockDevice(node=node) | ||
454 | 306 | other_disk = factory.make_PhysicalBlockDevice(node=node) | ||
455 | 307 | # No error should be raised. | ||
456 | 308 | factory.make_PartitionTable( | ||
457 | 309 | table_type=PARTITION_TABLE_TYPE.MBR, block_device=other_disk | ||
458 | 310 | ) | ||
459 | 311 | |||
460 | 312 | def test_clean_no_partition_table_on_logical_volume(self): | 235 | def test_clean_no_partition_table_on_logical_volume(self): |
461 | 313 | node = factory.make_Node() | 236 | node = factory.make_Node() |
462 | 314 | virtual_device = factory.make_VirtualBlockDevice(node=node) | 237 | virtual_device = factory.make_VirtualBlockDevice(node=node) |
463 | diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py | |||
464 | index 8f0d923..f5021e1 100644 | |||
465 | --- a/src/maasserver/testing/factory.py | |||
466 | +++ b/src/maasserver/testing/factory.py | |||
467 | @@ -2643,7 +2643,7 @@ class Factory(maastesting.factory.Factory): | |||
468 | 2643 | 2643 | ||
469 | 2644 | def make_PartitionTable( | 2644 | def make_PartitionTable( |
470 | 2645 | self, | 2645 | self, |
472 | 2646 | table_type=None, | 2646 | table_type=PARTITION_TABLE_TYPE.GPT, |
473 | 2647 | block_device=None, | 2647 | block_device=None, |
474 | 2648 | node=None, | 2648 | node=None, |
475 | 2649 | block_device_size=None, | 2649 | block_device_size=None, |
476 | diff --git a/src/maasserver/tests/test_preseed_storage.py b/src/maasserver/tests/test_preseed_storage.py | |||
477 | index 2593e12..7539f59 100644 | |||
478 | --- a/src/maasserver/tests/test_preseed_storage.py | |||
479 | +++ b/src/maasserver/tests/test_preseed_storage.py | |||
480 | @@ -2510,6 +2510,7 @@ class TestVMFS(MAASServerTestCase, AssertStorageConfigMixin): | |||
481 | 2510 | device: sda | 2510 | device: sda |
482 | 2511 | number: 1 | 2511 | number: 1 |
483 | 2512 | type: partition | 2512 | type: partition |
484 | 2513 | offset: 4194304B | ||
485 | 2513 | size: 3145728B | 2514 | size: 3145728B |
486 | 2514 | uuid: 7f79841c-9f57-4ab7-ada2-b2774e3908a3 | 2515 | uuid: 7f79841c-9f57-4ab7-ada2-b2774e3908a3 |
487 | 2515 | wipe: superblock | 2516 | wipe: superblock |
488 | @@ -2625,6 +2626,7 @@ class TestVMFS(MAASServerTestCase, AssertStorageConfigMixin): | |||
489 | 2625 | status=NODE_STATUS.ALLOCATED, | 2626 | status=NODE_STATUS.ALLOCATED, |
490 | 2626 | architecture="amd64/generic", | 2627 | architecture="amd64/generic", |
491 | 2627 | osystem="esxi", | 2628 | osystem="esxi", |
492 | 2629 | bios_boot_method="uefi", | ||
493 | 2628 | distro_series="6.7", | 2630 | distro_series="6.7", |
494 | 2629 | with_boot_disk=False, | 2631 | with_boot_disk=False, |
495 | 2630 | ) | 2632 | ) |
496 | diff --git a/src/maasserver/tests/test_storage_layouts.py b/src/maasserver/tests/test_storage_layouts.py | |||
497 | index 1918dca..44955d0 100644 | |||
498 | --- a/src/maasserver/tests/test_storage_layouts.py | |||
499 | +++ b/src/maasserver/tests/test_storage_layouts.py | |||
500 | @@ -548,7 +548,7 @@ class TestFlatStorageLayout(MAASServerTestCase, LayoutHelpersMixin): | |||
501 | 548 | ["root_device", "root_size", "boot_size"], layout.fields.keys() | 548 | ["root_device", "root_size", "boot_size"], layout.fields.keys() |
502 | 549 | ) | 549 | ) |
503 | 550 | 550 | ||
505 | 551 | def test__creates_layout_with_mbr_defaults(self): | 551 | def test__creates_layout_with_gpt_defaults(self): |
506 | 552 | node = factory.make_Node(with_boot_disk=False) | 552 | node = factory.make_Node(with_boot_disk=False) |
507 | 553 | boot_disk = factory.make_PhysicalBlockDevice( | 553 | boot_disk = factory.make_PhysicalBlockDevice( |
508 | 554 | node=node, size=LARGE_BLOCK_DEVICE | 554 | node=node, size=LARGE_BLOCK_DEVICE |
509 | @@ -558,7 +558,7 @@ class TestFlatStorageLayout(MAASServerTestCase, LayoutHelpersMixin): | |||
510 | 558 | 558 | ||
511 | 559 | # Validate partition table. | 559 | # Validate partition table. |
512 | 560 | partition_table = boot_disk.get_partitiontable() | 560 | partition_table = boot_disk.get_partitiontable() |
514 | 561 | self.assertEqual(PARTITION_TABLE_TYPE.MBR, partition_table.table_type) | 561 | self.assertEqual(PARTITION_TABLE_TYPE.GPT, partition_table.table_type) |
515 | 562 | 562 | ||
516 | 563 | # Validate root partition. | 563 | # Validate root partition. |
517 | 564 | partitions = partition_table.partitions.order_by("id").all() | 564 | partitions = partition_table.partitions.order_by("id").all() |
518 | @@ -715,7 +715,7 @@ class TestFlatStorageLayout(MAASServerTestCase, LayoutHelpersMixin): | |||
519 | 715 | 715 | ||
520 | 716 | # Validate partition table. | 716 | # Validate partition table. |
521 | 717 | partition_table = boot_disk.get_partitiontable() | 717 | partition_table = boot_disk.get_partitiontable() |
523 | 718 | self.assertEqual(PARTITION_TABLE_TYPE.MBR, partition_table.table_type) | 718 | self.assertEqual(PARTITION_TABLE_TYPE.GPT, partition_table.table_type) |
524 | 719 | 719 | ||
525 | 720 | # Validate boot partition. | 720 | # Validate boot partition. |
526 | 721 | partitions = partition_table.partitions.order_by("id").all() | 721 | partitions = partition_table.partitions.order_by("id").all() |
527 | diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py | |||
528 | index 0af3a36..f703a5b 100644 | |||
529 | --- a/src/maasserver/websockets/handlers/tests/test_machine.py | |||
530 | +++ b/src/maasserver/websockets/handlers/tests/test_machine.py | |||
531 | @@ -674,7 +674,7 @@ class TestMachineHandler(MAASServerTestCase): | |||
532 | 674 | # and slowing down the client waiting for the response. | 674 | # and slowing down the client waiting for the response. |
533 | 675 | self.assertEqual( | 675 | self.assertEqual( |
534 | 676 | queries, | 676 | queries, |
536 | 677 | 55, | 677 | 76, |
537 | 678 | "Number of queries has changed; make sure this is expected.", | 678 | "Number of queries has changed; make sure this is expected.", |
538 | 679 | ) | 679 | ) |
539 | 680 | 680 |
UNIT TESTS
-b storage-gpt-default lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED maas-ci. internal: 8080/job/ maas/job/ branch- tester/ 7500/console e2d382b3f34caad c3d765e8a3
LOG: http://
COMMIT: c378ba41c703830