MAAS

Merge ~seyeongkim/maas:multiple_lan_conf_channel into maas:master

  1. Git
  2. lp:~seyeongkim/maas
  3. multiple_lan_conf_channel
  4. Merge into master
Proposed by Seyeong Kim
Status: Merged
Approved by: Alexsander de Souza
Approved revision: a722f0979038a596f89e75b13bb19c284f96b3ed
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~seyeongkim/maas:multiple_lan_conf_channel
Merge into: maas:master
Diff against target: 365 lines (+122/-94)
3 files modified
creds.yaml (+0/-0)
src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py (+55/-40)
src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_bmc_config.py (+67/-54)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Jack Lloyd-Walters Approve
Review via email: mp+434404@code.launchpad.net

Commit message

Adding Lan_Conf_Channel_1 to 3

BMC with multiple channel can't find ip
so added Lan_Conf_Channel_1 to 3 to support them

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

UNIT TESTS
-b multiple_lan_conf_channel lp:~seyeongkim/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/1581/consoleText
COMMIT: 1b809d150f8f2a8aa53e9f9fac4849b4f13b9e74

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

UNIT TESTS
-b multiple_lan_conf_channel lp:~seyeongkim/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/1582/consoleText
COMMIT: 132dd079a68a6d687dc3ca4c28de48ee3ff3ad0d

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

UNIT TESTS
-b multiple_lan_conf_channel lp:~seyeongkim/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 1f2d365109315a49486487751d43904f09b06a84

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

UNIT TESTS
-b multiple_lan_conf_channel lp:~seyeongkim/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/1673/consoleText
COMMIT: 7e8d5393108789346eaafcd87ff99805849a025c

review: Needs Fixing
Revision history for this message
Seyeong Kim (seyeongkim) wrote :

hmm rebase and failed..

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

UNIT TESTS
-b multiple_lan_conf_channel lp:~seyeongkim/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: efd7bff6ec8658775a5186314845c66c25e453ce

review: Approve
Revision history for this message
Seyeong Kim (seyeongkim) wrote :

I test this with below script ( similar to original code )

https://pastebin.ubuntu.com/p/8x5BXrsm3g/

with verbose output

https://pastebin.ubuntu.com/p/8sM5HQqc5K/

Revision history for this message
Jack Lloyd-Walters (lloydwaltersj) wrote :

Inline nit, looks good elsewise

review: Needs Fixing
Revision history for this message
Jack Lloyd-Walters (lloydwaltersj) wrote :

jenkins: !test

Revision history for this message
Jack Lloyd-Walters (lloydwaltersj) wrote :

LGTM, have fired off for a new test from MAAS Lander

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

UNIT TESTS
-b multiple_lan_conf_channel lp:~seyeongkim/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: a722f0979038a596f89e75b13bb19c284f96b3ed

review: Approve
Revision history for this message
Alexsander de Souza (alexsander-souza) wrote :

We have another confirmation that this patch fixes the issue:

https://discourse.maas.io/t/multiple-bmc-interfaces-handling-for-30-maas-01-bmc-config/6499/5?u=alexsander-souza

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

