Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in specification regarding GC?? #1101

Closed
kanpov opened this issue Jun 27, 2024 · 5 comments · Fixed by #1103
Closed

Error in specification regarding GC?? #1101

kanpov opened this issue Jun 27, 2024 · 5 comments · Fixed by #1103

Comments

@kanpov
Copy link

kanpov commented Jun 27, 2024

CNI spec says extra JSON key in GC is cni.dev/attachments yet libcni uses cni.dev/valid-attachments? What is it then if the spec doesn't even correspond to the official runtime?

@kanpov
Copy link
Author

kanpov commented Jun 27, 2024

I'm asking this because I'm developing a CNI runtime and this lack of clarity is a problem (runtime in question: https://github.com/FirecrackerSharp/CniDotNet/tree/main)

@kanpov kanpov changed the title GC mismatch?? Error in specification regarding GC?? Jun 27, 2024
@squeed
Copy link
Member

squeed commented Jul 1, 2024

Oops! That is a very silly mistake indeed.

My plan is to change the spec to match libcni, as libni is probably more directly meaningful. In the unlikely event that there is spec-conformant code, I will set both keys. Make sense?

@kanpov
Copy link
Author

kanpov commented Jul 3, 2024

Oops! That is a very silly mistake indeed.

My plan is to change the spec to match libcni, as libni is probably more directly meaningful. In the unlikely event that there is spec-conformant code, I will set both keys. Make sense?

Yes, probably, but I think the spec should keep cni.dev/attachments and not be changed, but only libcni is changed to actually adhere the spec. What do you think?

@bleggett
Copy link
Contributor

bleggett commented Jul 8, 2024

My $0.02 is that

  • cni.dev/valid-attachments is a better name anyway
  • We don't have meaningful back compat concerns at this point for this feature

So tweaking the spec and throwing the back compat concern into libcni like #1103 does puts us into a better state.

@kanpov
Copy link
Author

kanpov commented Jul 8, 2024

My $0.02 is that

  • cni.dev/valid-attachments is a better name anyway
  • We don't have meaningful back compat concerns at this point for this feature

So tweaking the spec and throwing the back compat concern into libcni like #1103 does puts us into a better state.

Sure, but there's no way this change doesn't require a version bump (that is, if CNI follows semver) to 1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants