Skip to content

Commit

Permalink
Fixed inconsistent value types populated for LKENodePool.nodes (#446)
Browse files Browse the repository at this point in the history
* Updated The LKENodePool(...).nodes attribute to only be populated with LKENodePoolNode objects

* Addressed PR comments
  • Loading branch information
ezilber-akamai authored Aug 1, 2024
1 parent 0c0e649 commit 72e6de4
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 8 deletions.
31 changes: 23 additions & 8 deletions linode_api4/objects/lke.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
93 changes: 93 additions & 0 deletions test/unit/objects/lke_test.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -9,6 +10,7 @@
LKEClusterControlPlaneOptions,
LKENodePool,
)
from linode_api4.objects.lke import LKENodePoolNode


class LKETest(ClientBaseCase):
Expand Down Expand Up @@ -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")

0 comments on commit 72e6de4

Please sign in to comment.