Merge ~andreserl/maas:lp1664822_2 into maas:master

Proposed by Andres Rodriguez
Status: Merged
Approved by: Andres Rodriguez
Approved revision: af4666921684840c983209c7b28f0c66237dddb3
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~andreserl/maas:lp1664822_2
Merge into: maas:master
Diff against target: 102 lines (+29/-25)
4 files modified
contrib/preseeds_v2/enlist_userdata (+0/-12)
src/metadataserver/user_data/templates/commissioning.template (+0/-12)
src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py (+8/-1)
src/metadataserver/user_data/templates/snippets/tests/test_maas_ipmi_autodetect.py (+21/-0)
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
Review via email: mp+327396@code.launchpad.net

Commit message

LP: #1664822 - Ensure failure to set Lan_Channel doesn't cause complete script failure

To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Looks good, but missing a unit test for when bmc_set() raises.

review: Needs Fixing
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Looks good; thanks for the fix.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/contrib/preseeds_v2/enlist_userdata b/contrib/preseeds_v2/enlist_userdata
index 6085775..408a198 100644
--- a/contrib/preseeds_v2/enlist_userdata
+++ b/contrib/preseeds_v2/enlist_userdata
@@ -50,18 +50,6 @@ misc_bucket:
50 chmod "${2:-644}" "${IPMI_CONFIG_D}/$1"50 chmod "${2:-644}" "${IPMI_CONFIG_D}/$1"
51 }51 }
5252
53 # Example config: enable BMC remote access (on some systems.)
54 #add_ipmi_config "02-global-config.ipmi" <<"END_IPMI_CONFIG"
55 #Section Lan_Channel
56 # Volatile_Access_Mode Always_Available
57 # Volatile_Enable_User_Level_Auth Yes
58 # Volatile_Channel_Privilege_Limit Administrator
59 # Non_Volatile_Access_Mode Always_Available
60 # Non_Volatile_Enable_User_Level_Auth Yes
61 # Non_Volatile_Channel_Privilege_Limit Administrator
62 #EndSection
63 #END_IPMI_CONFIG
64
65 add_bin "maas-ipmi-autodetect-tool" <<"END_MAAS_IPMI_AUTODETECT_TOOL"53 add_bin "maas-ipmi-autodetect-tool" <<"END_MAAS_IPMI_AUTODETECT_TOOL"
66 {{for line in maas_ipmi_autodetect_tool_py.splitlines()}}54 {{for line in maas_ipmi_autodetect_tool_py.splitlines()}}
67 {{line}}55 {{line}}
diff --git a/src/metadataserver/user_data/templates/commissioning.template b/src/metadataserver/user_data/templates/commissioning.template
index e29c774..f0d49ab 100644
--- a/src/metadataserver/user_data/templates/commissioning.template
+++ b/src/metadataserver/user_data/templates/commissioning.template
@@ -69,18 +69,6 @@ main() {
6969
70### begin writing files ###70### begin writing files ###
7171
72# Example config: enable BMC remote access (on some systems.)
73#add_ipmi_config "02-global-config.ipmi" <<"END_IPMI_CONFIG"
74#Section Lan_Channel
75# Volatile_Access_Mode Always_Available
76# Volatile_Enable_User_Level_Auth Yes
77# Volatile_Channel_Privilege_Limit Administrator
78# Non_Volatile_Access_Mode Always_Available
79# Non_Volatile_Enable_User_Level_Auth Yes
80# Non_Volatile_Channel_Privilege_Limit Administrator
81#EndSection
82#END_IPMI_CONFIG
83
84add_bin "maas-ipmi-autodetect-tool" <<"END_MAAS_IPMI_AUTODETECT_TOOL"72add_bin "maas-ipmi-autodetect-tool" <<"END_MAAS_IPMI_AUTODETECT_TOOL"
85{{maas_ipmi_autodetect_tool_py}}73{{maas_ipmi_autodetect_tool_py}}
86END_MAAS_IPMI_AUTODETECT_TOOL74END_MAAS_IPMI_AUTODETECT_TOOL
diff --git a/src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py b/src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py
index 4fbc849..b5e2125 100644
--- a/src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py
+++ b/src/metadataserver/user_data/templates/snippets/maas_ipmi_autodetect.py
@@ -242,7 +242,14 @@ def set_ipmi_lan_channel_settings():
242 output = bmc_get(mode)242 output = bmc_get(mode)
243 show_re = re.compile('%s\s+Always_Available' % mode.split(':')[1])243 show_re = re.compile('%s\s+Always_Available' % mode.split(':')[1])
244 if show_re.search(output) is None:244 if show_re.search(output) is None:
245 bmc_set(mode, 'Always_Available')245 # Some BMC's don't support setting Lan_Channel (see LP: #1287274).
246 # If that happens, it would cause the script to fail preventing
247 # the script from continuing. To address this, simply catch the
248 # error, return and allow the script to continue.
249 try:
250 bmc_set(mode, 'Always_Available')
251 except subprocess.CalledProcessError:
252 return
246253
247254
248def commit_ipmi_settings(config):255def commit_ipmi_settings(config):
diff --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
index 2252e90..b7d9411 100644
--- 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
@@ -16,6 +16,7 @@ from maastesting.matchers import (
16 MockCalledOnceWith,16 MockCalledOnceWith,
17 MockCallsMatch,17 MockCallsMatch,
18 MockNotCalled,18 MockNotCalled,
19 HasLength,
19)20)
20from maastesting.testcase import MAASTestCase21from maastesting.testcase import MAASTestCase
21from snippets import maas_ipmi_autodetect22from snippets import maas_ipmi_autodetect
@@ -626,3 +627,23 @@ class TestEnableLanChannel(MAASTestCase):
626 self.assertThat(627 self.assertThat(
627 bmc_set_mock,628 bmc_set_mock,
628 MockNotCalled())629 MockNotCalled())
630
631 def test_set_ipmi_lan_channel_handles_calledprocesserror(self):
632 "Test that set_ipmi_lan_channel handles CalledProcessError"
633 response = (
634 "Section Lan_Channel\n"
635 " Volatile_Access_Mode Always_Available\n"
636 " Non_Volatile_Access_Mode Always_Available\n"
637 "EndSection"
638 )
639 self.patch(
640 maas_ipmi_autodetect, 'bmc_get').return_value = response
641 bmc_set_mock = self.patch(
642 maas_ipmi_autodetect, 'bmc_set')
643 bmc_set_mock.side_effect = subprocess.CalledProcessError(
644 1, 'bmc-set')
645 set_ipmi_lan_channel_settings()
646 self.assertRaises(
647 subprocess.CalledProcessError, bmc_set_mock)
648 self.assertThat(
649 bmc_set_mock.call_args_list, HasLength(1))

Subscribers

People subscribed via source and target branches