From ec4085bfdfbb3e4471b3a02019c14d5a4918bf8a Mon Sep 17 00:00:00 2001 From: Rajat Chopra Date: Tue, 14 Jul 2026 08:49:18 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20Disable=20chmod=20hook.=20Completely=20f?= =?UTF-8?q?rom=20the=20code.=20Details:=20=20=20-=20Deleted=20the=20nvidia?= =?UTF-8?q?-cdi-hook=20chmod=20implementation.=20=20=20-=20Deleted=20the?= =?UTF-8?q?=20device-folder-permissions=20workaround=20that=20emitted=20ch?= =?UTF-8?q?mod=20hooks.=20=20=20-=20Kept=20the=20unsupported-command=20beh?= =?UTF-8?q?avior,=20so=20legacy=20CDI=20specs=20invoking=20chmod=20still?= =?UTF-8?q?=20warn=20and=20don=E2=80=99t=20block=20startup.=20=20=20-=20Su?= =?UTF-8?q?itably=20modified=20the=20unit=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rajat Chopra --- cmd/nvidia-cdi-hook/README.md | 1 - cmd/nvidia-cdi-hook/chmod/chmod.go | 160 ------------------ cmd/nvidia-cdi-hook/commands/commands.go | 2 - cmd/nvidia-ctk/cdi/generate/generate_test.go | 85 ---------- internal/discover/hooks.go | 25 +-- internal/discover/hooks_test.go | 99 ++++------- pkg/nvcdi/full-gpu-nvml.go | 5 - pkg/nvcdi/management.go | 10 +- .../workarounds-device-folder-permissions.go | 110 ------------ 9 files changed, 33 insertions(+), 464 deletions(-) delete mode 100644 cmd/nvidia-cdi-hook/chmod/chmod.go delete mode 100644 pkg/nvcdi/workarounds-device-folder-permissions.go diff --git a/cmd/nvidia-cdi-hook/README.md b/cmd/nvidia-cdi-hook/README.md index 6352f530d..cb2fc3907 100644 --- a/cmd/nvidia-cdi-hook/README.md +++ b/cmd/nvidia-cdi-hook/README.md @@ -26,7 +26,6 @@ on generating a CDI file. The `nvidia-cdi-hook` CLI provides the following functionality: -* `chmod` - Change the permissions of a file or directory inside the directory path to be mounted into a container. * `create-symlinks` - Create symlinks inside the directory path to be mounted into a container. * `update-ldcache` - Update the dynamic linker cache inside the directory path to be mounted into a container. * `enable-cuda-compat` - Ensure that the directory containing the CUDA compat libraries is added to the ldconfig search path if required. diff --git a/cmd/nvidia-cdi-hook/chmod/chmod.go b/cmd/nvidia-cdi-hook/chmod/chmod.go deleted file mode 100644 index 9b06fb129..000000000 --- a/cmd/nvidia-cdi-hook/chmod/chmod.go +++ /dev/null @@ -1,160 +0,0 @@ -/** -# Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -**/ - -package chmod - -import ( - "context" - "errors" - "fmt" - "io/fs" - "os" - "path/filepath" - "strconv" - "strings" - - "github.com/urfave/cli/v3" - - "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" - "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" -) - -type command struct { - logger logger.Interface -} - -type config struct { - paths []string - modeStr string - mode fs.FileMode - containerSpec string -} - -// NewCommand constructs a chmod command with the specified logger -func NewCommand(logger logger.Interface) *cli.Command { - c := command{ - logger: logger, - } - return c.build() -} - -// build the chmod command -func (m command) build() *cli.Command { - cfg := config{} - - // Create the 'chmod' command - c := cli.Command{ - Name: "chmod", - Usage: "Set the permissions of folders in the container by running chmod. The container root is prefixed to the specified paths.", - Before: func(ctx context.Context, cmd *cli.Command) (context.Context, error) { - return ctx, m.validateFlags(cmd, &cfg) - }, - Action: func(ctx context.Context, cmd *cli.Command) error { - return m.run(cmd, &cfg) - }, - Flags: []cli.Flag{ - &cli.StringSliceFlag{ - Name: "path", - Usage: "Specify a path to apply the specified mode to", - Destination: &cfg.paths, - }, - &cli.StringFlag{ - Name: "mode", - Usage: "Specify the file mode", - Destination: &cfg.modeStr, - }, - &cli.StringFlag{ - Name: "container-spec", - Usage: "Specify the path to the OCI container spec. If empty or '-' the spec will be read from STDIN", - Destination: &cfg.containerSpec, - }, - }, - } - - return &c -} - -func (m command) validateFlags(_ *cli.Command, cfg *config) error { - if strings.TrimSpace(cfg.modeStr) == "" { - return fmt.Errorf("a non-empty mode must be specified") - } - - modeInt, err := strconv.ParseUint(cfg.modeStr, 8, 32) - if err != nil { - return fmt.Errorf("failed to parse mode as octal: %v", err) - } - cfg.mode = fs.FileMode(modeInt) - - for _, p := range cfg.paths { - if strings.TrimSpace(p) == "" { - return fmt.Errorf("paths must not be empty") - } - } - - return nil -} - -func (m command) run(_ *cli.Command, cfg *config) error { - s, err := oci.LoadContainerState(cfg.containerSpec) - if err != nil { - return fmt.Errorf("failed to load container state: %v", err) - } - - containerRoot, err := s.GetContainerRoot() - if err != nil { - return fmt.Errorf("failed to determined container root: %v", err) - } - if containerRoot == "" { - return fmt.Errorf("empty container root detected") - } - - paths := m.getPaths(containerRoot, cfg.paths, cfg.mode) - if len(paths) == 0 { - m.logger.Debugf("No paths specified; exiting") - return nil - } - - for _, path := range paths { - err = os.Chmod(path, cfg.mode) - // in some cases this is not an issue (e.g. whole /dev mounted), see #143 - if errors.Is(err, fs.ErrPermission) { - m.logger.Debugf("Ignoring permission error with chmod: %v", err) - err = nil - } - } - - return err -} - -// getPaths updates the specified paths relative to the root. -func (m command) getPaths(root string, paths []string, desiredMode fs.FileMode) []string { - var pathsInRoot []string - for _, f := range paths { - path := filepath.Join(root, f) - stat, err := os.Stat(path) - if err != nil { - m.logger.Debugf("Skipping path %q: %v", path, err) - continue - } - if (stat.Mode()&(fs.ModePerm|fs.ModeSetuid|fs.ModeSetgid|fs.ModeSticky))^desiredMode == 0 { - m.logger.Debugf("Skipping path %q: already desired mode", path) - continue - } - pathsInRoot = append(pathsInRoot, path) - } - - return pathsInRoot -} diff --git a/cmd/nvidia-cdi-hook/commands/commands.go b/cmd/nvidia-cdi-hook/commands/commands.go index 85f734204..27c25af8a 100644 --- a/cmd/nvidia-cdi-hook/commands/commands.go +++ b/cmd/nvidia-cdi-hook/commands/commands.go @@ -23,7 +23,6 @@ import ( "github.com/urfave/cli/v3" cudamemorylimits "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/apply-cuda-memory-limits" - "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/chmod" symlinks "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/create-symlinks" "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/cudacompat" disabledevicenodemodification "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/disable-device-node-modification" @@ -89,7 +88,6 @@ func ConfigureCDIHookCommand(logger logger.Interface, base *cli.Command) *cli.Co base.Commands = []*cli.Command{ ldcache.NewCommand(logger), symlinks.NewCommand(logger), - chmod.NewCommand(logger), cudacompat.NewCommand(logger), disabledevicenodemodification.NewCommand(logger), cudamemorylimits.NewCommand(logger), diff --git a/cmd/nvidia-ctk/cdi/generate/generate_test.go b/cmd/nvidia-ctk/cdi/generate/generate_test.go index 6cac036c2..56e67faab 100644 --- a/cmd/nvidia-ctk/cdi/generate/generate_test.go +++ b/cmd/nvidia-ctk/cdi/generate/generate_test.go @@ -478,91 +478,6 @@ containerEdits: - nodev - rbind - rprivate -`, - }, - { - description: "enableChmodHook", - options: options{ - format: "yaml", - mode: "management", - vendor: "example.com", - class: "device", - driverRoot: driverRoot, - enabledHooks: []string{"chmod"}, - disabledHooks: []string{"enable-cuda-compat", "update-ldcache", "disable-device-node-modification"}, - }, - expectedOptions: options{ - format: "yaml", - mode: "management", - vendor: "example.com", - class: "device", - nvidiaCDIHookPath: "/usr/bin/nvidia-cdi-hook", - driverRoot: driverRoot, - enabledHooks: []string{"chmod"}, - disabledHooks: []string{"enable-cuda-compat", "update-ldcache", "disable-device-node-modification"}, - }, - expectedSpec: `--- -cdiVersion: 0.5.0 -kind: example.com/device -devices: - - name: all - containerEdits: - deviceNodes: - - path: /dev/nvidia0 - hostPath: {{ .driverRoot }}/dev/nvidia0 - - path: /dev/nvidiactl - hostPath: {{ .driverRoot }}/dev/nvidiactl - - path: /dev/nvidia-caps-imex-channels/channel0 - hostPath: {{ .driverRoot }}/dev/nvidia-caps-imex-channels/channel0 - - path: /dev/nvidia-caps-imex-channels/channel1 - hostPath: {{ .driverRoot }}/dev/nvidia-caps-imex-channels/channel1 - - path: /dev/nvidia-caps-imex-channels/channel2047 - hostPath: {{ .driverRoot }}/dev/nvidia-caps-imex-channels/channel2047 - - path: /dev/nvidia-caps/nvidia-cap1 - hostPath: {{ .driverRoot }}/dev/nvidia-caps/nvidia-cap1 - hooks: - - hookName: createContainer - path: /usr/bin/nvidia-cdi-hook - args: - - nvidia-cdi-hook - - chmod - - --mode - - "755" - - --path - - /dev/nvidia-caps - env: - - NVIDIA_CTK_DEBUG=false -containerEdits: - env: - - NVIDIA_CTK_LIBCUDA_DIR=/lib/x86_64-linux-gnu - - NVIDIA_VISIBLE_DEVICES=void - hooks: - - hookName: createContainer - path: /usr/bin/nvidia-cdi-hook - args: - - nvidia-cdi-hook - - create-symlinks - - --link - - libcuda.so.1::/lib/x86_64-linux-gnu/libcuda.so - env: - - NVIDIA_CTK_DEBUG=false - mounts: - - hostPath: {{ .driverRoot }}/lib/x86_64-linux-gnu/libcuda.so.999.88.77 - containerPath: /lib/x86_64-linux-gnu/libcuda.so.999.88.77 - options: - - ro - - nosuid - - nodev - - rbind - - rprivate - - hostPath: {{ .driverRoot }}/lib/x86_64-linux-gnu/vdpau/libvdpau_nvidia.so.999.88.77 - containerPath: /lib/x86_64-linux-gnu/vdpau/libvdpau_nvidia.so.999.88.77 - options: - - ro - - nosuid - - nodev - - rbind - - rprivate `, }, } diff --git a/internal/discover/hooks.go b/internal/discover/hooks.go index 173291f7f..ec0a2e32b 100644 --- a/internal/discover/hooks.go +++ b/internal/discover/hooks.go @@ -46,10 +46,6 @@ const ( // profiles". It currently restricts EGL/Vulkan GPU visibility inside the // container to the GPUs actually mounted. ApplicationProfileHook = HookName("update-application-profile") - // A ChmodHook is used to set the file mode of the specified paths. - // - // Deprecated: The chmod hook is deprecated and will be removed in a future release. - ChmodHook = HookName("chmod") // A CreateSymlinksHook is used to create symlinks in the container. CreateSymlinksHook = HookName("create-symlinks") // DisableDeviceNodeModificationHook refers to the hook used to ensure that @@ -69,14 +65,6 @@ const ( defaultNvidiaCDIHookPath = "/usr/bin/nvidia-cdi-hook" ) -// defaultDisabledHooks defines hooks that are disabled by default. -// These hooks can be explicitly enabled using the WithEnabledHooks option. -var defaultDisabledHooks = []HookName{ - // ChmodHook is disabled by default as it was a workaround for older - // versions of crun that has since been fixed. - ChmodHook, -} - var _ Discover = (*Hook)(nil) // Devices returns an empty list of devices for a Hook discoverer. @@ -151,7 +139,7 @@ func WithDisabledHooks(hooks ...HookName) Option { } // WithEnabledHooks explicitly enables the specified hooks. -// This is useful for enabling hooks that are disabled by default. +// This is useful for overriding hooks passed to WithDisabledHooks. func WithEnabledHooks(hooks ...HookName) Option { return func(c *hookCreatorOptions) { c.enabledHooks = append(c.enabledHooks, hooks...) @@ -179,8 +167,6 @@ func NewHookCreator(opts ...Option) HookCreator { opt(o) } - o.disabledHooks = append(o.disabledHooks, defaultDisabledHooks...) - disabledHooks := make(map[HookName]bool) for _, h := range o.disabledHooks { disabledHooks[h] = true @@ -222,7 +208,7 @@ func (c cdiHookCreator) Create(name HookName, args ...string) *Hook { func (c cdiHookCreator) getOCIHookType(name HookName) OCIHookType { switch name { - case CreateSymlinksHook, ChmodHook, DisableDeviceNodeModificationHook, EnableCudaCompatHook, UpdateLDCacheHook, ApplicationProfileHook: + case CreateSymlinksHook, DisableDeviceNodeModificationHook, EnableCudaCompatHook, UpdateLDCacheHook, ApplicationProfileHook: return OCIHookTypeCreateContainer case ApplyCudaMemoryLimitsHook: return OCIHookTypeCreateRuntime @@ -242,7 +228,7 @@ func (c cdiHookCreator) isDisabled(name HookName, args ...string) bool { // still reject hooks that require args if none were provided switch name { - case CreateSymlinksHook, ChmodHook, ApplyCudaMemoryLimitsHook: + case CreateSymlinksHook, ApplyCudaMemoryLimitsHook: return len(args) == 0 } return false @@ -259,11 +245,6 @@ func (c cdiHookCreator) transformArgs(name HookName, args ...string) []string { for _, arg := range args { transformedArgs = append(transformedArgs, "--link", arg) } - case ChmodHook: - transformedArgs = append(transformedArgs, "--mode", "755") - for _, arg := range args { - transformedArgs = append(transformedArgs, "--path", arg) - } case UpdateLDCacheHook: if c.ldconfigPath != "" { transformedArgs = append(transformedArgs, "--ldconfig-path", c.ldconfigPath) diff --git a/internal/discover/hooks_test.go b/internal/discover/hooks_test.go index 1bac3190a..f53e1deb9 100644 --- a/internal/discover/hooks_test.go +++ b/internal/discover/hooks_test.go @@ -34,9 +34,7 @@ func TestNewHookCreator(t *testing.T) { expected: &cdiHookCreator{ nvidiaCDIHookPath: defaultNvidiaCDIHookPath, fixedArgs: []string{"nvidia-cdi-hook"}, - disabledHooks: map[HookName]bool{ - ChmodHook: true, // ChmodHook is disabled by default - }, + disabledHooks: map[HookName]bool{}, }, }, { @@ -47,9 +45,7 @@ func TestNewHookCreator(t *testing.T) { expected: &cdiHookCreator{ nvidiaCDIHookPath: "/custom/path/nvidia-cdi-hook", fixedArgs: []string{"nvidia-cdi-hook"}, - disabledHooks: map[HookName]bool{ - ChmodHook: true, - }, + disabledHooks: map[HookName]bool{}, }, }, { @@ -71,7 +67,6 @@ func TestNewHookCreator(t *testing.T) { disabledHooks: map[HookName]bool{ AllHooks: true, UpdateLDCacheHook: false, - ChmodHook: true, }, }, }, @@ -79,7 +74,7 @@ func TestNewHookCreator(t *testing.T) { name: "multiple hooks disabled and enabled", opts: []Option{ WithDisabledHooks(UpdateLDCacheHook, CreateSymlinksHook, EnableCudaCompatHook, DisableDeviceNodeModificationHook), - WithEnabledHooks(ChmodHook, UpdateLDCacheHook), + WithEnabledHooks(UpdateLDCacheHook), }, expected: &cdiHookCreator{ nvidiaCDIHookPath: defaultNvidiaCDIHookPath, @@ -88,7 +83,6 @@ func TestNewHookCreator(t *testing.T) { UpdateLDCacheHook: false, CreateSymlinksHook: true, EnableCudaCompatHook: true, - ChmodHook: false, DisableDeviceNodeModificationHook: true, }, }, @@ -107,20 +101,20 @@ func TestNewHookCreator(t *testing.T) { UpdateLDCacheHook: true, CreateSymlinksHook: true, EnableCudaCompatHook: true, - ChmodHook: true, // Default disabled }, }, }, { - name: "WithEnabledHooks overrides defaultDisabledHooks", + name: "WithEnabledHooks overrides disabled hooks", opts: []Option{ - WithEnabledHooks(ChmodHook), + WithDisabledHooks(UpdateLDCacheHook), + WithEnabledHooks(UpdateLDCacheHook), }, expected: &cdiHookCreator{ nvidiaCDIHookPath: defaultNvidiaCDIHookPath, fixedArgs: []string{"nvidia-cdi-hook"}, disabledHooks: map[HookName]bool{ - ChmodHook: false, // ChmodHook is enabled + UpdateLDCacheHook: false, }, }, }, @@ -132,9 +126,7 @@ func TestNewHookCreator(t *testing.T) { expected: &cdiHookCreator{ nvidiaCDIHookPath: "/usr/bin/nvidia-ctk", fixedArgs: []string{"nvidia-ctk", "hook"}, - disabledHooks: map[HookName]bool{ - ChmodHook: true, - }, + disabledHooks: map[HookName]bool{}, }, }, { @@ -145,9 +137,7 @@ func TestNewHookCreator(t *testing.T) { expected: &cdiHookCreator{ nvidiaCDIHookPath: "/usr/local/nvidia/toolkit/nvidia-ctk", fixedArgs: []string{"nvidia-ctk", "hook"}, - disabledHooks: map[HookName]bool{ - ChmodHook: true, - }, + disabledHooks: map[HookName]bool{}, }, }, } @@ -188,26 +178,18 @@ func TestCDIHookCreator_Create(t *testing.T) { expectedHook: nil, }, { - name: "ChmodHook with args (when enabled)", - hookCreator: NewHookCreator( - WithNVIDIACDIHookPath(defaultNvidiaCDIHookPath), - WithEnabledHooks(ChmodHook), - ), - hookName: ChmodHook, - args: []string{"/path/to/file1", "/path/to/file2"}, - expectedHook: &Hook{ - Lifecycle: "createContainer", - Path: defaultNvidiaCDIHookPath, - Args: []string{"nvidia-cdi-hook", "chmod", "--mode", "755", "--path", "/path/to/file1", "--path", "/path/to/file2"}, - Env: []string{"NVIDIA_CTK_DEBUG=false"}, - }, + name: "CreateSymlinksHook disabled returns nil", + hookCreator: NewHookCreator(WithDisabledHooks(CreateSymlinksHook)), + hookName: CreateSymlinksHook, + args: []string{"/source::/target"}, + expectedHook: nil, }, { - name: "ChmodHook disabled by default returns nil", - hookCreator: NewHookCreator(WithNVIDIACDIHookPath(defaultNvidiaCDIHookPath)), - hookName: ChmodHook, - args: []string{"/path/to/file"}, - expectedHook: nil, // ChmodHook is disabled by default + name: "CreateSymlinksHook disabled with multiple args returns nil", + hookCreator: NewHookCreator(WithDisabledHooks(CreateSymlinksHook)), + hookName: CreateSymlinksHook, + args: []string{"/source::/target", "/source2::/target2"}, + expectedHook: nil, }, { name: "UpdateLDCacheHook with no args", @@ -351,22 +333,22 @@ func TestCDIHookCreator_isDisabled(t *testing.T) { }{ { name: "hook explicitly disabled", - disabledHooks: []HookName{ChmodHook}, - hookName: ChmodHook, - args: []string{"/path/to/file"}, - expectedResult: true, // ChmodHook is disabled by default and explicitly disabled + disabledHooks: []HookName{UpdateLDCacheHook}, + hookName: UpdateLDCacheHook, + args: []string{}, + expectedResult: true, }, { name: "hook explicitly enabled overrides disabled", - disabledHooks: []HookName{ChmodHook}, - enabledHooks: []HookName{ChmodHook}, - hookName: ChmodHook, - args: []string{"/path/to/file"}, + disabledHooks: []HookName{UpdateLDCacheHook}, + enabledHooks: []HookName{UpdateLDCacheHook}, + hookName: UpdateLDCacheHook, + args: []string{}, expectedResult: false, }, { name: "hook not in disabled map and not AllHooks disabled", - disabledHooks: []HookName{ChmodHook}, + disabledHooks: []HookName{}, hookName: UpdateLDCacheHook, args: []string{}, expectedResult: false, @@ -385,21 +367,6 @@ func TestCDIHookCreator_isDisabled(t *testing.T) { args: []string{"/path/to/symlink"}, expectedResult: false, }, - { - name: "ChmodHook requires args - no args provided", - disabledHooks: []HookName{}, - hookName: ChmodHook, - args: []string{}, - expectedResult: true, - }, - { - name: "ChmodHook requires args - args provided", - disabledHooks: []HookName{}, - enabledHooks: []HookName{ChmodHook}, // Enable ChmodHook since it's disabled by default - hookName: ChmodHook, - args: []string{"/path/to/file"}, - expectedResult: false, - }, { name: "UpdateLDCacheHook doesn't require args - no args provided", disabledHooks: []HookName{}, @@ -438,7 +405,7 @@ func TestCDIHookCreator_isDisabled(t *testing.T) { }, { name: "unknown hook name", - disabledHooks: []HookName{ChmodHook}, + disabledHooks: []HookName{}, hookName: HookName("unknown-hook"), args: []string{}, expectedResult: false, @@ -451,14 +418,6 @@ func TestCDIHookCreator_isDisabled(t *testing.T) { args: []string{"/path1", "/path2", "/path3"}, expectedResult: false, }, - { - name: "ChmodHook with multiple args", - disabledHooks: []HookName{}, - enabledHooks: []HookName{ChmodHook}, // Enable ChmodHook since it's disabled by default - hookName: ChmodHook, - args: []string{"/path1", "/path2"}, - expectedResult: false, - }, { name: "UpdateLDCacheHook with multiple args", disabledHooks: []HookName{}, diff --git a/pkg/nvcdi/full-gpu-nvml.go b/pkg/nvcdi/full-gpu-nvml.go index 554d8cbdb..ccb77722a 100644 --- a/pkg/nvcdi/full-gpu-nvml.go +++ b/pkg/nvcdi/full-gpu-nvml.go @@ -169,10 +169,6 @@ func (l *fullGPUDeviceSpecGenerator) newFullGPUDiscoverer(d device.Device) (disc return nil, fmt.Errorf("failed to create device discoverer: %v", err) } - deviceFolderPermissionHooks := (*nvcdilib)(l.nvmllib).newDeviceFolderPermissionHookDiscoverer( - deviceNodes, - ) - cudaMemoryLimitsHook, err := (*nvcdilib)(l.nvmllib).newCudaMemoryLimits(d) if err != nil { return nil, fmt.Errorf("failed to create cuda memory limits discoverer: %w", err) @@ -182,7 +178,6 @@ func (l *fullGPUDeviceSpecGenerator) newFullGPUDiscoverer(d device.Device) (disc discoverers = append(discoverers, deviceNodes, - deviceFolderPermissionHooks, cudaMemoryLimitsHook, ) diff --git a/pkg/nvcdi/management.go b/pkg/nvcdi/management.go index 772c754b4..33ff48572 100644 --- a/pkg/nvcdi/management.go +++ b/pkg/nvcdi/management.go @@ -108,15 +108,7 @@ func (l *managementlib) newManagementDeviceDiscoverer() (discover.Discover, erro }, ) - deviceFolderPermissionHooks := (*nvcdilib)(l).newDeviceFolderPermissionHookDiscoverer( - deviceNodes, - ) - - d := discover.Merge( - &managementDiscoverer{deviceNodes}, - deviceFolderPermissionHooks, - ) - return d, nil + return &managementDiscoverer{deviceNodes}, nil } func (m *managementDiscoverer) Devices() ([]discover.Device, error) { diff --git a/pkg/nvcdi/workarounds-device-folder-permissions.go b/pkg/nvcdi/workarounds-device-folder-permissions.go deleted file mode 100644 index fc7ff578f..000000000 --- a/pkg/nvcdi/workarounds-device-folder-permissions.go +++ /dev/null @@ -1,110 +0,0 @@ -/** -# Copyright (c) NVIDIA CORPORATION. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -**/ - -package nvcdi - -import ( - "fmt" - "path/filepath" - - "github.com/NVIDIA/nvidia-container-toolkit/internal/discover" - "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" -) - -type deviceFolderPermissions struct { - logger logger.Interface - devRoot string - devices discover.Discover - hookCreator discover.HookCreator -} - -var _ discover.Discover = (*deviceFolderPermissions)(nil) - -// newDeviceFolderPermissionHookDiscoverer creates a discoverer that can be used to update the permissions for the parent folders of nested device nodes from the specified set of device specs. -// This works around an issue with rootless podman when using crun as a low-level runtime. -// See https://github.com/containers/crun/issues/1047 -// The nested devices that are applicable to the NVIDIA GPU devices are: -// - DRM devices at /dev/dri/* -// - NVIDIA Caps devices at /dev/nvidia-caps/* -func (l *nvcdilib) newDeviceFolderPermissionHookDiscoverer(devices discover.Discover) discover.Discover { - d := &deviceFolderPermissions{ - logger: l.logger, - devRoot: l.driver.DevRoot, - hookCreator: l.hookCreator, - devices: devices, - } - - return d -} - -// Devices are empty for this discoverer -func (d *deviceFolderPermissions) Devices() ([]discover.Device, error) { - return nil, nil -} - -// EnvVars are empty for this discoverer -func (d *deviceFolderPermissions) EnvVars() ([]discover.EnvVar, error) { - return nil, nil -} - -// Hooks returns a set of hooks that sets the file mode to 755 of parent folders for nested device nodes. -func (d *deviceFolderPermissions) Hooks() ([]discover.Hook, error) { - folders, err := d.getDeviceSubfolders() - if err != nil { - return nil, fmt.Errorf("failed to get device subfolders: %v", err) - } - - //nolint:staticcheck // The ChmodHook is deprecated and will be removed in a future release. - return d.hookCreator.Create(discover.ChmodHook, folders...).Hooks() -} - -func (d *deviceFolderPermissions) getDeviceSubfolders() ([]string, error) { - // For now we only consider the following special case paths - allowedPaths := map[string]bool{ - "/dev/dri": true, - "/dev/nvidia-caps": true, - } - - devices, err := d.devices.Devices() - if err != nil { - return nil, fmt.Errorf("failed to get devices: %v", err) - } - - var folders []string - seen := make(map[string]bool) - for _, device := range devices { - df := filepath.Dir(device.Path) - if seen[df] { - continue - } - // We only consider the special case paths - if !allowedPaths[df] { - continue - } - folders = append(folders, df) - seen[df] = true - if len(folders) == len(allowedPaths) { - break - } - } - - return folders, nil -} - -// Mounts are empty for this discoverer -func (d *deviceFolderPermissions) Mounts() ([]discover.Mount, error) { - return nil, nil -}