From fd92209d4dd57c76d550322b029525871c7aa622 Mon Sep 17 00:00:00 2001
From: Markus Hilger <markus.hilger@megware.com>
Date: Sun, 16 Aug 2026 21:37:50 +0200
Subject: [PATCH] Read a port off the manager address over ipmi too

The plugin connected to 623 whatever the address said, with a TODO in place of
the parsing. It now reads one as the redfish plugin does, minus the brackets,
which getaddrinfo rejects.

IpmiConsole keys its endpoint mapping on host and port so several bmcs behind
one address stay distinct, and unregisters only an entry it actually claimed.
---
 .../plugins/hardwaremanagement/ipmi.py        | 52 +++++++++++++++----
 1 file changed, 43 insertions(+), 9 deletions(-)

diff --git a/confluent_server/confluent/plugins/hardwaremanagement/ipmi.py b/confluent_server/confluent/plugins/hardwaremanagement/ipmi.py
index 1e28a187..90720295 100644
--- a/confluent_server/confluent/plugins/hardwaremanagement/ipmi.py
+++ b/confluent_server/confluent/plugins/hardwaremanagement/ipmi.py
@@ -262,14 +262,37 @@ def get_conn_params(node, configdata):
         kg = configdata['secret.ipmikg']['value']
     else:
         kg = passphrase
-    # TODO(jbjohnso): check if the end has some number after a : without []
-    # for non default port
+    # Read a port off the address, as the redfish plugin does. Brackets come
+    # off, unlike there: this address goes to socket.getaddrinfo, which rejects
+    # a bracketed literal, rather than into a URL where one is required.
+    bmc = bmc.strip()
+    port = 623
+    if bmc.startswith('['):
+        bracket_end = bmc.find(']')
+        if bracket_end > 0:
+            if len(bmc) > bracket_end + 1 and bmc[bracket_end + 1] == ':':
+                try:
+                    port = int(bmc[bracket_end + 2:])
+                except (ValueError, TypeError):
+                    pass
+            bmc = bmc[1:bracket_end]
+    elif bmc.count(':') == 1:
+        hostpart, _, portstr = bmc.rpartition(':')
+        try:
+            port = int(portstr)
+        except (ValueError, TypeError):
+            pass
+        bmc = hostpart
+    if not 0 < port <= 65535:
+        # NOTE: Fallback to 623 if port read is not valid
+        port = 623
+
     return {
         'username': username,
         'passphrase': passphrase,
         'kg': kg,
         'bmc': bmc,
-        'port': 623,
+        'port': port,
     }
 
 
@@ -287,6 +310,11 @@ def _donothing(data):
 class IpmiConsole(conapi.Console):
     configattributes = frozenset(_configattributes)
     bmctonodemapping = {}
+    # Whether this instance is the one that put its endpoint in the mapping
+    # above. False until it does, so that an instance rejected as a duplicate,
+    # or one that failed earlier in __init__, does not remove on the way out an
+    # entry that belongs to the node holding the endpoint.
+    claimedbmc = False
 
     def __init__(self, node, config):
         self.error = None
@@ -303,20 +331,26 @@ class IpmiConsole(conapi.Console):
         self.kg = connparams['kg']
         self.bmc = connparams['bmc']
         self.port = connparams['port']
+        # The port is part of the identity: several bmcs may sit behind one
+        # address on different ports, and they are distinct devices.
+        self.bmckey = (self.bmc, self.port)
         self.connected = False
-        # ok, is self.bmc unique among nodes already
+        # ok, is this bmc unique among nodes already
         # Cannot actually create console until 'connect', when we get callback
-        if (self.bmc in self.bmctonodemapping and
-                self.bmctonodemapping[self.bmc] != node):
+        if (self.bmckey in self.bmctonodemapping and
+                self.bmctonodemapping[self.bmckey] != node):
             raise Exception(
                 "Duplicate hardwaremanagement.manager attribute for {0} and {1}".format(
-                    node, self.bmctonodemapping[self.bmc]))
-        self.bmctonodemapping[self.bmc] = node
+                    node, self.bmctonodemapping[self.bmckey]))
+        self.bmctonodemapping[self.bmckey] = node
+        self.claimedbmc = True
 
     def __del__(self):
         self.solconnection = None
+        if not self.claimedbmc:
+            return
         try:
-            del self.bmctonodemapping[self.bmc]
+            del self.bmctonodemapping[self.bmckey]
         except KeyError:
             pass