Support dots in Kind name segment

[adrianchiris]

CDI spec states that dots ('.') are allowed.
Add support in parser for this.

reference: https://github.com/container-orchestrated-devices/container-device-interface/blob/main/SPEC.md#kind

[elezar]

Thanks for the contribution @adrianchiris. I don't think I have an issue with this.

@klihub can you recall why dots were not included in device classes when this was initially implemented?

One thing that would also have to be added is that we would have to add a check and an associated test to the requiresV060 function in pkg/cdi/version.go. This would mean that a lower spec version can be generated if a non-dotted class name is used to ensure better compatibility with our clients.

[adrianchiris]

Ack, thx for the feedback will work on adding the needed check & test

[adrianchiris]

One thing that would also have to be added is that we would have to add a check and an associated test to the requiresV060 function in pkg/cdi/version.go. This would mean that a lower spec version can be generated if a non-dotted class name is used to ensure better compatibility with our clients.

Done.

[elezar]

lgtm

@klihub @bart0sh any comments here?

[klihub]

Thanks for the contribution @adrianchiris. I don't think I have an issue with this.

@klihub can you recall why dots were not included in device classes when this was initially implemented?

I think it was simply considered unnecessary (there was no immediate need for it) and cleaner to only accept dots in the vendor part where it was clearly needed (to allow for DNS-like vendor prefixes).

[klihub]

lgtm

@klihub @bart0sh any comments here?

I have no strong feelings. I don't see why dots would be necessary or useful in the class part. But you guys have been playing around much more with generating Specs and if you say it is necessary/helpful, then I believe you.

CDI spec states that dots ('.') are allowed.
Add support in parser for this.

A much simpler alternative would have been to simply update the docs to reflect the implementation, stating that dots are only allowed in the vendor prefix. But as I said, no strong feelings and I'm fine with this.

[bart0sh]

I'm ok with this.

[adrianchiris]

Some context on what triggered me to add this:

im working on adding support via DRA to network resources for NVIDIA NICs
part of this work is to generate CDI files for container runtime to consume.

there may be different network device types (e.g SRIOV virtual function[1] and subfunctions[2])

So, i was thinking on how to name them in CDI.

as spec allowed dots in name segment this is what I had in mind for the kind string (and its what triggered this PR):

nvidia.com/net.sf=<SF device name>
nvidia.com/net.vf=<VF device address>

since the first part usually refers to the vendor (at least that what i got from the examples within CDI proj)
it made sense to me to choose the kind string as above.

some alternatives that conform to current implementation (without dot in name segment)

nvidia.com/net-sf=<SF device name>
nvidia.com/net-vf=<VF device address>

net.nvidia.com/sf=<SF device name>
net.nvidia.com/vf=<VF device address>

[1] https://en.wikipedia.org/wiki/Single-root_input/output_virtualization
[2] https://docs.kernel.org/networking/devlink/devlink-port.html#subfunction

[elezar]

I discussed this briefly in the COD Working group meeting, and have no objections. This also aligns with what is allowed for Kubernetes Annotations and does provide more flexibility from a user perspective.

@adrianchiris as a last update, please add a line to the "Released versions of the spec are available as Git tags." table in SPEC.md that indicates this relaxation. (I think we're missing one for the relaxation of the device name too which I will add in a follow-up).

Note: In terms of generating the spec, it would be useful to make use of the DetectMinimumVersion function (for example) to ensure the widest compatibility for consumers based on content).

[adrianchiris]

I discussed this briefly in the COD Working group meeting, and have no objections. This also aligns with what is allowed for Kubernetes Annotations and does provide more flexibility from a user perspective.

Thanks for bringing this one up for discussion.

@adrianchiris as a last update, please add a line to the "Released versions of the spec are available as Git tags." table in SPEC.md that indicates this relaxation. (I think we're missing one for the relaxation of the device name too which I will add in a follow-up).

Done. updated v0.6.0 entry. LMK if wording is OK

[bart0sh]

@elezar Isn't it a backward-incompatible change? If it is we should consider incrementing spec version.

[elezar]

@elezar Isn't it a backward-incompatible change? If it is we should consider incrementing spec version.

It is backward incompatible, but we haven't released (tagged) the v0.6.0 spec yet. Meaning that this change would be included as part of the v0.6.0 spec once we tag that.

[elezar]

Thanks @adrianchiris.

This looks good now. (Minor formatting suggestion for the spec table).

[adrianchiris]

@bart0sh @elezar we OK with merging this one ?