sts-sponsors team mailing list archive

[Seyeong Kim <mp+437594@xxxxxxxxxxxxxxxxxx>]

Seyeong Kim has proposed merging ~seyeongkim/maas:lp1979403_to_3.0 into maas:3.0.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~seyeongkim/maas/+git/maas/+merge/437594
-- 
Your team MAAS Maintainers is requested to review the proposed merge of ~seyeongkim/maas:lp1979403_to_3.0 into maas:3.0.

diff --git a/creds.yaml b/creds.yaml
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/creds.yaml
diff --git a/src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py b/src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py
index 42e4c79..f87a136 100755
--- a/src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py
+++ b/src/metadataserver/builtin_scripts/commissioning_scripts/bmc_config.py
@@ -174,7 +174,7 @@ class IPMI(BMCConfig):
     def _bmc_get_config(self, section=None):
         """Fetch and cache all BMC settings."""
         print("INFO: Reading current IPMI BMC values...")
-        cmd = ["bmc-config", "--checkout"]
+        cmd = ["bmc-config", "--checkout", "--verbose"]
         if section:
             cmd += ["-S", section]
         try:
@@ -438,41 +438,53 @@ class IPMI(BMCConfig):
     def _config_ipmi_lan_channel_settings(self):
         """Enable IPMI-over-Lan (Lan_Channel) if it is disabled"""
         print("INFO: Configuring IPMI Lan_Channel...")
-        lan_channel = self._bmc_config.get("Lan_Channel", {})
 
