feat(reporter-plugin): support report rdma topology #314
Conversation
fjding
force-pushed
the
support_rdma_topology
branch
from
f60bd2d to
d42ebe2
Compare
|
|
fjding
force-pushed
the
support_rdma_topology
branch
from
d42ebe2 to
997b7ad
Compare
| @@ -50,12 +52,15 @@ func (o *KubeletPluginOptions) AddFlags(fss *cliflag.NamedFlagSets) { | |||
| "the path of kubelet resource plugin") | |||
| fs.BoolVar(&o.EnableReportTopologyPolicy, "enable-report-topology-policy", o.EnableReportTopologyPolicy, | |||
| "whether to report topology policy") | |||
| fs.BoolVar(&o.EnableReportRDMATopology, "enable-report-rdma-topology", false, "enable report rdma topology, default false") | |||
There was a problem hiding this comment.
Maybe we can use o.EnableReportRDMATopology as the default value instead of false?
pkg/agent/resourcemanager/fetcher/kubelet/topology/topology_adapter.go
Outdated
Show resolved
Hide resolved
pkg/util/cnr.go
Outdated
| @@ -42,6 +42,8 @@ const ( | |||
| CNRFieldNameTopologyZone = "TopologyZone" | |||
| CNRFieldNameResources = "Resources" | |||
| CNRFieldNameTopologyPolicy = "TopologyPolicy" | |||
|
|
|||
| ResourceRDMA = "vke.volcengine.com/rdma" | |||
There was a problem hiding this comment.
Could you make the resource name configurable? We can assign the default value to resource.katalyst.kubewharf.io, and then pass in a custom value.
| @@ -315,6 +342,42 @@ func (p *topologyAdapterImpl) addNumaSocketChildrenZoneNodes(generator *util.Top | |||
| return nil | |||
| } | |||
|
|
|||
| // addNumaSocketChildrenZoneNodes add the child nodes of socket or numa zone nodes to the generator, the child nodes are | |||
| // generated by generateZoneNode according to TopologyLevel, Type and Name in TopologyAwareAllocatableQuantityList | |||
| func (p *topologyAdapterImpl) addNICNumaChildrenZoneNodes(generator *util.TopologyZoneGenerator, | |||
There was a problem hiding this comment.
We can make this function more generic by using a map configuration (key: device.ResourceName, value: ZoneType). If the user has set this map, the function can construct NUMA's child zones by iterating through allocatableResources.Devices. If device.ResourceName is present in the map configuration, the zone name will be the device ID.
There was a problem hiding this comment.
I have refactored this pr. Could you PTAL?
fjding
force-pushed
the
support_rdma_topology
branch
from
997b7ad to
ea4c7c5
Compare
Signed-off-by: fjding <dingfangjie@bytedance.com>
Signed-off-by: caohe <caohe9603@gmail.com>
caohe
force-pushed
the
support_rdma_topology
branch
from
ea4c7c5 to
9153bef
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #314 +/- ##
==========================================
- Coverage 53.43% 53.29% -0.14%
==========================================
Files 437 437
Lines 48155 48181 +26
==========================================
- Hits 25732 25680 -52
- Misses 19505 19586 +81
+ Partials 2918 2915 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -38,6 +39,7 @@ func NewKubeletPluginOptions() *KubeletPluginOptions { | |||
| pluginapi.ResourcePluginPath, | |||
| }, | |||
| EnableReportTopologyPolicy: false, | |||
| ResourceNameToZoneNameMap: make(map[string]string), | |||
There was a problem hiding this comment.
DeviceZoneResourceNameToZoneTypeMap ?
pkg/util/cnr.go
Outdated
| @@ -373,3 +373,13 @@ func GenerateSocketZoneNode(socketID int) ZoneNode { | |||
| }, | |||
| } | |||
| } | |||
|
|
|||
| // GenerateDeviceZoneNode generates device zone node through device id, which must be unique | |||
| func GenerateDeviceZoneNode(deviceId, zoneName string) ZoneNode { | |||
There was a problem hiding this comment.
zoneType is more properly?
| for _, deviceId := range device.DeviceIds { | ||
| deviceNode := util.GenerateDeviceZoneNode(deviceId, targetZoneName) | ||
| if _, ok := zoneAllocationsMap[deviceNode]; !ok { | ||
| zoneAllocationsMap[deviceNode] = []*nodev1alpha1.Allocation{ |
There was a problem hiding this comment.
all this logic can implement in addContainerDevices, because the zoneNode should also need have allocatable and capacity when allocation has request
caohe
force-pushed
the
support_rdma_topology
branch
2 times, most recently
from
af0d880 to
5c64218
Compare
…ices Signed-off-by: caohe <caohe9603@gmail.com>
caohe
force-pushed
the
support_rdma_topology
branch
from
5c64218 to
7362121
Compare
What type of PR is this?
Features
What this PR does / why we need it:
Support reporting devices (such as RDMA, GPU, etc.) as topology zones under NUMA, thereby supporting inter-RDMA affinity at switch level.
Which issue(s) this PR fixes:
Special notes for your reviewer: