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

reconsider dependencies #97

Closed
pohly opened this issue Nov 10, 2022 · 5 comments · Fixed by #170
Closed

reconsider dependencies #97

pohly opened this issue Nov 10, 2022 · 5 comments · Fixed by #170

Comments

@pohly
Copy link
Contributor

pohly commented Nov 10, 2022

Using this repository is not possible in Kubernetes because even just having problematic dependencies in the dependency graph is flagged as a problem, even if no code from those dependencies is needed in Kubernetes:

The solution I came up with for now consists of copying some code from this repo into Kubernetes:

  • test/e2e/dra/test-driver/app/cdi.go (simplified struct defs)
  • pkg/kubelet/cm/dra/cdi.go (helper functions for working with CDI strings).

Long term it would be good to have that code in a Go module with no problematic dependencies.
This can be done by splitting this repo or with multiple Go modules in the existing one.

@klihub
Copy link
Contributor

klihub commented Nov 17, 2022

@klihub
Copy link
Contributor

klihub commented Nov 17, 2022

Filed this issue for runtime-tools, describing the problem and telling that we're (as in the folks from Intel and Nvidia working on DRA and CDI) ready to do the actual heavy lifting as long as we can agree with the runtime-tools maintainers about what is acceptable lifting.

@klihub
Copy link
Contributor

klihub commented Nov 17, 2022

/cc @elezar @bart0sh

bart0sh added a commit to bart0sh/container-device-interface that referenced this issue Oct 26, 2023
This should allow to import CDI spec structures without adding
a lot of CDI runtime deps.

It should partly fix cncf-tags#97 and allow Kubernetes to refer this module directly.
There are at least 2 places in the k/k codebase that would benefit
from this change:
- this can be replaced by the specs-go import:
  https://github.com/kubernetes/kubernetes/blob/master/test/e2e/dra/test-driver/app/cdi.go
- this can import specs-go and use JSON marshalling instead of
  quite ugly approach it uses to write CDI files:
  https://github.com/kubernetes/kubernetes/blob/master/test/images/sample-device-plugin/sampledeviceplugin.go#L237
bart0sh added a commit to bart0sh/container-device-interface that referenced this issue Oct 26, 2023
This should allow to import CDI spec structures without adding
a lot of CDI runtime deps.

It should partly fix cncf-tags#97 and allow Kubernetes to refer this module directly.
There are at least 2 places in the k/k codebase that would benefit
from this change:
- this can be replaced by the specs-go import:
  https://github.com/kubernetes/kubernetes/blob/master/test/e2e/dra/test-driver/app/cdi.go
- this can import specs-go and use JSON marshalling instead of
  quite ugly approach it uses to write CDI files:
  https://github.com/kubernetes/kubernetes/blob/master/test/images/sample-device-plugin/sampledeviceplugin.go#L237

Signed-off-by: Ed Bartosh <[email protected]>
bart0sh added a commit to bart0sh/container-device-interface that referenced this issue Oct 26, 2023
This should allow to import CDI spec structures without adding
a lot of CDI runtime deps.

It should partly fix cncf-tags#97 and allow Kubernetes to refer this module directly.
There are at least 2 places in the k/k codebase that would benefit
from this change:
- this can be replaced by the specs-go import:
  https://github.com/kubernetes/kubernetes/blob/master/test/e2e/dra/test-driver/app/cdi.go
- this can import specs-go and use JSON marshalling instead of
  quite ugly approach it uses to write CDI files:
  https://github.com/kubernetes/kubernetes/blob/master/test/images/sample-device-plugin/sampledeviceplugin.go#L237

Signed-off-by: Ed Bartosh <[email protected]>
bart0sh added a commit to bart0sh/container-device-interface that referenced this issue Oct 27, 2023
This should allow to import CDI spec structures without adding
a lot of CDI runtime deps.

It should partly fix cncf-tags#97 and allow Kubernetes to refer this module directly.
There are at least 2 places in the k/k codebase that would benefit
from this change:
- this can be replaced by the specs-go import:
  https://github.com/kubernetes/kubernetes/blob/master/test/e2e/dra/test-driver/app/cdi.go
- this can import specs-go and use JSON marshalling instead of
  quite ugly approach it uses to write CDI files:
  https://github.com/kubernetes/kubernetes/blob/master/test/images/sample-device-plugin/sampledeviceplugin.go#L237

Signed-off-by: Ed Bartosh <[email protected]>
elezar pushed a commit to bart0sh/container-device-interface that referenced this issue Oct 30, 2023
This should allow to import CDI spec structures without adding
a lot of CDI runtime deps.

It should partly fix cncf-tags#97 and allow Kubernetes to refer this module directly.
There are at least 2 places in the k/k codebase that would benefit
from this change:
- this can be replaced by the specs-go import:
  https://github.com/kubernetes/kubernetes/blob/master/test/e2e/dra/test-driver/app/cdi.go
- this can import specs-go and use JSON marshalling instead of
  quite ugly approach it uses to write CDI files:
  https://github.com/kubernetes/kubernetes/blob/master/test/images/sample-device-plugin/sampledeviceplugin.go#L237

Signed-off-by: Ed Bartosh <[email protected]>
@elezar elezar reopened this Oct 30, 2023
Copy link

This issue is stale because it has been open 90 days with no activity. This issue will be closed in 30 days unless new comments are made or the stale label is removed.

Copy link

This issue was automatically closed due to inactivity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants