blockio: add blockio_reconfigure option#7418
blockio: add blockio_reconfigure option#7418askervin wants to merge 1 commit intocontainerd:mainfrom
Conversation
|
Hi @askervin. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
This patch solves the issue pointed out by @d-honeybadger: blockio parameters are not applied to block devices that are created after containerd restart. |
askervin
force-pushed
the
5Kv_blockio_reconfigure
branch
from
9c4e073 to
d460cf1
Compare
|
Does it affect the pods/containerds which are already created? For example, pod A has throttle of 100M bps, after changing the configuration, pod A should be throttled to 200M according to configuration. What will pod A do then ? Keep 100M or change to 200M ? |
New configuration is applied only on containers that are created after the configuration change. If containers in your pods are restarted in case of a crash, you can force recreating a container without restarting its pod ( Online adjustment of this kind of parameters without disrupting running containers could be done with NRI plugins. |
|
An accidental |
| if !blockIOEnabled || !blockIOReconfigure || blockIOConfigFilePath == "" { | ||
| return nil | ||
| } | ||
| err := blockio.SetConfigFromFile(blockIOConfigFilePath, true) |
There was a problem hiding this comment.
nit:
Looking at this function, it looks quite heavy as it iterates all schedulers and all devices.
TODO: Maybe we need a better way to do that. (may need intel/goresctl side updates)
There was a problem hiding this comment.
Thanks @gjkim42, this is definitely a valid point. goresctrl can, for instance, skip scanning io schedulers of those block devices that it already knows.
Another thing that crossed my mind is that it would be nice to re-read block devices only once for a pod. However, it's still possible that something more hackish happens, like an init container creates new block device to the system. This patch is very conservative in this sense: it makes sure that a container always gets the most recent configuration for all block devices when created. I'd leave more complex logic and optimizations to later changes.
|
@askervin Could you pls resolve conflicts? |
Reconfiguring blockio is needed in the following scenarios. - Block devices (/dev/*) change, for example when a CSI driver adds a new block device for a pod. Reconfiguration finds current devices and enables applying appropriate blockio parameters on them. - Blockio class configuration file has been updated in the system. Reconfiguration applies changes without restarting containerd. Signed-off-by: Antti Kervinen <[email protected]>
askervin
force-pushed
the
5Kv_blockio_reconfigure
branch
from
d460cf1 to
e07be9c
Compare
|
@mxpv, thanks for pinging, rebasing done. |
|
PR needs rebase. DetailsInstructions 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. |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed. |
|
This PR was closed because it has been stalled for 7 days with no activity. |
6 participants
Reconfiguring blockio is needed in the following scenarios.
Signed-off-by: Antti Kervinen [email protected]