-        for key in [
-            "Volatile_Access_Mode",
-            "Non_Volatile_Access_Mode",
+        for channel in [
+            "Lan_Channel",
+            "Lan_Channel_Channel_1",
+            "Lan_Channel_Channel_2",
+            "Lan_Channel_Channel_3",
         ]:
-            if lan_channel.get(key) != "Always_Available":
-                print(
-                    "INFO: Enabling BMC network access - Lan_Channel:%s" % key
-                )
-                # Some BMC's don't support setting Lan_Channel (see LP: #1287274).
-                # If that happens, it would cause the script to fail preventing
-                # the script from continuing. To address this, simply catch the
-                # error, return and allow the script to continue.
-                try:
-                    self._bmc_set("Lan_Channel", key, "Always_Available")
-                except Exception:
+            lan_channel = self._bmc_config.get(channel, {})
+
+            if not lan_channel:
+                continue
+
+            for key in [
+                "Volatile_Access_Mode",
+                "Non_Volatile_Access_Mode",
+            ]:
+                if lan_channel.get(key) != "Always_Available":
                     print(
-                        "WARNING: Unable to set Lan_Channel:%s. "
-                        "BMC may be unavailable over the network!" % key
+                        "INFO: Enabling BMC network access - %s:%s"
+                        % (channel, key)
                     )
-
-        self._bmc_set_keys(
-            "Lan_Channel",
-            [
-                "%s_%s" % (auth_type, volatility)
-                for auth_type in [
-                    "Enable_User_Level_Auth",
-                    "Enable_Per_Message_Auth",
-                    "Enable_Pef_Alerting",
-                ]
-                for volatility in ["Volatile", "Non_Volatile"]
-            ],
-            "Yes",
-        )
+                    # Some BMC's don't support setting Lan_Channel (see LP: #1287274).
+                    # If that happens, it would cause the script to fail preventing
+                    # the script from continuing. To address this, simply catch the
+                    # error, return and allow the script to continue.
+                    try:
+                        self._bmc_set(channel, key, "Always_Available")
+                    except Exception:
+                        print(
+                            "WARNING: Unable to set %s:%s. "
+                            "BMC may be unavailable over the network!"
+                            % (channel, key)
+                        )
+
+            self._bmc_set_keys(
+                channel,
+                [
+                    f"{auth_type}_{volatility}"
+                    for auth_type in [
+                        "Enable_User_Level_Auth",
+                        "Enable_Per_Message_Auth",
+                        "Enable_Pef_Alerting",
+                    ]
+                    for volatility in ["Volatile", "Non_Volatile"]
+                ],
+                "Yes",
+            )
 
     def _config_lan_conf_auth(self):
         """Configure Lan_Conf_Auth."""
@@ -707,6 +719,9 @@ class IPMI(BMCConfig):
         mac_address = None
         for section_name, key in [
             ("Lan_Conf", "IP_Address"),
+            ("Lan_Conf_Channel_1", "IP_Address"),
+            ("Lan_Conf_Channel_2", "IP_Address"),
+            ("Lan_Conf_Channel_3", "IP_Address"),
             ("Lan6_Conf", "IPv6_Static_Addresses"),
             ("Lan6_Conf", "IPv6_Dynamic_Addresses"),
         ]:
@@ -729,28 +744,28 @@ class IPMI(BMCConfig):
                     time.sleep(2)
                     continue
                 if section_name.startswith("Lan6_"):
-                    return "[%s]" % ip, mac_address
-                return ip, mac_address
+                    return section_name, "[%s]" % ip, mac_address
+                return section_name, ip, mac_address
             # No valid IP address was found.
-        return None, mac_address
+        return None, None, mac_address
 
     def get_bmc_ip(self):
         """Configure and retreive IPMI BMC IP."""
-        ip_address, mac_address = self._get_bmc_ip()
+        section_name, ip_address, mac_address = self._get_bmc_ip()
         if ip_address:
             return ip_address, mac_address
         print("INFO: Attempting to enable preconfigured static IP on BMC...")
-        self._bmc_set("Lan_Conf", "IP_Address_Source", "Static")
+        self._bmc_set(section_name, "IP_Address_Source", "Static")
         for _ in range(6):
             time.sleep(10)
-            ip_address, mac_address = self._get_bmc_ip(True)
+            _, ip_address, mac_address = self._get_bmc_ip(True)
             if ip_address:
                 return ip_address, mac_address
         print("INFO: Attempting to enable DHCP on BMC...")
-        self._bmc_set("Lan_Conf", "IP_Address_Source", "Use_DHCP")
+        self._bmc_set(section_name, "IP_Address_Source", "Use_DHCP")
         for _ in range(6):
             time.sleep(10)
-            ip_address, mac_address = self._get_bmc_ip(True)
+            _, ip_address, mac_address = self._get_bmc_ip(True)
             if ip_address:
                 print("WARNING: BMC is configured to use DHCP!")
                 return ip_address, mac_address
diff --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
index 90f4af1..73e84b0 100644
--- a/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_bmc_config.py
+++ b/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_bmc_config.py
@@ -538,20 +538,25 @@ EndSection
         self.assertRaises(SystemExit, self.ipmi.add_bmc_user)
 
     def test_set_ipmi_lan_channel_setting_verifies(self):
-        self.ipmi._bmc_config = {
-            "Lan_Channel": {
-                "Volatile_Access_Mode": "Always_Available",
-                "Non_Volatile_Access_Mode": "Always_Available",
+
+        for channel in [
+            "Lan_Channel",
+            "Lan_Channel_Channel_1",
+            "Lan_Channel_Channel_2",
+            "Lan_Channel_Channel_3",
+        ]:
+            self.ipmi._bmc_config = {
+                channel: {
+                    "Volatile_Access_Mode": "Always_Available",
+                    "Non_Volatile_Access_Mode": "Always_Available",
+                },
             }
-        }
-        mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
-        mock_bmc_set_keys = self.patch(self.ipmi, "_bmc_set_keys")
-        self.ipmi._config_ipmi_lan_channel_settings()
-        self.assertThat(mock_bmc_set, MockNotCalled())
-        self.assertThat(
-            mock_bmc_set_keys,
-            MockCalledOnceWith(
-                "Lan_Channel",
+            mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
+            mock_bmc_set_keys = self.patch(self.ipmi, "_bmc_set_keys")
+            self.ipmi._config_ipmi_lan_channel_settings()
+            self.assertFalse(mock_bmc_set.called)
+            mock_bmc_set_keys.assert_called_once_with(
+                channel,
                 [
                     "%s_%s" % (auth_type, volatility)
                     for auth_type in [
@@ -562,36 +567,36 @@ EndSection
                     for volatility in ["Volatile", "Non_Volatile"]
                 ],
                 "Yes",
-            ),
-        )
+            )
 
     def test_set_ipmi_lan_channel_setting_enables(self):
-        self.ipmi._bmc_config = {
-            "Lan_Channel": {
-                "Volatile_Access_Mode": "Disabled",
-                "Non_Volatile_Access_Mode": "Pre_Boot_only",
+        for channel in [
+            "Lan_Channel",
+            "Lan_Channel_Channel_1",
+            "Lan_Channel_Channel_2",
+            "Lan_Channel_Channel_3",
+        ]:
+            self.ipmi._bmc_config = {
+                channel: {
+                    "Volatile_Access_Mode": "Disabled",
+                    "Non_Volatile_Access_Mode": "Pre_Boot_only",
+                },
             }
-        }
-        mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
-        mock_bmc_set_keys = self.patch(self.ipmi, "_bmc_set_keys")
-        self.ipmi._config_ipmi_lan_channel_settings()
-        self.assertThat(
-            mock_bmc_set,
-            MockCallsMatch(
-                call(
-                    "Lan_Channel", "Volatile_Access_Mode", "Always_Available"
-                ),
-                call(
-                    "Lan_Channel",
-                    "Non_Volatile_Access_Mode",
-                    "Always_Available",
-                ),
-            ),
-        )
-        self.assertThat(
-            mock_bmc_set_keys,
-            MockCalledOnceWith(
-                "Lan_Channel",
+            mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
+            mock_bmc_set_keys = self.patch(self.ipmi, "_bmc_set_keys")
+            self.ipmi._config_ipmi_lan_channel_settings()
+            mock_bmc_set.assert_has_calls(
+                (
+                    call(channel, "Volatile_Access_Mode", "Always_Available"),
+                    call(
+                        channel,
+                        "Non_Volatile_Access_Mode",
+                        "Always_Available",
+                    ),
+                )
+            )
+            mock_bmc_set_keys.assert_called_once_with(
+                channel,
                 [
                     "%s_%s" % (auth_type, volatility)
                     for auth_type in [
@@ -602,8 +607,7 @@ EndSection
                     for volatility in ["Volatile", "Non_Volatile"]
                 ],
                 "Yes",
-            ),
-        )
+            )
 
     def test_config_lan_conf_auth(self):
         self.ipmi._bmc_config = {"Lan_Channel_Auth": {}}
@@ -943,7 +947,9 @@ EndSection
                 "MAC_Address": mac_address,
             }
         }
-        self.assertEqual((ip, mac_address), self.ipmi._get_bmc_ip())
+        self.assertEqual(
+            ("Lan_Conf", ip, mac_address), self.ipmi._get_bmc_ip()
+        )
 
     def test_get_bmc_ipv6_static(self):
         ip = factory.make_ipv6_address()
@@ -954,7 +960,9 @@ EndSection
                 "MAC_Address": mac_address,
             }
         }
-        self.assertEqual((f"[{ip}]", mac_address), self.ipmi._get_bmc_ip())
+        self.assertEqual(
+            ("Lan6_Conf", f"[{ip}]", mac_address), self.ipmi._get_bmc_ip()
+        )
 
     def test_get_bmc_ipv6_dynamic(self):
         ip = factory.make_ipv6_address()
@@ -965,7 +973,9 @@ EndSection
                 "MAC_Address": mac_address,
             }
         }
