smt_via_label via machine config #47
Conversation
| @@ -38,6 +38,10 @@ | |||
| when: powervm_rmc | |||
| import_tasks: powervm_rmc.yaml | |||
|
|
|||
| - name: Allow SMT configuration via node label (SMT=X) | |||
| when: ansible_architecture == "ppc64le" | |||
| import_tasks: smt_configuration.yaml | |||
There was a problem hiding this comment.
Let's make this task optional. I'm not very comfortable fiddling with SMT settings and would like to keep this optional and run through some iterations of e2e tests to verify stability.
There was a problem hiding this comment.
you mean adding a variable smt_control (true|false) in the playbook ?
There was a problem hiding this comment.
done in my last commit
| done | ||
| let INITOFFTHREAD=$INITOFFTHREAD+$MAXSMT | ||
| done | ||
| systemctl restart kubelet |
There was a problem hiding this comment.
Since we are rebooting the node, we should avoid restarting kubelet
There was a problem hiding this comment.
Btw, is this setting persistent when done in this way?
There was a problem hiding this comment.
yes it is persistent as it seems to start before kubelet.
Do you think it is really dangerous to restart kubelet ?
There was a problem hiding this comment.
I'm not sure how MCO will behave if kubelet is restarted. Additionally since anyway node reboot will happen post application of the machine config then why to restart kubelet.
There was a problem hiding this comment.
MCO deploy a systemd service which start a script that monitor every 30s the status of the SMT label for the current node.
If this label is set (or changed) the script change the SMT mode on the current node and restart kubelet to update the CPU resource count without rebooting the node (which is REALLY long on powerVM).
So the main idea is to avoid a reboot when changing mode. (and because I’m 90% sure that users will always forget to reboot the node to refresh the kubelet)
There was a problem hiding this comment.
There is kubelet, there is crio and then there is impact to the containers that are already running. Some scenarios that I can think of and have no idea how will it get handled if kubelet is restarted.
- number of logical cpus < total cpu requests for PODs
- number of logical cpus < total cpu limits for PODs
I agree reboot will be longer process. However according to me the safest way to set the SMT value during install time (as a oneshot service) via machineconfig and letting MCO take care of the reboot.
Let's think through this and get some datapoints. The base code as part of this PR is good start, it's just finding the right way and agreeing to it.
| echo "Target SMT must be smaller or equal than Maximum SMT supported" | ||
| fi | ||
| fi | ||
| /bin/sleep 30 |
There was a problem hiding this comment.
What is not needed ? sleep 30 is here because of the while loop...
script start and loop forever, checking node SMT label every 30s.
|
@schabrolles the commits will need squashing to make it a single commit :-) |
|
Yes, sure I will do that soon... just finishing testing and adding modification from your comments |
Fixes: ocp-power-automation#46 Signed-off-by: Sebastien Chabrolles <[email protected]>
schabrolles
force-pushed
the
smt_via_label
branch
from
16971bd
to
f1a52e2
Compare
|
@schabrolles: PR is not mergeable. The PR state is: DIRTY Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
1 similar comment
|
@schabrolles: PR is not mergeable. The PR state is: DIRTY Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/meow |
Fixes: #46
Signed-off-by: Sebastien Chabrolles [email protected]