-
Notifications
You must be signed in to change notification settings - Fork 15
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
base: master
Are you sure you want to change the base?
Changes from 15 commits
dc95c38
84ecd61
0ef2bc1
4c08d74
7b19c26
0ad3d4e
a92fbae
1266491
7c40dfd
e309a67
7f5f459
880c0cd
09e3b0c
ef73470
40828a7
a5590cd
6874c63
2fa9587
b3dae41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe 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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
There was a problem hiding this comment.
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 ?