Dev: ui_corosync: refine the error messages for missing or duplicated…

… nodes (jsc#PED-8083)

crmsh/corosync.py

@@ -5,6 +5,7 @@
 configuration file, and also the corosync-* utilities.
 '''
 import dataclasses
+import ipaddress
 import itertools
 import os
 import re
@@ -582,6 +583,19 @@ def __load_option(self, data: dict[str, str], field: dataclasses.Field):
 
 
 class LinkManager:
+    class LinkManageException(Exception):
+        pass
+
+    @dataclasses.dataclass
+    class MissingNodesException(LinkManageException):
+        nodeids: list[int]
+
+    @dataclasses.dataclass
+    class DuplicatedNodeAddressException(LinkManageException):
+        address: str
+        node1: int
+        node2: int
+
     LINK_OPTIONS_UPDATABLE = {
         field.name
         for field in dataclasses.fields(Link)
@@ -754,7 +768,7 @@ def __upsert_node_addr_impl(
                 # need to change uniqueness
                 existing = existing_addr_node_map.get(canonical_addr, None)
                 if existing is not None:
-                    raise ValueError(f'Duplicated node address: {addr}: already used by node {existing}')
+                    raise LinkManager.DuplicatedNodeAddressException(addr, nodeid, existing)
             found.addr = addr
             existing_addr_node_map[canonical_addr] = found.nodeid
         nodes = config['nodelist']['node']
@@ -769,10 +783,9 @@ def add_link(self, node_addresses: typing.Mapping[int, str], options: dict[str,
         links = self.links()
         if len(links) >= KNET_LINK_NUM_LIMIT:
             raise ValueError(f'Cannot add a new link. The maximum number of links supported is {KNET_LINK_NUM_LIMIT}.')
-        node_ids = {node.nodeid for node in links[0].nodes}
-        for nodeid in node_ids:
-            if nodeid not in node_addresses:
-                raise ValueError(f'The address of node {nodeid} is not specified.')
+        unspecified_nodes = [node.nodeid for node in links[0].nodes if node.nodeid not in node_addresses]
+        if unspecified_nodes:
+            raise self.MissingNodesException(unspecified_nodes)
         next_linknumber = len(links)
         links.append(Link(next_linknumber, [dataclasses.replace(node, addr='') for node in links[0].nodes]))
         self.__upsert_node_addr_impl(self._config, links, next_linknumber, node_addresses)

crmsh/ui_corosync.py

@@ -162,11 +162,17 @@ def do_update(self, context, *argv):
         if not args.nodes and not args.options:
             logger.info("Nothing is updated.")
         else:
-            self.__save_changed_config(
-                lm,
-                lm.update_node_addr(args.linknumber, node_addresses),
-                reload=not args.nodes,    # changes to node addresses are not hot reloadable
-            )
+            try:
+                self.__save_changed_config(
+                    lm,
+                    lm.update_node_addr(args.linknumber, node_addresses),
+                    reload=not args.nodes,    # changes to node addresses are not hot reloadable
+                )
+            except corosync.LinkManager.DuplicatedNodeAddressException as e:
+                node1 = next(x.name for x in nodes if x.nodeid == e.node1)
+                node2 = next(x.name for x in nodes if x.nodeid == e.node2)
+                logger.error('Duplicated ip address %s: used for both %s and %s.', e.address, e.node1, e.node2)
+                return False
 
     def do_add(self, context, *argv):
         lm = self.__load_and_validate_links()
@@ -184,11 +190,24 @@ def do_add(self, context, *argv):
             if nodeid == -1:
                 logger.error(f'Unknown node {name}.')
             node_addresses[nodeid] = addr
-        self.__save_changed_config(
-            lm,
-            lm.add_link(node_addresses, args.options),
-            reload=True,
-        )
+        try:
+            self.__save_changed_config(
+                lm,
+                lm.add_link(node_addresses, args.options),
+                reload=True,
+            )
+        except corosync.LinkManager.MissingNodesException as e:
+            for nodeid in e.nodeids:
+                name = next(x.name for x in nodes if x.nodeid == nodeid)
+                logger.error('The address of node %s is not specified.', name)
+            logger.error('The address of all nodes must be specified when adding a new link.')
+            return False
+        except corosync.LinkManager.DuplicatedNodeAddressException as e:
+            node1 = next(x.name for x in nodes if x.nodeid == e.node1)
+            node2 = next(x.name for x in nodes if x.nodeid == e.node2)
+            logger.error('Duplicated ip address %s=%s: already used for %s=%s.', e.node1, e.address, e.node2, e.address)
+            return False
+
 
     @command.completer(completers.call(lambda: [
         str(link.linknumber)

test/unittests/test_corosync.py

@@ -512,13 +512,13 @@ def test_update_unchanged_addr(self):
         self.assertListEqual(self.ORIGINAL['nodelist']['node'], self.lm._config['nodelist']['node'])
 
     def test_update_duplicated_addr(self):
-        with self.assertRaises(ValueError):
+        with self.assertRaises(corosync.LinkManager.DuplicatedNodeAddressException):
             self.lm.update_node_addr(1, {1: "192.0.2.1"})
-        with self.assertRaises(ValueError):
+        with self.assertRaises(corosync.LinkManager.DuplicatedNodeAddressException):
             self.lm.update_node_addr(1, {1: "192.0.2.2"})
-        with self.assertRaises(ValueError):
+        with self.assertRaises(corosync.LinkManager.DuplicatedNodeAddressException):
             self.lm.update_node_addr(1, {1: "192.0.2.102"})
-        with self.assertRaises(ValueError):
+        with self.assertRaises(corosync.LinkManager.DuplicatedNodeAddressException):
             self.lm.update_node_addr(
                 1,
                 {
@@ -527,7 +527,7 @@ def test_update_duplicated_addr(self):
                     3: "192.0.2.65",
                 }
             )
-        with self.assertRaises(ValueError):
+        with self.assertRaises(corosync.LinkManager.DuplicatedNodeAddressException):
             self.lm.update_node_addr(
                 1,
                 {
@@ -638,7 +638,7 @@ def test_unspecified_node(self, mock_links, mock_upsert_node, mock_update_link):
             corosync.LinkNode(2, 'node2', '192.0.2.102'),
         ])]
         lm = corosync.LinkManager(dict())
-        with self.assertRaises(ValueError):
+        with self.assertRaises(corosync.LinkManager.MissingNodesException):
             lm.add_link({1: '192.0.2.201'}, dict())
         mock_upsert_node.assert_not_called()
         mock_update_link.assert_not_called()