Merge ~andreserl/maas:ipmi_boot_type_2 into maas:master

Proposed by Andres Rodriguez
Status: Superseded
Proposed branch: ~andreserl/maas:ipmi_boot_type_2
Merge into: maas:master
Prerequisite: ~andreserl/maas:ipmi_boot_type
Diff against target: 116 lines (+21/-11)
5 files modified
src/maasserver/api/tests/test_enlistment.py (+1/-1)
src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py (+2/-2)
src/metadataserver/user_data/templates/snippets/tests/test_maas_ipmi_autodetect.py (+2/-2)
src/provisioningserver/drivers/power/ipmi.py (+13/-5)
src/provisioningserver/drivers/power/tests/test_ipmi.py (+3/-1)
Reviewer Review Type Date Requested Status
MAAS Lander Needs Fixing
Newell Jensen (community) Approve
Review via email: mp+342119@code.launchpad.net

This proposal supersedes a proposal from 2018-03-26.

Commit message

Change the IPMI_BOOT_TYPE options to be more user friendly.

This allows to specify the boot type with generalized options instead of forcing the user of freeipmi-tools terminology.

To post a comment you must log in.
Revision history for this message
Newell Jensen (newell-jensen) wrote :

Looks good. One inline comment.

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

UNIT TESTS
-b ipmi_boot_type_2 lp:~andreserl/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/2081/console
COMMIT: 7d9a04fb65d162db82df511cd70fdd1064911a08

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

Unmerged commits

7d9a04f... by Andres Rodriguez

Change the options for power_boot_type so that API users can specify them by using auto|legacy|efi instead of freeipmi-tools specific <null>|EFI|PC-COMPATIBLE

f5180d0... by Andres Rodriguez

Fix lint

87fc9dd... by Andres Rodriguez

Add missing tests

8a1cb40... by Andres Rodriguez

Fix tests

e9ffd06... by Andres Rodriguez

Fix tests

bf35c2f... by Andres Rodriguez

LP: #1750622 - Add ability to force the BIOS boot method, and auto discover it during enlistment/commissioning

2b2a439... by Andres Rodriguez

Fix lint

07d5287... by Andres Rodriguez

Fix review comments

36cdc4b... by Andres Rodriguez

Fix test

9e45488... by Andres Rodriguez