-        self.assertEqual((f"[{ip}]", mac_address), self.ipmi._get_bmc_ip())
+        self.assertEqual(
+            ("Lan6_Conf", f"[{ip}]", mac_address), self.ipmi._get_bmc_ip()
+        )
 
     def test_get_bmc_ipv6_gets_mac_From_ipv4(self):
         ip = factory.make_ipv6_address()
@@ -974,18 +984,20 @@ EndSection
             "Lan_Conf": {"MAC_Address": mac_address},
             "Lan6_Conf": {"IPv6_Dynamic_Addresses": ip},
         }
-        self.assertEqual((f"[{ip}]", mac_address), self.ipmi._get_bmc_ip())
+        self.assertEqual(
+            ("Lan6_Conf", f"[{ip}]", mac_address), self.ipmi._get_bmc_ip()
+        )
 
     def test_get_bmc_ip_finds_none(self):
         self.patch(self.ipmi, "_bmc_get").return_value = ""
-        self.assertEqual((None, None), self.ipmi._get_bmc_ip())
+        self.assertEqual((None, None, None), self.ipmi._get_bmc_ip())
 
     def test_get_bmc_ip(self):
         ip = factory.make_ip_address()
         mac_address = factory.make_mac_address()
         mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
         mock_get_bmc_ip = self.patch(self.ipmi, "_get_bmc_ip")
-        mock_get_bmc_ip.return_value = ip, mac_address
+        mock_get_bmc_ip.return_value = None, ip, mac_address
 
         self.assertEqual((ip, mac_address), self.ipmi.get_bmc_ip())
         self.assertThat(mock_bmc_set, MockNotCalled())
@@ -997,9 +1009,9 @@ EndSection
         mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
         mock_get_bmc_ip = self.patch(self.ipmi, "_get_bmc_ip")
         mock_get_bmc_ip.side_effect = (
-            (None, mac_address),
-            (None, mac_address),
-            (ip, mac_address),
+            ("Lan_Conf", None, mac_address),
+            ("Lan_Conf", None, mac_address),
+            ("Lan_Conf", ip, mac_address),
         )
 
         self.assertEqual((ip, mac_address), self.ipmi.get_bmc_ip())
@@ -1017,8 +1029,8 @@ EndSection
         mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
         mock_get_bmc_ip = self.patch(self.ipmi, "_get_bmc_ip")
         mock_get_bmc_ip.side_effect = (
-            *[(None, mac_address) for _ in range(8)],
-            (ip, mac_address),
+            *[("Lan_Conf", None, mac_address) for _ in range(8)],
+            ("Lan_Conf", ip, mac_address),
         )
 
         self.assertEqual((ip, mac_address), self.ipmi.get_bmc_ip())
@@ -1029,6 +1041,7 @@ EndSection
                 call("Lan_Conf", "IP_Address_Source", "Use_DHCP"),
             ),
         )
+
         self.assertThat(
             mock_get_bmc_ip,
             MockCallsMatch(call(), *[call(True) for _ in range(8)]),
@@ -1037,7 +1050,7 @@ EndSection
     def test_get_bmc_ip_fails(self):
         mock_bmc_set = self.patch(self.ipmi, "_bmc_set")
         mock_get_bmc_ip = self.patch(self.ipmi, "_get_bmc_ip")
-        mock_get_bmc_ip.return_value = (None, None)
+        mock_get_bmc_ip.return_value = ("Lan_Conf", None, None)
 
         self.assertRaises(SystemExit, self.ipmi.get_bmc_ip)
         self.assertThat(