LANDING
-b multiple_lan_conf_channel lp:~seyeongkim/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas-tester/1917/consoleText

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/creds.yaml b/creds.yaml
2new file mode 100644
3index 0000000..e69de29
4--- /dev/null
5+++ b/creds.yaml
6diff --git a/src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py b/src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py
7index 4964f08..02e900d 100755
8--- a/src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py
9+++ b/src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py
10@@ -226,7 +226,7 @@ class IPMIBase(BMCConfig):
11 def _bmc_get_config(self, section=None):
12 """Fetch and cache all BMC settings."""
13 print("INFO: Reading current IPMI BMC values...")
14- cmd = ["bmc-config", "--checkout"]
15+ cmd = ["bmc-config", "--checkout", "--verbose"]
16 if section:
17 cmd += ["-S", section]
18 try:
19@@ -392,41 +392,53 @@ class IPMIBase(BMCConfig):
20 def _config_ipmi_lan_channel_settings(self):
21 """Enable IPMI-over-Lan (Lan_Channel) if it is disabled"""
22 print("INFO: Configuring IPMI Lan_Channel...")
23- lan_channel = self._bmc_config.get("Lan_Channel", {})
24
25- for key in [
26- "Volatile_Access_Mode",
27- "Non_Volatile_Access_Mode",
28+ for channel in [
29+ "Lan_Channel",
30+ "Lan_Channel_Channel_1",
31+ "Lan_Channel_Channel_2",
32+ "Lan_Channel_Channel_3",
33 ]:
34- if lan_channel.get(key) != "Always_Available":
35- print(
36- "INFO: Enabling BMC network access - Lan_Channel:%s" % key
37- )
38- # Some BMC's don't support setting Lan_Channel (see LP: #1287274).
39- # If that happens, it would cause the script to fail preventing
40- # the script from continuing. To address this, simply catch the
41- # error, return and allow the script to continue.
42- try:
43- self._bmc_set("Lan_Channel", key, "Always_Available")
44- except Exception:
45+ lan_channel = self._bmc_config.get(channel, {})
46+
47+ if not lan_channel:
48+ continue
49+
50+ for key in [
51+ "Volatile_Access_Mode",
52+ "Non_Volatile_Access_Mode",
53+ ]:
54+ if lan_channel.get(key) != "Always_Available":
55 print(
56- "WARNING: Unable to set Lan_Channel:%s. "
57- "BMC may be unavailable over the network!" % key
58+ "INFO: Enabling BMC network access - %s:%s"
59+ % (channel, key)
60 )
61-
62- self._bmc_set_keys(
63- "Lan_Channel",
64- [
65- f"{auth_type}_{volatility}"
66- for auth_type in [
67- "Enable_User_Level_Auth",
68- "Enable_Per_Message_Auth",
69- "Enable_Pef_Alerting",
70- ]
71- for volatility in ["Volatile", "Non_Volatile"]
72- ],
73- "Yes",
74- )
75+ # Some BMC's don't support setting Lan_Channel (see LP: #1287274).
76+ # If that happens, it would cause the script to fail preventing
77+ # the script from continuing. To address this, simply catch the
78+ # error, return and allow the script to continue.
79+ try:
80+ self._bmc_set(channel, key, "Always_Available")
81+ except Exception:
82+ print(
83+ "WARNING: Unable to set %s:%s. "
84+ "BMC may be unavailable over the network!"
85+ % (channel, key)
86+ )
87+
88+ self._bmc_set_keys(
89+ channel,
90+ [
91+ f"{auth_type}_{volatility}"
92+ for auth_type in [
93+ "Enable_User_Level_Auth",
94+ "Enable_Per_Message_Auth",
95+ "Enable_Pef_Alerting",
96+ ]
97+ for volatility in ["Volatile", "Non_Volatile"]
98+ ],
99+ "Yes",
100+ )
101
102 def _config_lan_conf_auth(self):
103 """Configure Lan_Conf_Auth."""
104@@ -577,6 +589,9 @@ class IPMI(IPMIBase):
105 mac_address = None
106 for section_name, key in [
107 ("Lan_Conf", "IP_Address"),
108+ ("Lan_Conf_Channel_1", "IP_Address"),
109+ ("Lan_Conf_Channel_2", "IP_Address"),
110+ ("Lan_Conf_Channel_3", "IP_Address"),
111 ("Lan6_Conf", "IPv6_Static_Addresses"),
112 ("Lan6_Conf", "IPv6_Dynamic_Addresses"),
113 ]:
114@@ -599,28 +614,28 @@ class IPMI(IPMIBase):
115 time.sleep(2)
116 continue
117 if section_name.startswith("Lan6_"):
118- return "[%s]" % ip, mac_address
119- return ip, mac_address
120+ return section_name, "[%s]" % ip, mac_address
121+ return section_name, ip, mac_address
122 # No valid IP address was found.
123- return None, mac_address
124+ return None, None, mac_address
125
126 def get_bmc_ip(self):
127 """Configure and retreive IPMI BMC IP."""
128- ip_address, mac_address = self._get_bmc_ip()
129+ section_name, ip_address, mac_address = self._get_bmc_ip()
130 if ip_address:
131 return ip_address, mac_address
132 print("INFO: Attempting to enable preconfigured static IP on BMC...")
133- self._bmc_set("Lan_Conf", "IP_Address_Source", "Static")
134+ self._bmc_set(section_name, "IP_Address_Source", "Static")
135 for _ in range(6):
136 time.sleep(10)
137- ip_address, mac_address = self._get_bmc_ip(True)
138+ _, ip_address, mac_address = self._get_bmc_ip(True)
139 if ip_address:
140 return ip_address, mac_address
141 print("INFO: Attempting to enable DHCP on BMC...")
142- self._bmc_set("Lan_Conf", "IP_Address_Source", "Use_DHCP")
143+ self._bmc_set(section_name, "IP_Address_Source", "Use_DHCP")
144 for _ in range(6):
145 time.sleep(10)
146- ip_address, mac_address = self._get_bmc_ip(True)
147+ _, ip_address, mac_address = self._get_bmc_ip(True)
148 if ip_address:
149 print("WARNING: BMC is configured to use DHCP!")
150 return ip_address, mac_address
151diff --git a/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_bmc_config.py b/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_bmc_config.py
152index 1fced30..ea4b710 100644
153--- a/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_bmc_config.py
154+++ b/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_bmc_config.py
155@@ -490,20 +490,25 @@ EndSection
156 self.assertRaises(SystemExit, self.ipmi.add_bmc_user)
157
158 def test_set_ipmi_lan_channel_setting_verifies(self):
159- self.ipmi._bmc_config = {
160- "Lan_Channel": {
161- "Volatile_Access_Mode": "Always_Available",
162- "Non_Volatile_Access_Mode": "Always_Available",
163+
164+ for channel in [
165+ "Lan_Channel",
166+ "Lan_Channel_Channel_1",
167+ "Lan_Channel_Channel_2",
168+ "Lan_Channel_Channel_3",
169+ ]:
170+ self.ipmi._bmc_config = {
171+ channel: {
172+ "Volatile_Access_Mode": "Always_Available",
173+ "Non_Volatile_Access_Mode": "Always_Available",
174+ },
175 }
176- }
177- mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
178- mock_bmc_set_keys = self.patch(self.ipmi, "_bmc_set_keys")
179- self.ipmi._config_ipmi_lan_channel_settings()
180- self.assertThat(mock_bmc_set, MockNotCalled())
181- self.assertThat(
182- mock_bmc_set_keys,
183- MockCalledOnceWith(
184- "Lan_Channel",
185+ mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
186+ mock_bmc_set_keys = self.patch(self.ipmi, "_bmc_set_keys")
187+ self.ipmi._config_ipmi_lan_channel_settings()
188+ self.assertFalse(mock_bmc_set.called)
189+ mock_bmc_set_keys.assert_called_once_with(
190+ channel,
191 [
192 f"{auth_type}_{volatility}"
193 for auth_type in [
194@@ -514,36 +519,36 @@ EndSection
195 for volatility in ["Volatile", "Non_Volatile"]
196 ],
197 "Yes",
198- ),
199- )
200+ )
201
202 def test_set_ipmi_lan_channel_setting_enables(self):
203- self.ipmi._bmc_config = {
204- "Lan_Channel": {
205- "Volatile_Access_Mode": "Disabled",
206- "Non_Volatile_Access_Mode": "Pre_Boot_only",
207+ for channel in [
208+ "Lan_Channel",
209+ "Lan_Channel_Channel_1",
210+ "Lan_Channel_Channel_2",
211+ "Lan_Channel_Channel_3",
212+ ]:
213+ self.ipmi._bmc_config = {
214+ channel: {
215+ "Volatile_Access_Mode": "Disabled",
216+ "Non_Volatile_Access_Mode": "Pre_Boot_only",
217+ },
218 }
219- }
220- mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
221- mock_bmc_set_keys = self.patch(self.ipmi, "_bmc_set_keys")
222- self.ipmi._config_ipmi_lan_channel_settings()
223- self.assertThat(
224- mock_bmc_set,
225- MockCallsMatch(
226- call(
227- "Lan_Channel", "Volatile_Access_Mode", "Always_Available"
228- ),
229- call(
230- "Lan_Channel",
231- "Non_Volatile_Access_Mode",
232- "Always_Available",
233- ),
234- ),
235- )
236- self.assertThat(
237- mock_bmc_set_keys,
238- MockCalledOnceWith(
239- "Lan_Channel",
240+ mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
241+ mock_bmc_set_keys = self.patch(self.ipmi, "_bmc_set_keys")
242+ self.ipmi._config_ipmi_lan_channel_settings()
243+ mock_bmc_set.assert_has_calls(
244+ (
245+ call(channel, "Volatile_Access_Mode", "Always_Available"),
246+ call(
247+ channel,
248+ "Non_Volatile_Access_Mode",
249+ "Always_Available",
250+ ),
251+ )
252+ )
253+ mock_bmc_set_keys.assert_called_once_with(
254+ channel,
255 [
256 f"{auth_type}_{volatility}"
257 for auth_type in [
258@@ -554,8 +559,7 @@ EndSection
259 for volatility in ["Volatile", "Non_Volatile"]
260 ],
261 "Yes",
262- ),
263- )
264+ )
265
266 def test_config_lan_conf_auth(self):
267 self.ipmi._bmc_config = {"Lan_Channel_Auth": {}}
268@@ -718,7 +722,9 @@ EndSection
269 "MAC_Address": mac_address,
270 }
271 }
272- self.assertEqual((ip, mac_address), self.ipmi._get_bmc_ip())
273+ self.assertEqual(
274+ ("Lan_Conf", ip, mac_address), self.ipmi._get_bmc_ip()
275+ )
276
277 def test_get_bmc_ipv6_static(self):
278 ip = factory.make_ipv6_address()
279@@ -729,7 +735,9 @@ EndSection
280 "MAC_Address": mac_address,
281 }
282 }
283- self.assertEqual((f"[{ip}]", mac_address), self.ipmi._get_bmc_ip())
284+ self.assertEqual(
285+ ("Lan6_Conf", f"[{ip}]", mac_address), self.ipmi._get_bmc_ip()
286+ )
287
288 def test_get_bmc_ipv6_dynamic(self):
289 ip = factory.make_ipv6_address()
290@@ -740,7 +748,9 @@ EndSection
291 "MAC_Address": mac_address,
292 }
293 }
294- self.assertEqual((f"[{ip}]", mac_address), self.ipmi._get_bmc_ip())
295+ self.assertEqual(
296+ ("Lan6_Conf", f"[{ip}]", mac_address), self.ipmi._get_bmc_ip()
297+ )
298
299 def test_get_bmc_ipv6_gets_mac_From_ipv4(self):
300 ip = factory.make_ipv6_address()
301@@ -749,18 +759,20 @@ EndSection
302 "Lan_Conf": {"MAC_Address": mac_address},
303 "Lan6_Conf": {"IPv6_Dynamic_Addresses": ip},
304 }
305- self.assertEqual((f"[{ip}]", mac_address), self.ipmi._get_bmc_ip())
306+ self.assertEqual(
307+ ("Lan6_Conf", f"[{ip}]", mac_address), self.ipmi._get_bmc_ip()
308+ )
309
310 def test_get_bmc_ip_finds_none(self):
311 self.patch(self.ipmi, "_bmc_get").return_value = ""
312- self.assertEqual((None, None), self.ipmi._get_bmc_ip())
313+ self.assertEqual((None, None, None), self.ipmi._get_bmc_ip())
314
315 def test_get_bmc_ip(self):
316 ip = factory.make_ip_address()
317 mac_address = factory.make_mac_address()
318 mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
319 mock_get_bmc_ip = self.patch(self.ipmi, "_get_bmc_ip")
320- mock_get_bmc_ip.return_value = ip, mac_address
321+ mock_get_bmc_ip.return_value = None, ip, mac_address
322
323 self.assertEqual((ip, mac_address), self.ipmi.get_bmc_ip())
324 self.assertThat(mock_bmc_set, MockNotCalled())
325@@ -772,9 +784,9 @@ EndSection
326 mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
327 mock_get_bmc_ip = self.patch(self.ipmi, "_get_bmc_ip")
328 mock_get_bmc_ip.side_effect = (
329- (None, mac_address),
330- (None, mac_address),
331- (ip, mac_address),
332+ ("Lan_Conf", None, mac_address),
333+ ("Lan_Conf", None, mac_address),
334+ ("Lan_Conf", ip, mac_address),
335 )
336
337 self.assertEqual((ip, mac_address), self.ipmi.get_bmc_ip())
338@@ -792,8 +804,8 @@ EndSection
339 mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
340 mock_get_bmc_ip = self.patch(self.ipmi, "_get_bmc_ip")
341 mock_get_bmc_ip.side_effect = (
342- *[(None, mac_address) for _ in range(8)],
343- (ip, mac_address),
344+ *[("Lan_Conf", None, mac_address) for _ in range(8)],
345+ ("Lan_Conf", ip, mac_address),
346 )
347
348 self.assertEqual((ip, mac_address), self.ipmi.get_bmc_ip())
349@@ -804,6 +816,7 @@ EndSection
350 call("Lan_Conf", "IP_Address_Source", "Use_DHCP"),
351 ),
352 )
353+
354 self.assertThat(
355 mock_get_bmc_ip,
356 MockCallsMatch(call(), *[call(True) for _ in range(8)]),
357@@ -812,7 +825,7 @@ EndSection
358 def test_get_bmc_ip_fails(self):
359 mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
360 mock_get_bmc_ip = self.patch(self.ipmi, "_get_bmc_ip")
361- mock_get_bmc_ip.return_value = (None, None)
362+ mock_get_bmc_ip.return_value = ("Lan_Conf", None, None)
363
364 self.assertRaises(SystemExit, self.ipmi.get_bmc_ip)
365 self.assertThat(

Subscribers

People subscribed via source and target branches