Add/fix tests

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/api/tests/test_enlistment.py b/src/maasserver/api/tests/test_enlistment.py
2index 5b636fb..2f03848 100644
3--- a/src/maasserver/api/tests/test_enlistment.py
4+++ b/src/maasserver/api/tests/test_enlistment.py
5@@ -115,7 +115,7 @@ class EnlistmentAPITest(APITestCase.ForAnonymousAndUserAndAdmin):
6 # Add the default values.
7 power_parameters['power_driver'] = 'LAN_2_0'
8 power_parameters['mac_address'] = ''
9- power_parameters['power_boot_type'] = ''
10+ power_parameters['power_boot_type'] = 'auto'
11 self.assertEqual(http.client.OK, response.status_code)
12 [machine] = Machine.objects.filter(hostname=hostname)
13 self.assertEqual(power_parameters, machine.power_parameters)
14diff --git a/src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py b/src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py
15index a2caa8e..72e2496 100644
16--- a/src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py
17+++ b/src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py
18@@ -321,8 +321,8 @@ def bmc_supports_lan2_0():
19 def get_system_boot_type():
20 """Detect if the system has boot EFI."""
21 if os.path.isdir('/sys/firmware/efi'):
22- return 'EFI'
23- return ''
24+ return 'efi'
25+ return 'auto'
26
27
28 def main():
29diff --git a/src/metadataserver/user_data/templates/snippets/tests/test_maas_ipmi_autodetect.py b/src/metadataserver/user_data/templates/snippets/tests/test_maas_ipmi_autodetect.py
30index 37f52b6..86803eb 100644
31--- a/src/metadataserver/user_data/templates/snippets/tests/test_maas_ipmi_autodetect.py
32+++ b/src/metadataserver/user_data/templates/snippets/tests/test_maas_ipmi_autodetect.py
33@@ -726,7 +726,7 @@ class TestGetSystemBootType(MAASTestCase):
34
35 def test_get_system_boot_type_efi(self):
36 """Test that returns ."""
37- boot_type = 'EFI'
38+ boot_type = 'efi'
39 # path os.path.isdir to return True to simulate
40 # that /sys/firmware/efi exists.
41 self.patch(os.path, "isdir").return_value = True
42@@ -735,7 +735,7 @@ class TestGetSystemBootType(MAASTestCase):
43
44 def test_get_system_boot_type_non_efi(self):
45 """Test """
46- boot_type = ''
47+ boot_type = 'auto'
48 # path os.path.isdir to return False to simulate
49 # that /sys/firmware/efi doesn't exist.
50 self.patch(os.path, "isdir").return_value = False
51diff --git a/src/provisioningserver/drivers/power/ipmi.py b/src/provisioningserver/drivers/power/ipmi.py
52index 84835e1..ee5b413 100644
53--- a/src/provisioningserver/drivers/power/ipmi.py
54+++ b/src/provisioningserver/drivers/power/ipmi.py
55@@ -169,9 +169,9 @@ IPMI_DRIVER_CHOICES = [
56
57 class IPMI_BOOT_TYPE:
58 # DEFAULT used to provide backwards compatibility
59- DEFAULT = ''
60- LEGACY = 'PC-COMPATIBLE'
61- EFI = 'EFI'
62+ DEFAULT = 'auto'
63+ LEGACY = 'legacy'
64+ EFI = 'efi'
65
66
67 IPMI_BOOT_TYPE_CHOICES = [
68@@ -180,6 +180,11 @@ IPMI_BOOT_TYPE_CHOICES = [
69 [IPMI_BOOT_TYPE.EFI, "EFI boot"]
70 ]
71
72+IPMI_BOOT_TYPE_MAPPING = {
73+ IPMI_BOOT_TYPE.EFI: 'EFI',
74+ IPMI_BOOT_TYPE.LEGACY: 'PC-COMPATIBLE',
75+ }
76+
77
78 class IPMIPowerDriver(PowerDriver):
79
80@@ -216,10 +221,13 @@ class IPMIPowerDriver(PowerDriver):
81 env = shell.get_env_with_locale()
82 with NamedTemporaryFile("w+", encoding="utf-8") as tmp_config:
83 # Write out the chassis configuration.
84- if power_boot_type is None or power_boot_type == '':
85+ if (power_boot_type is None
86+ or power_boot_type == IPMI_BOOT_TYPE.DEFAULT):
87 tmp_config.write(IPMI_CONFIG)
88 else:
89- tmp_config.write(IPMI_CONFIG_WITH_BOOT_TYPE % power_boot_type)
90+ tmp_config.write(
91+ IPMI_CONFIG_WITH_BOOT_TYPE %
92+ IPMI_BOOT_TYPE_MAPPING[power_boot_type])
93 tmp_config.flush()
94 # Use it when running the chassis config command.
95 # XXX: Not using call_and_check here because we
96diff --git a/src/provisioningserver/drivers/power/tests/test_ipmi.py b/src/provisioningserver/drivers/power/tests/test_ipmi.py
97index 6e7a4b9..548846e 100644
98--- a/src/provisioningserver/drivers/power/tests/test_ipmi.py
99+++ b/src/provisioningserver/drivers/power/tests/test_ipmi.py
100@@ -26,6 +26,7 @@ from provisioningserver.drivers.power import (
101 )
102 from provisioningserver.drivers.power.ipmi import (
103 IPMI_BOOT_TYPE,
104+ IPMI_BOOT_TYPE_MAPPING,
105 IPMI_CONFIG,
106 IPMI_CONFIG_WITH_BOOT_TYPE,
107 IPMI_ERRORS,
108@@ -379,6 +380,7 @@ class TestIPMIPowerDriver(MAASTestCase):
109 self.assertThat(
110 tmpfile.write,
111 MockCalledOnceWith(
112- IPMI_CONFIG_WITH_BOOT_TYPE % IPMI_BOOT_TYPE.EFI))
113+ IPMI_CONFIG_WITH_BOOT_TYPE % IPMI_BOOT_TYPE_MAPPING[
114+ IPMI_BOOT_TYPE.EFI]))
115 self.assertThat(tmpfile.flush, MockCalledOnceWith())
116 self.assertThat(tmpfile.__exit__, MockCalledOnceWith(None, None, None))

Subscribers

People subscribed via source and target branches