Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uncordon the node #80

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
20 changes: 20 additions & 0 deletions piqe_ocp_lib/api/resources/ocp_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adtandale could you please help me understand why would we need this ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

"""
Return node which was make uncordon
:param node_name:
:return: A V1Node object. None on failure.
"""
api_response = None
body = {
"spec": {
"taints": [{"effect": "NoSchedule", "key": "node.kubernetes.io/unschedulable"}],
"unschedulable": false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be True?

}
}
try:
api_response = self.ocp_nodes.patch(name=node_name, body=body)
logger.info("Node %s maked uncordon" % node_name)
except ApiException as e:
logger.error("Exception encountered while making node uncordon: %s\n", e)
return api_response

def are_all_nodes_ready(self) -> bool:
"""
Return the status of all node based on the condition type Ready.
Expand Down
24 changes: 24 additions & 0 deletions piqe_ocp_lib/tests/resources/test_ocp_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, so we should write a method to cordon the node and then uncordon it again to convert it into its original state?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I think that's the best resolution.

"""
Verify that a masters/worker nodes are maked uncordon
1. Call get_master_nodes method via a ocp_nodes instance
2. Call make_node_uncordon method
3. Verify that the response object has uncordon set to None
4. Call get_worker_nodes method via a ocp_nodes instance
5. Call make_node_uncordon method
6. Verify that the response object has uncordon set to True
:param setup_params:
:return:
"""
node_api_obj = setup_params["node_api_obj"]
# Mark all master nodes uncordon
master_node_list = node_api_obj.get_master_nodes()
for master_node_name in master_node_list.items:
api_response = node_api_obj.make_node_uncordon(node_name=master_node_name.metadata.name)
assert api_response.kind == "Node"
# Mark all worker nodes uncordon
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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add an assert to make sure that the key 'taints' is present under 'spec'


def test_are_all_nodes_ready(self, setup_params):
"""
Verify that all nodes status is Ready.
Expand Down