Add k8s settings: prefer-closest-numa-nodes and max-allowable-numa-nodes#117
Conversation
piyush-jena
force-pushed
the
add-k8s-settings
branch
from
e732aae to
169e39f
Compare
|
Can you add sign-off in the commit. |
| cpu_manager_policy_options: Vec<KubernetesCPUManagerPolicyOption>, | ||
| topology_manager_scope: TopologyManagerScope, | ||
| topology_manager_policy: TopologyManagerPolicy, | ||
| topology_manager_policy_options: KubernetesTopologyManagerPolicyOptions, |
There was a problem hiding this comment.
Should this be TopologyManagerPolicyOptions? The sibling types TopologyManagerScope and TopologyManagerPolicy don't have the Kubernetes prefix, and the parent struct already establishes the Kubernetes context.
There was a problem hiding this comment.
The more closer equivalent setting is the cpu_manager_policy_options of type Vec<KubernetesCPUManagerPolicyOption>. Maybe that's why AI generated it this way but your comment definitely applies to KubernetesMaxAllowableNumaNodesValue.
There was a problem hiding this comment.
After going through other settings it seems like Kubernetes prefix is used in other types as well and seems to be the appropriate one here as described above. However, there is no good resource that tells us when this should be used. But I think setting types seem to have the prefix and the underlying settings don't in most cases.
|
|
||
| /// KubernetesTopologyManagerPolicyOptions represents the topology manager policy options | ||
| /// for the kubelet. These are rendered as `topologyManagerPolicyOptions` in KubeletConfiguration. | ||
| /// Upstream source: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/cm/topologymanager/policy_options.go |
There was a problem hiding this comment.
Nit: can we use a permanent link here?
Signed-off-by: Piyush Jena <jepiyush@amazon.com>
Signed-off-by: Piyush Jena <jepiyush@amazon.com>
piyush-jena
force-pushed
the
add-k8s-settings
branch
2 times, most recently
from
fd281d7 to
635254c
Compare
Signed-off-by: Piyush Jena <jepiyush@amazon.com>
piyush-jena
force-pushed
the
add-k8s-settings
branch
from
635254c to
17f2309
Compare
Signed-off-by: Piyush Jena <jepiyush@amazon.com>
piyush-jena
force-pushed
the
add-k8s-settings
branch
from
17f2309 to
beff7cb
Compare
|
force push addresses @arnaldo2792 's suggestion. |
4 participants
Issue # bottlerocket-os/bottlerocket#4750
Related to:
Description of changes:
Add 2 topology manager policy options:
max-allowable-numa-nodes- GA k8s-1.35+prefer-closest-numa-nodes- GA k8s-1.32+Testing details available in bottlerocket-os/bottlerocket-core-kit#848
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.