✨ Configurable QoS resources for cluster-manager and klusterlet operators and their managed containers #400
Conversation
pkg/cmd/init/cmd.go
Outdated
| @@ -54,6 +54,17 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream | |||
| "If set, the command will initialize the OCM control plan in foreground.") | |||
| cmd.Flags().StringVarP(&o.output, "output", "o", "text", "output foramt, should be json or text") | |||
| cmd.Flags().BoolVar(&o.singleton, "singleton", false, "If true, deploy singleton controlplane instead of cluster-manager. This is an alpha stage flag.") | |||
| cmd.Flags().StringVar(&o.operatorResourceQosClass, "operator-resource-qos-class", "Default", "the resource QoS class of the cluster manager operator. Can be one of Default or BestEffort") | |||
There was a problem hiding this comment.
I think we could ref the command here https://kubernetes.io/docs/reference/kubectl/generated/kubectl_set/kubectl_set_resources/.
something like:
clusteradm init --resource-requests=cpu=xxx,memory=xxx --resource-limits=cpu=xxx,memory=xxx
it is not necessary to specify the type, we can decide type if resource-requests is set.
There was a problem hiding this comment.
I updated the flag value format as you suggested.
type is still necessary I suppose because we still need to know it's Default or BestEffort if limits and requests are not set.
dongbeiqing91
force-pushed
the
dev/qos
branch
from
e7e288f to
ba32dae
Compare
pkg/cmd/init/cmd.go
Outdated
| @@ -54,6 +54,13 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream | |||
| "If set, the command will initialize the OCM control plan in foreground.") | |||
| cmd.Flags().StringVarP(&o.output, "output", "o", "text", "output foramt, should be json or text") | |||
| cmd.Flags().BoolVar(&o.singleton, "singleton", false, "If true, deploy singleton controlplane instead of cluster-manager. This is an alpha stage flag.") | |||
| cmd.Flags().StringVar(&o.operatorResourceQosClass, "operator-resource-qos-class", "Default", "the resource QoS class of the cluster manager operator. Can be one of Default, BestEffort or ResourceRequirement.") | |||
| cmd.Flags().StringToStringVar(&o.operatorResourceLimits, "operator-resource-limits", nil, "the resource limits of the cluster manager operator, for example: cpu=800m,memory=800Mi") | |||
There was a problem hiding this comment.
could we also update the flag for operator. Actually, why do we need to set operator separately? Can we just use the same resource-limits flag also for operator?
I am thinking we can start with using the same flat for both operator and all other containers. We could add flat specifically for operator later if there is a real requirement, WDYT?
There was a problem hiding this comment.
I am OK with that.
Updated.
dongbeiqing91
force-pushed
the
dev/qos
branch
3 times, most recently
from
59aea94 to
e0c9aa2
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: promid, qiujian16 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| return nil, err | ||
| } | ||
| rr.Requests[corev1.ResourceName(rsc)] = quantity | ||
| } |
There was a problem hiding this comment.
not sure if it is necessary to add some checks to ensure that the requirements <= limits?
|
@promid, thanks for your PR, this is great! generally lgtm, and left some nit comments. |
dongbeiqing91
force-pushed
the
dev/qos
branch
from
e0c9aa2 to
81c994a
Compare
…rs and their managed pods Signed-off-by: Dong Beiqing <[email protected]>
dongbeiqing91
force-pushed
the
dev/qos
branch
from
81c994a to
794faa9
Compare
|
@qiujian16 @zhujian7 Can you PTAL the e2e CI? |
|
the e2e faiils, I think you need to run the test locally to check...I guess the API change brings some compatible issue with the released version. |
|
local e2e passed @qiujian16 
|
|
/lgtm |
openshift-merge-bot
bot
merged commit c384319
into
open-cluster-management-io:main
Summary
Configurable QoS resources for cluster-manager and klusterlet operators and their managed pods
Related issue(s)
Related PRs
open-cluster-management-io/ocm#351
open-cluster-management-io/api#316
Fixes #