Skip to content

Commit 20e615c

Browse files
committed
feat: add sth close to the no-cgroups legacy feature in CDI mode [WIP/Draft]
Related: #1542 Use case: same as the no-cgroups flags in legacy mode (= with the hook) -> create dev nodes, setup environment, mount shared libraries but do not give access to devices yet Signed-off-by: Raphael Glon <oOraph@users.noreply.github.com>
1 parent 786aa3b commit 20e615c

File tree

5 files changed

+40
-7
lines changed

5 files changed

+40
-7
lines changed

internal/config/runtime.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ type RuntimeConfig struct {
2727
Runtimes []string `toml:"runtimes"`
2828
Mode string `toml:"mode"`
2929
Modes modesConfig `toml:"modes"`
30+
// Close to the "no-cgroups" bool when using the legacy mode with the hooks. Not stricly equivalent since we add
31+
// the mknod permission, due to some constraint in the CDI spec
32+
// https://github.com/cncf-tags/container-device-interface/issues/300
33+
MknodOnly bool `toml:"mknod-only"`
3034
}
3135

3236
// modesConfig defines (optional) per-mode configs

internal/modifier/cdi.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ func NewCDIModifier(logger logger.Interface, cfg *config.Config, image image.CUD
7777
cdi.WithLogger(logger),
7878
cdi.WithDevices(devices...),
7979
cdi.WithSpecDirs(cfg.NVIDIAContainerRuntimeConfig.Modes.CDI.SpecDirs...),
80+
cdi.WithMknodOnly(cfg.NVIDIAContainerRuntimeConfig.MknodOnly),
8081
)
8182
}
8283

@@ -192,6 +193,7 @@ func newAutomaticCDISpecModifier(logger logger.Interface, cfg *config.Config, de
192193
cdiDeviceRequestor, err := cdi.New(
193194
cdi.WithLogger(logger),
194195
cdi.WithSpec(spec.Raw()),
196+
cdi.WithMknodOnly(cfg.NVIDIAContainerRuntimeConfig.MknodOnly),
195197
)
196198
if err != nil {
197199
return nil, fmt.Errorf("failed to construct CDI modifier for mode %q: %w", mode, err)

internal/modifier/cdi/builder.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,11 @@ import (
2727
)
2828

2929
type builder struct {
30-
logger logger.Interface
31-
specDirs []string
32-
devices []string
33-
cdiSpec *specs.Spec
30+
logger logger.Interface
31+
specDirs []string
32+
devices []string
33+
cdiSpec *specs.Spec
34+
mknodOnly bool
3435
}
3536

3637
// Option represents a functional option for creating a CDI mofifier.
@@ -56,7 +57,8 @@ func (m builder) build() (oci.SpecModifier, error) {
5657

5758
if m.cdiSpec != nil {
5859
modifier := fromCDISpec{
59-
cdiSpec: &cdi.Spec{Spec: m.cdiSpec},
60+
cdiSpec: &cdi.Spec{Spec: m.cdiSpec},
61+
mknodOnly: m.mknodOnly,
6062
}
6163
return modifier, nil
6264
}
@@ -69,6 +71,7 @@ func (m builder) build() (oci.SpecModifier, error) {
6971
return nil, fmt.Errorf("failed to create CDI registry: %v", err)
7072
}
7173

74+
// FIXME: cannot edit, vendor lib, mknodOnly has no effect in this case
7275
modifier := fromRegistry{
7376
logger: m.logger,
7477
registry: registry,
@@ -105,3 +108,10 @@ func WithSpec(spec *specs.Spec) Option {
105108
b.cdiSpec = spec
106109
}
107110
}
111+
112+
// WithSpec sets the mknodOnly for the CDI modifier builder.
113+
func WithMknodOnly(mknodOnly bool) Option {
114+
return func(b *builder) {
115+
b.mknodOnly = mknodOnly
116+
}
117+
}

internal/modifier/cdi/spec.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,31 @@ import (
2727

2828
// fromCDISpec represents the modifications performed from a raw CDI spec.
2929
type fromCDISpec struct {
30-
cdiSpec *cdi.Spec
30+
cdiSpec *cdi.Spec
31+
mknodOnly bool
3132
}
3233

3334
var _ oci.SpecModifier = (*fromCDISpec)(nil)
3435

35-
// Modify applies the mofiications defined by the raw CDI spec to the incomming OCI spec.
36+
// Modify applies the mofications defined by the raw CDI spec to the incomming OCI spec.
3637
func (m fromCDISpec) Modify(spec *specs.Spec) error {
3738
for _, device := range m.cdiSpec.Devices {
3839
device := device
40+
if m.mknodOnly {
41+
for i := range device.ContainerEdits.DeviceNodes {
42+
// We cannot set an empty string as this will be translated to rwm in the OCI spec generation
43+
// see here:
44+
// https://github.com/NVIDIA/nvidia-container-toolkit/blob/786aa3baf25bf0acd26ae48b5934fa5d503fa1ec/vendor/tags.cncf.io/container-device-interface/pkg/cdi/container-edits.go#L110
45+
// Since we use the CDI implem above and we can actually pass any arbitrary string, for some oci compatible
46+
// runtimes like runc we could set "_", and this would have the intended effect (no permissions) but
47+
// this relies on two successive flaws in the CDI/OCI specs implems
48+
// (non repect of the CDI spec in the implem above + non respect of the OCI spec in the underlying runtime)
49+
// Let's just add the mknod right as a trade off:
50+
// container users should not go really far it this is all they are allowed to do
51+
device.ContainerEdits.DeviceNodes[i].Permissions = "m"
52+
}
53+
}
54+
3955
cdiDevice := cdi.Device{
4056
Device: &device,
4157
}

internal/modifier/csv.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ func NewCSVModifier(logger logger.Interface, cfg *config.Config, container image
7272
return cdi.New(
7373
cdi.WithLogger(logger),
7474
cdi.WithSpec(spec.Raw()),
75+
cdi.WithMknodOnly(cfg.NVIDIAContainerRuntimeConfig.MknodOnly),
7576
)
7677
}
7778

0 commit comments

Comments
 (0)