From f3f0405c9e47621fb850f33c64019bb553beffce Mon Sep 17 00:00:00 2001 From: Raphael Glon Date: Fri, 12 Dec 2025 16:41:01 +0100 Subject: [PATCH] feat: add sth close to the no-cgroups legacy feature in CDI mode [WIP/Draft] Related: https://github.com/NVIDIA/nvidia-container-toolkit/issues/1542 Use case: same as the no-cgroups flags in legacy mode (= with the hook + nvidia-container-cli) -> create dev nodes, setup environment, mount shared libraries but do not give access to devices yet Signed-off-by: Raphael Glon --- internal/config/runtime.go | 2 ++ internal/modifier/cdi.go | 2 ++ internal/modifier/cdi/builder.go | 20 +++++++++++++++----- internal/modifier/cdi/spec.go | 21 +++++++++++++++++++-- internal/modifier/csv.go | 1 + 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/internal/config/runtime.go b/internal/config/runtime.go index 5df04e90f..2ad5e8ef5 100644 --- a/internal/config/runtime.go +++ b/internal/config/runtime.go @@ -27,6 +27,8 @@ type RuntimeConfig struct { Runtimes []string `toml:"runtimes"` Mode string `toml:"mode"` Modes modesConfig `toml:"modes"` + // Close to the "no-cgroups" bool when using the legacy mode with the hooks. + MknodOnly bool `toml:"mknod-only"` } // modesConfig defines (optional) per-mode configs diff --git a/internal/modifier/cdi.go b/internal/modifier/cdi.go index e2de6a4fb..412a37cc3 100644 --- a/internal/modifier/cdi.go +++ b/internal/modifier/cdi.go @@ -74,6 +74,7 @@ func NewCDIModifier(logger logger.Interface, cfg *config.Config, image image.CUD cdi.WithLogger(logger), cdi.WithDevices(devices...), cdi.WithSpecDirs(cfg.NVIDIAContainerRuntimeConfig.Modes.CDI.SpecDirs...), + cdi.WithMknodOnly(cfg.NVIDIAContainerRuntimeConfig.MknodOnly), ) } @@ -197,6 +198,7 @@ func newAutomaticCDISpecModifier(logger logger.Interface, cfg *config.Config, de cdiDeviceRequestor, err := cdi.New( cdi.WithLogger(logger), cdi.WithSpec(spec.Raw()), + cdi.WithMknodOnly(cfg.NVIDIAContainerRuntimeConfig.MknodOnly), ) if err != nil { return nil, fmt.Errorf("failed to construct CDI modifier for mode %q: %w", mode, err) diff --git a/internal/modifier/cdi/builder.go b/internal/modifier/cdi/builder.go index 9d49488d7..c0d366e0c 100644 --- a/internal/modifier/cdi/builder.go +++ b/internal/modifier/cdi/builder.go @@ -27,10 +27,11 @@ import ( ) type builder struct { - logger logger.Interface - specDirs []string - devices []string - cdiSpec *specs.Spec + logger logger.Interface + specDirs []string + devices []string + cdiSpec *specs.Spec + mknodOnly bool } // Option represents a functional option for creating a CDI mofifier. @@ -56,7 +57,8 @@ func (m builder) build() (oci.SpecModifier, error) { if m.cdiSpec != nil { modifier := fromCDISpec{ - cdiSpec: &cdi.Spec{Spec: m.cdiSpec}, + cdiSpec: &cdi.Spec{Spec: m.cdiSpec}, + mknodOnly: m.mknodOnly, } return modifier, nil } @@ -69,6 +71,7 @@ func (m builder) build() (oci.SpecModifier, error) { return nil, fmt.Errorf("failed to create CDI registry: %v", err) } + // FIXME: cannot edit, vendor lib, mknodOnly has no effect in this case modifier := fromRegistry{ logger: m.logger, registry: registry, @@ -105,3 +108,10 @@ func WithSpec(spec *specs.Spec) Option { b.cdiSpec = spec } } + +// WithSpec sets the mknodOnly for the CDI modifier builder. +func WithMknodOnly(mknodOnly bool) Option { + return func(b *builder) { + b.mknodOnly = mknodOnly + } +} diff --git a/internal/modifier/cdi/spec.go b/internal/modifier/cdi/spec.go index 24b475ee0..3d564913b 100644 --- a/internal/modifier/cdi/spec.go +++ b/internal/modifier/cdi/spec.go @@ -27,15 +27,32 @@ import ( // fromCDISpec represents the modifications performed from a raw CDI spec. type fromCDISpec struct { - cdiSpec *cdi.Spec + cdiSpec *cdi.Spec + mknodOnly bool } var _ oci.SpecModifier = (*fromCDISpec)(nil) -// Modify applies the mofiications defined by the raw CDI spec to the incomming OCI spec. +// Modify applies the mofications defined by the raw CDI spec to the incomming OCI spec. func (m fromCDISpec) Modify(spec *specs.Spec) error { for _, device := range m.cdiSpec.Devices { device := device + if m.mknodOnly { + for i := range device.ContainerEdits.DeviceNodes { + // We cannot set an empty string as this will be translated to rwm in the OCI spec generation + // see here: + // https://github.com/NVIDIA/nvidia-container-toolkit/blob/786aa3baf25bf0acd26ae48b5934fa5d503fa1ec/vendor/tags.cncf.io/container-device-interface/pkg/cdi/container-edits.go#L110 + // Since we use the CDI implem above and we can actually pass any arbitrary string, for some oci compatible + // runtimes like runc we could set "_", and this would have the intended effect (no permissions) but + // this relies on two successive flaws in the CDI/OCI specs implems + // (non repect of the CDI spec in the implem above + non respect of the OCI spec in the underlying runtime) + // Let's just add the mknod permission as a trade off: + // container users should not go really far if this is all they are allowed to do (besides runc will end up giving the mknod permission anyway + // https://github.com/opencontainers/runc/blob/ef5e8a5505d6fe022daf016e2535adbda0d89c72/libcontainer/specconv/spec_linux.go#L220-L235) + device.ContainerEdits.DeviceNodes[i].Permissions = "m" + } + } + cdiDevice := cdi.Device{ Device: &device, } diff --git a/internal/modifier/csv.go b/internal/modifier/csv.go index c8cf4ead3..76ed3e7b9 100644 --- a/internal/modifier/csv.go +++ b/internal/modifier/csv.go @@ -72,6 +72,7 @@ func NewCSVModifier(logger logger.Interface, cfg *config.Config, container image return cdi.New( cdi.WithLogger(logger), cdi.WithSpec(spec.Raw()), + cdi.WithMknodOnly(cfg.NVIDIAContainerRuntimeConfig.MknodOnly), ) }