PodResources interface enhancements

[AlexeyPerevalov]

This change is necessary for resource with topology exporting daemon,
which used in topology aware scheduling.
This PR proposes following changes:

• Adds cpuids and hardware topology of devices into podresource interface
• Extends podresources interface by Watch endpoint
• Extends podresources interface by GetAllocatableResources endpoint

Information about CPU is keeping in cpu_ids, since it's enough to
represent both quantity and numaid. NUMAid can be obtained from
cadvisor MachineInfo, since id in cpus_ids is a thread_id.
This API doesn't provide cpu fraction, since it could be obtained from
Pod's request/limits and in case of non-integer CPU quantity and
non-guaranteed QoS cpu assigned is not exclusive and NUMA id is not
interesting.

Issue: #2043

[dashpole]

Are these changes applicable to both windows and linux?

[AlexeyPerevalov]

Are these changes applicable to both windows and linux?

Changes tightly coupled with TopologyManager on the worker node, but now only fake TopologyManager is created on the Windows. So now it's not necessary for Windows hosts.
Only linux hosts are supported for both CPUManager and TopologyManager. Technically it's possible to implement all these features on Windows, but probably nobody supported it.

[AlexeyPerevalov]

btw, deviceplugin represents numaid as int64. Not so clear why it was done in such way, since 64 bit to much for numa id and sign also not necessary.

[dashpole]

Interesting. In that case, uint32 is fine, as long as we handle the conversion correctly.

[AlexeyPerevalov]

