Fixed inconsistent value types populated for LKENodePool.nodes (#446)

* Updated The LKENodePool(...).nodes attribute to only be populated with LKENodePoolNode objects

* Addressed PR comments

Author: ezilber-akamai

linode_api4/objects/lke.py

@@ -146,14 +146,29 @@ def _populate(self, json):
         Parse Nodes into more useful LKENodePoolNode objects
         """
         if json is not None and json != {}:
-            new_nodes = [
-                (
-                    LKENodePoolNode(self._client, c)
-                    if not isinstance(c, dict)
-                    else c
-                )
-                for c in json["nodes"]
-            ]
+            new_nodes = []
+            for c in json["nodes"]:
+                if isinstance(c, LKENodePoolNode):
+                    new_nodes.append(c)
+                elif isinstance(c, dict):
+                    node_id = c.get("id")
+                    if node_id is not None:
+                        new_nodes.append(LKENodePoolNode(self._client, c))
+                    else:
+                        raise ValueError(
+                            "Node dictionary does not contain 'id' key"
+                        )
+                elif isinstance(c, str):
+                    node_details = self._client.get(
+                        LKENodePool.api_endpoint.format(
+                            cluster_id=self.id, id=c
+                        )
+                    )
+                    new_nodes.append(
+                        LKENodePoolNode(self._client, node_details)
+                    )
+                else:
+                    raise TypeError("Unsupported node type: {}".format(type(c)))
             json["nodes"] = new_nodes
 
         super()._populate(json)

test/unit/objects/lke_test.py

@@ -1,5 +1,6 @@
 from datetime import datetime
 from test.unit.base import ClientBaseCase
+from unittest.mock import MagicMock
 
 from linode_api4 import InstanceDiskEncryptionType
 from linode_api4.objects import (
@@ -9,6 +10,7 @@
     LKEClusterControlPlaneOptions,
     LKENodePool,
 )
+from linode_api4.objects.lke import LKENodePoolNode
 
 
 class LKETest(ClientBaseCase):
@@ -262,3 +264,94 @@ def test_cluster_delete_acl(self):
 
             assert m.call_url == "/lke/clusters/18881/control_plane_acl"
             assert m.method == "get"
+
+    def test_populate_with_node_objects(self):
+        """
+        Tests that LKENodePool correctly handles a list of LKENodePoolNode objects.
+        """
+        self.client = MagicMock()
+        self.pool = LKENodePool(self.client, 456, 18881)
+
+        node1 = LKENodePoolNode(
+            self.client, {"id": "node1", "instance_id": 101, "status": "active"}
+        )
+        node2 = LKENodePoolNode(
+            self.client,
+            {"id": "node2", "instance_id": 102, "status": "inactive"},
+        )
+        self.pool._populate({"nodes": [node1, node2]})
+
+        self.assertEqual(len(self.pool.nodes), 2)
+        self.assertIsInstance(self.pool.nodes[0], LKENodePoolNode)
+        self.assertIsInstance(self.pool.nodes[1], LKENodePoolNode)
+        self.assertEqual(self.pool.nodes[0].id, "node1")
+        self.assertEqual(self.pool.nodes[1].id, "node2")
+
+    def test_populate_with_node_dicts(self):
+        """
+        Tests that LKENodePool correctly handles a list of node dictionaries.
+        """
+        self.client = MagicMock()
+        self.pool = LKENodePool(self.client, 456, 18881)
+
+        node_dict1 = {"id": "node3", "instance_id": 103, "status": "pending"}
+        node_dict2 = {"id": "node4", "instance_id": 104, "status": "failed"}
+        self.pool._populate({"nodes": [node_dict1, node_dict2]})
+
+        self.assertEqual(len(self.pool.nodes), 2)
+        self.assertIsInstance(self.pool.nodes[0], LKENodePoolNode)
+        self.assertIsInstance(self.pool.nodes[1], LKENodePoolNode)
+        self.assertEqual(self.pool.nodes[0].id, "node3")
+        self.assertEqual(self.pool.nodes[1].id, "node4")
+
+    def test_populate_with_node_ids(self):
+        """
+        Tests that LKENodePool correctly handles a list of node IDs.
+        """
+        self.client = MagicMock()
+        self.pool = LKENodePool(self.client, 456, 18881)
+
+        node_id1 = "node5"
+        node_id2 = "node6"
+        # Mock instances creation
+        self.client.get = MagicMock(
+            side_effect=[
+                {"id": "node5", "instance_id": 105, "status": "active"},
+                {"id": "node6", "instance_id": 106, "status": "inactive"},
+            ]
+        )
+        self.pool._populate({"nodes": [node_id1, node_id2]})
+
+        self.assertEqual(len(self.pool.nodes), 2)
+        self.assertIsInstance(self.pool.nodes[0], LKENodePoolNode)
+        self.assertIsInstance(self.pool.nodes[1], LKENodePoolNode)
+        self.assertEqual(self.pool.nodes[0].id, "node5")
+        self.assertEqual(self.pool.nodes[1].id, "node6")
+
+    def test_populate_with_mixed_types(self):
+        """
+        Tests that LKENodePool correctly handles a mixed list of node objects, dicts, and IDs.
+        """
+        self.client = MagicMock()
+        self.pool = LKENodePool(self.client, 456, 18881)
+
+        node1 = LKENodePoolNode(
+            self.client, {"id": "node7", "instance_id": 107, "status": "active"}
+        )
+        node_dict = {"id": "node8", "instance_id": 108, "status": "inactive"}
+        node_id = "node9"
+        # Mock instances creation
+        self.client.get = MagicMock(
+            side_effect=[
+                {"id": "node9", "instance_id": 109, "status": "pending"}
+            ]
+        )
+        self.pool._populate({"nodes": [node1, node_dict, node_id]})
+
+        self.assertEqual(len(self.pool.nodes), 3)
+        self.assertIsInstance(self.pool.nodes[0], LKENodePoolNode)
+        self.assertIsInstance(self.pool.nodes[1], LKENodePoolNode)
+        self.assertIsInstance(self.pool.nodes[2], LKENodePoolNode)
+        self.assertEqual(self.pool.nodes[0].id, "node7")
+        self.assertEqual(self.pool.nodes[1].id, "node8")
+        self.assertEqual(self.pool.nodes[2].id, "node9")