uncordon the node #80
Conversation
lint changes
| @@ -343,6 +343,22 @@ def mark_node_unschedulable(self, node_name: str) -> Optional[ResourceInstance]: | |||
| logger.error("Exception encountered while marking node unschedulable: %s\n", e) | |||
| return api_response | |||
|
|
|||
| def make_node_uncordon(self, node_name: str) -> Optional[ResourceInstance]: | |||
There was a problem hiding this comment.
@adtandale could you please help me understand why would we need this ?
| body = { | ||
| "spec": { | ||
| "taints": [{"effect": "NoSchedule", "key": "node.kubernetes.io/unschedulable"}], | ||
| "unschedulable": false, |
| @@ -343,6 +343,26 @@ def mark_node_unschedulable(self, node_name: str) -> Optional[ResourceInstance]: | |||
| logger.error("Exception encountered while marking node unschedulable: %s\n", e) | |||
| return api_response | |||
|
|
|||
| def make_node_uncordon(self, node_name: str) -> Optional[ResourceInstance]: | |||
There was a problem hiding this comment.
Unless I am mistaken, I think making cordon is equivalent to making the node unschedulable so this should be the opposite, as in 'make_node_cordoned' correct?
| worker_node_list = node_api_obj.get_worker_nodes() | ||
| for worker_node_name in worker_node_list.items: | ||
| api_response = node_api_obj.make_node_uncordon(node_name=worker_node_name.metadata.name) | ||
| assert api_response.kind == "Node" |
There was a problem hiding this comment.
Maybe add an assert to make sure that the key 'taints' is present under 'spec'
| @@ -340,6 +340,30 @@ def test_mark_node_schedulable(self, setup_params): | |||
| api_response = node_api_obj.mark_node_schedulable(node_name=worker_node_name.metadata.name) | |||
| assert api_response.kind == "Node" | |||
|
|
|||
| def test_make_node_uncordon(self, setup_params): | |||
There was a problem hiding this comment.
The test should return the node to its original state. I think we can either add a method that does the opposite and provide a test for it or simply clean up after the test. I am leaning towards the first option.
There was a problem hiding this comment.
okay, so we should write a method to cordon the node and then uncordon it again to convert it into its original state?
There was a problem hiding this comment.
Correct. I think that's the best resolution.
| :param node_name: | ||
| :return: | ||
| """ | ||
| body = { |
There was a problem hiding this comment.
I don't think it's sufficient to just set 'unschedulable' to False. We should be removing the whole 'taints' element from 'spec'. We can do this by passing "op": "remove", "path": "....
Please refer to the following:
https://stackoverflow.com/questions/27439986/what-is-the-json-patch-format-to-remove-an-element-from-an-array
There was a problem hiding this comment.
okay, I do agree, thanks!
There was a problem hiding this comment.
For this jsonpatch module needs to be installed explicitly using pip.
No description provided.