search

NVIDIA / k8s-dra-driver

Dynamic Resource Allocation (DRA) for NVIDIA GPUs in Kubernetes
Apache License 2.0
212 stars 38 forks source link

Use k8s-test-infra devel image #86

Closed ArangoGutierrez closed 4 months ago

ArangoGutierrez commented 4 months ago

@elezar PR is now rebased

ArangoGutierrez commented 4 months ago

last week @elezar and I had a quick ad-hock conversation on whether it makes sense to continue to push for a centralized image (the k8s-test-infra) image, or we should roll back the per-repo devel image, just with some rules so it looks similar across all our repos. So far the centralized image has added no value, and perhaps has added complications like the one leading to this PR.

If @elezar / @klueska think we should roll back, I'll have it as a TODO and prepare a per-repo solution.

klueska commented 4 months ago

last week @elezar and I had a quick ad-hock conversation on whether it makes sense to continue to push for a centralized image (the k8s-test-infra) image, or we should roll back the per-repo devel image, just with some rules so it looks similar across all our repos. So far the centralized image has added no value, and perhaps has added complications like the one leading to this PR.

If @elezar / @klueska think we should roll back, I'll have it as a TODO and prepare a per-repo solution.

I think a centralized one is fine as long as it's configurable for each repo.

elezar commented 4 months ago

I think a centralized one is fine as long as it's configurable for each repo.

My point was that our repos represent a disjoint set of components with different requirements and lifecycles. Trying to consolidate tooling in a central location may be effort that doesn't serve a direct purpose at present. Adding per-repo customization removes the primary benefit of this which is image reuse since a local image needs to be built for non-standard requirements. Furthermore, a user may not be aware of the fact that they NEED to build a custom image for local development since this is not explicitly encoded in the makefile.

Looking at the current devel image's Dockerfile the use of moq, golangci-lint, and setting the git config options on the /work directory are the only three things that are (somewhat) universally applicable. The other tooling was added for projects such as the k8s-dra-driver (although it may be used in some other projects). When considering codebases such as go-nvml the tooling becomes more specific and it may not make sense to centrally manage these.

klueska commented 4 months ago

I don't feel strongly either way.

On one hand, I like the per-repo images that we had originally because I could just update them as necessary for the particular project at hand, without worrying that I might "break" some other project (or become out of sync with some centralized image). This image is only meant to be used locally for build / devel purposes, so it makes sense to also define / build it locally instead of pulling it from some centralized location.

On the other hand, I can see how it can quickly become hard to keep the set of local Dockerfiles we have consistent across repos (for the parts of them that can/should be kept consistent).

Does it then make sense to have a "base" image hosted in k8s-test-infra that only contains the base set of tooling we know all devel images should have and then have each repo define a local Dockerfile that uses this as a base?

ArangoGutierrez commented 4 months ago

Rebased and addressed comments