I took a look at linux kernel implementation. It represents internally both cpu & numaid as int (https://github.com/torvalds/linux/blob/6f0d349d922ba44e4348a17a78ea51b7135965b1/arch/x86/mm/numa.c#L62), but numa id on the most architectures has a range (https://github.com/torvalds/linux/blob/8dcd175bc3d50b78413c56d5b17d4bddd77412ef/arch/x86/Kconfig#L1590 and https://github.com/torvalds/linux/blob/6f0d349d922ba44e4348a17a78ea51b7135965b1/include/linux/numa.h#L12) so now on x86_64 it's limited by 2^10. But I'm already afraid, so let's use int64 as in deviceplugin )

[dashpole]

Changes tightly coupled with TopologyManager on the worker node, but now only fake TopologyManager is created on the Windows. So now it's not necessary for Windows hosts.
Only linux hosts are supported for both CPUManager and TopologyManager. Technically it's possible to implement all these features on Windows, but probably nobody supported it.

Yeah, I know it isn't implemented now. But if possible, we should ensure the same concepts exist across OSs so someone could implement them and get similar functionality in the future if they wanted to. I googled a bit, and it seems like the concept of a numa node exists in windows as well, so this addition should be fine. It might be good to confirm this with sig-windows.

cc @PatrickLang

[dashpole]

This looks mostly good, assuming the other KEP is accepted.

[AlexeyPerevalov]

This looks mostly good, assuming the other KEP is accepted.

Thank you, the most problem now it's where to land the daemon and code for CRD.

[AlexeyPerevalov]

so on the node with e.g. 96 core (I've it now), we will see
[0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83
84 85 86 87 88 89 90 91 92 93]
for any best-effort pods, I think it's too long list (it's not about UX, since it's not for humans), but we can represent it shorter just like kernel it does, e.g. 0-96.
I suggest list format from here http://man7.org/linux/man-pages/man7/cpuset.7.html#FORMATS

[kad]

Using cpuset as a format (and especially in the string form) is really not the best idea in terms of engineering of APIs. It means that we are loosing validation on the grpc/protobuf level, and in additional all clients that supposed to consume that field will be required to implement parser for cpuset format (which is also non-trivial in terms of syntax and validation).
Having repeated uint allows to properly validate content and handle situations with empty/undefined conditions of this field cheaply.

[AlexeyPerevalov]

ok, no problem, cpuset format is mostly for human readability (it's not about unix domain socket). I just was focusing on golang/python clients (where cpuset parsing is publicly available),

[kad]

To best of my knowledge, neither Python nor Go std libraries have cpuset string format parser available.
Passing string puts additional technical debt on the clients, either by implementing parser or by pulling non-std library.

[dashpole]

do we even need to put the cpusets of containers which are using the shared pool? Should we just have a global default cpu set, and omit all pods using shared CPUs? Or just omit containers using the shared pool all-together?

I agree that concrete lists of integers is better than cpuset notation.

[kad]

instead of re-inventing wheel here, it will be more valuable to simply re-use TopologyInfo type from device plugin APIs. Because that's practically what you can map to with 100% certainty, as device plugins reports mapping device_id/TopologyInfo.

Also, TopologyInfo is device_id specific. If you have repeated device_ids, you should have repeated numaid. Or better represent device allocation in the manner that re-uses device plugin APIs and structures.

[AlexeyPerevalov]

I just tried to keep backward compatibility. Additional field at the end of the structure doesn't break backward compatibility.
In case when one resource assigned to different numa node (e.g. it has 2 devices) in this interface looks like 2 instances of ContainerDevices, in this case we'll know numa of each device (but for scheduling it's not necessary, it's enough to know about resource).
If I truly understand you, if we have TopologyInfo (it's repeated NUMANode nodes) instead of numaid. We still know numa ids of resource, but doesn't know numa id of devices.
I'm ok with both: current one and your suggestion, but I want to know @dashpole opinion.

[kad]

I'm talking about different problem here. resource name is 'vendor.example.com/type`. it has instances 'id0','id1',...'id47'. IDs from 0 to 17 are from PCI bus 0. IDs from 18 to 35 are from PCI bus 1, and IDs 36-47 are from PCI bus 3. 'NUMA ID' is not about whole resource, it is property of each instance of particular resource type announced by device plugins.

[dashpole]

Its probably best to keep each API self-contained. We may want to introduce fields in the device plugin API that we don't want here, or vice-versa. I don't have any objections to modeling this API after the device plugin API, but we shouldn't use the types directly.

[AlexeyPerevalov]

I'm talking about different problem here. resource name is 'vendor.example.com/type`. it has instances 'id0','id1',...'id47'. IDs from 0 to 17 are from PCI bus 0. IDs from 18 to 35 are from PCI bus 1, and IDs 36-47 are from PCI bus 3. 'NUMA ID' is not about whole resource, it is property of each instance of particular resource type announced by device plugins.

In this case we will have 2 instances of ContainerDevices. 2 instances will have the same resource name, different list of device ids and different numaid.

[kad]

Each device instance can have repeated numa_id field. Please re-check definitions of structures how device plugins are announcing device instances upward to the kubelet.

[AlexeyPerevalov]

yes, I know it. And this interface change takes it into account: If we have only dev1 and it attaches to NUMA id 1 and 2, in this case we will have
ContainerDevices [ {resource_name: "res1", ["dev1"], 1}, {resource_name: "res1", ["dev2"], 2}]

[kad]

Your example shows "dev1" and "dev2". I'm talking about "dev1" with 1,2 NUMA IDs from TopologyInfo.

[AlexeyPerevalov]

oh, yes, thank you. How about this:
ContainerDevices [ {resource_name: "res1", ["dev1"], 1}, {resource_name: "res1", ["dev1"], 2}]
I'll add such test case.

[swatisehgal]

@AlexeyPerevalov @kad @fromani Based on the API, a representation of this could be: ContainerDevices [ {resource_name: "res1", device_ids: ["dev1", "dev2"], topology: 0 }, {resource_name: "res1", device_ids: ["dev3", "dev4"], topology: 1}, {resource_name: "res2", device_ids: ["dev1"], topology: 0},{resource_name: "res2", device_ids: ["dev2", "dev3"], topology: 1},] .

So essentially we are saying that we would have containersDevices grouped by resourcename and topology info. We need to document this assumption very carefully and provide examples of the intended use of the API as it is not very intuitive especially given that this information is obtained device plugin API where a Device data structure looks like below storing its corresponding device id, health and topology info:

message Device {
	// A unique ID assigned by the device plugin used
	// to identify devices during the communication
	// Max length of this field is 63 characters
	string ID = 1;
	// Health of the device, can be healthy or unhealthy, see constants.go
	string health = 2;
	// Topology for device
	TopologyInfo topology = 3;
}

The information of all the devices known to the device manager are stored as below:
allDevices map[string]map[string]pluginapi.Device // map[resourceName][ID]pluginapi.Device

[kad]

How come the information about already allocated resources can be used for future scheduling?
Device IDs are dynamic objects, device plugins can add/remove instances of the devices on the fly.
Previous information about same device ID can't be re-used for future decisions with 100% certainty.

[AlexeyPerevalov]

We mostly interesting in resource and its numaid. Deviceid itself is not so interesting.

[kad]

We mostly interesting in resource and its numaid. Deviceid itself is not so interesting.

What do you mean by that statement? numa_id can't be assigned to container. It is a property of other resource instance.

[AlexeyPerevalov]

we're using the quantity of resources per numa node, that quantity is calculated by number of element in device list. We don't use device id here.

numa_id can't be assigned to container. It is a property of other resource instance.

---

Frankly saying I didn't find container word in the thread above, except yours.
Yes it can't be, since linux kernel doesn't have container entity, not so clear why container is mentioned here.

[kad]

"Container" word comes from changes below in ContainerDevices. Device instance can be uniquely mapped to what is announced by device plugins. Device plugins announcing device instances with TopologyInfo filed. this field contains repeated numa_id per device instance.

[swatisehgal]

In this KEP we are enabling support for obtaining information about kubelet's assignment of cpus to containers in addition to devices. I think we should update the title of this KEP to reflect that too. From the current title it seems to be only catering to devices.

[AlexeyPerevalov]

I think, yes ) I'll update.

[kad]

"Container" word comes from changes below in ContainerDevices. Device instance can be uniquely mapped to what is announced by device plugins. Device plugins announcing device instances with TopologyInfo filed. this field contains repeated numa_id per device instance.

[kad]

What does it mean "containers with NUMA id"? NUMA ID in Linux kernel terms or in hardware terms is not something that can be linked to container directly.

[AlexeyPerevalov]

"NUMA id" was related to cpus and devices in this sentence, ok, I'll rephrase it.

[kad]

Each device instance can have repeated numa_id field. Please re-check definitions of structures how device plugins are announcing device instances upward to the kubelet.