Welcome @belo4ya!

It looks like this is your first PR to volcano-sh/volcano 馃帀.

Thank you, and welcome to Volcano. 😃

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign k82cn
You can assign the PR to them by writing /assign @k82cn in a comment when ready.

The full list of commands accepted by this bot can be found here.

/assign @k82cn

How about set IgnoredDevicesList in nodeInfo?

How about set IgnoredDevicesList in nodeInfo?

Hi, any progress here?

any update? i can help u finish this pr, @belo4ya

any update? i can help u finish this pr, @belo4ya

I glanced at the idea of linking IgnoredDevicesList with NodeInfo - and it turned out that there is already such a relationship:

ni.Others[GPUSharingDevice].(Devices).GetIgnoredDevices(),
ni.Others[vgpu.DeviceName].(Devices).GetIgnoredDevices(),

Then, for each filtering operation of ignored resources, an IgnoredDevicesList could be calculated locally:

        var niList []NodeInfo
	ignoredDevicesList := map[string]struct{}{}
	for _, ni := range niList {
		for _, device := range ni.Others[GPUSharingDevice].(Devices).GetIgnoredDevices() {
			ignoredDevicesList[device] = struct{}{}
		}
		for _, device := range ni.Others[vgpu.DeviceName].(Devices).GetIgnoredDevices() {
			ignoredDevicesList[device] = struct{}{}
		}
	}

But I have a sharp contradiction when I go further. I don't fully understand the current work with IgnoredDevicesList - it doesn't seem right to me.

And at the same time, it seems to me that NodeInfo and IgnoredDevicesList are different levels of abstraction. IgnoredDevicesList is calculated at the time of NodeInfo initialization, and the further (and previous) initialization of NodeInfo depends on IgnoredDevicesList. And IgnoredDevicesList is an aggregate that is useless before initializing all NodeInfo.

That is, I mean that IgnoredDevicesList is not an object that should exist inside NodeInfo as an attribute. And this is an object that should be at the level with nodes []*v1.Node

It seems that such a change will require major edits and I would like to know your opinion, how do you see how this should be implemented? @Monokaix, @Vacant2333

any update? i can help u finish this pr, @belo4ya

I glanced at the idea of linking IgnoredDevicesList with NodeInfo - and it turned out that there is already such a relationship:

ni.Others[GPUSharingDevice].(Devices).GetIgnoredDevices(),
ni.Others[vgpu.DeviceName].(Devices).GetIgnoredDevices(),

Then, for each filtering operation of ignored resources, an IgnoredDevicesList could be calculated locally:

        var niList []NodeInfo
	ignoredDevicesList := map[string]struct{}{}
	for _, ni := range niList {
		for _, device := range ni.Others[GPUSharingDevice].(Devices).GetIgnoredDevices() {
			ignoredDevicesList[device] = struct{}{}
		}
		for _, device := range ni.Others[vgpu.DeviceName].(Devices).GetIgnoredDevices() {
			ignoredDevicesList[device] = struct{}{}
		}
	}

But I have a sharp contradiction when I go further. I don't fully understand the current work with IgnoredDevicesList - it doesn't seem right to me.

And at the same time, it seems to me that NodeInfo and IgnoredDevicesList are different levels of abstraction. IgnoredDevicesList is calculated at the time of NodeInfo initialization, and the further (and previous) initialization of NodeInfo depends on IgnoredDevicesList. And IgnoredDevicesList is an aggregate that is useless before initializing all NodeInfo.

That is, I mean that IgnoredDevicesList is not an object that should exist inside NodeInfo as an attribute. And this is an object that should be at the level with nodes []*v1.Node

It seems that such a change will require major edits and I would like to know your opinion, how do you see how this should be implemented? @Monokaix, @Vacant2333

Yeah, we can keep this: )