From 879cc99aac82638e479b9a6ed7e0723358b43566 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 27 Nov 2023 10:52:04 +0100 Subject: [PATCH 1/2] Add transformer for container roots This change renames the root transformer to indicate that it operates on host paths and adds a container root transformer for explicitly transforming container roots. The transform.NewRootTransformer constructor still exists, but has been marked as deprecated. Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/cdi/transform/root/root.go | 2 +- pkg/nvcdi/transform/{ => noop}/no-op.go | 11 +- pkg/nvcdi/transform/root.go | 114 ------ pkg/nvcdi/transform/root/builder.go | 38 ++ pkg/nvcdi/transform/root/container-root.go | 112 ++++++ .../transform/root/container-root_test.go | 342 ++++++++++++++++++ pkg/nvcdi/transform/root/host-root.go | 115 ++++++ .../{root_test.go => root/host-root_test.go} | 108 +++++- pkg/nvcdi/transform/root/options.go | 41 +++ pkg/nvcdi/transform/root/root.go | 47 +++ tools/container/toolkit/toolkit.go | 8 +- 11 files changed, 811 insertions(+), 127 deletions(-) rename pkg/nvcdi/transform/{ => noop}/no-op.go (76%) delete mode 100644 pkg/nvcdi/transform/root.go create mode 100644 pkg/nvcdi/transform/root/builder.go create mode 100644 pkg/nvcdi/transform/root/container-root.go create mode 100644 pkg/nvcdi/transform/root/container-root_test.go create mode 100644 pkg/nvcdi/transform/root/host-root.go rename pkg/nvcdi/transform/{root_test.go => root/host-root_test.go} (64%) create mode 100644 pkg/nvcdi/transform/root/options.go create mode 100644 pkg/nvcdi/transform/root/root.go diff --git a/cmd/nvidia-ctk/cdi/transform/root/root.go b/cmd/nvidia-ctk/cdi/transform/root/root.go index 35c8b63e..8933a815 100644 --- a/cmd/nvidia-ctk/cdi/transform/root/root.go +++ b/cmd/nvidia-ctk/cdi/transform/root/root.go @@ -104,7 +104,7 @@ func (m command) run(c *cli.Context, opts *options) error { return fmt.Errorf("failed to load CDI specification: %w", err) } - err = transform.NewRootTransformer( + err = transform.NewHostRootTransformer( opts.from, opts.to, ).Transform(spec.Raw()) diff --git a/pkg/nvcdi/transform/no-op.go b/pkg/nvcdi/transform/noop/no-op.go similarity index 76% rename from pkg/nvcdi/transform/no-op.go rename to pkg/nvcdi/transform/noop/no-op.go index be2bacf7..3b90b61b 100644 --- a/pkg/nvcdi/transform/no-op.go +++ b/pkg/nvcdi/transform/noop/no-op.go @@ -14,22 +14,23 @@ # limitations under the License. **/ -package transform +package noop import ( + "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" "tags.cncf.io/container-device-interface/specs-go" ) type noop struct{} -var _ Transformer = (*noop)(nil) +var _ transform.Transformer = (*noop)(nil) -// NewNoopTransformer returns a no-op transformer -func NewNoopTransformer() Transformer { +// New returns a no-op transformer. +func New() transform.Transformer { return noop{} } -// Transform is a no-op +// Transform is a no-op for a noop transformer. func (n noop) Transform(spec *specs.Spec) error { return nil } diff --git a/pkg/nvcdi/transform/root.go b/pkg/nvcdi/transform/root.go deleted file mode 100644 index 555b9e51..00000000 --- a/pkg/nvcdi/transform/root.go +++ /dev/null @@ -1,114 +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 transform - -import ( - "fmt" - "path/filepath" - "strings" - - "tags.cncf.io/container-device-interface/specs-go" -) - -type rootTransformer struct { - root string - targetRoot string -} - -var _ Transformer = (*rootTransformer)(nil) - -// NewRootTransformer creates a new transformer for modifying -// the root for paths in a CDI spec. If both roots are identical, -// this tranformer is a no-op. -func NewRootTransformer(root string, targetRoot string) Transformer { - if root == targetRoot { - return NewNoopTransformer() - } - - t := rootTransformer{ - root: root, - targetRoot: targetRoot, - } - return t -} - -// Transform replaces the root in a spec with a new root. -// It walks the spec and replaces all host paths that start with root with the target root. -func (t rootTransformer) Transform(spec *specs.Spec) error { - if spec == nil { - return nil - } - - for _, d := range spec.Devices { - d := d - if err := t.applyToEdits(&d.ContainerEdits); err != nil { - return fmt.Errorf("failed to apply root transform to device %s: %w", d.Name, err) - } - } - - if err := t.applyToEdits(&spec.ContainerEdits); err != nil { - return fmt.Errorf("failed to apply root transform to spec: %w", err) - } - return nil -} - -func (t rootTransformer) applyToEdits(edits *specs.ContainerEdits) error { - for i, dn := range edits.DeviceNodes { - dn.HostPath = t.transformPath(dn.HostPath) - edits.DeviceNodes[i] = dn - } - - for i, hook := range edits.Hooks { - hook.Path = t.transformPath(hook.Path) - - var args []string - for _, arg := range hook.Args { - if !strings.Contains(arg, "::") { - args = append(args, t.transformPath(arg)) - continue - } - - // For the 'create-symlinks' hook, special care is taken for the - // '--link' flag argument which takes the form ::. - // Both paths, the target and link paths, are transformed. - split := strings.Split(arg, "::") - if len(split) != 2 { - return fmt.Errorf("unexpected number of '::' separators in hook argument") - } - split[0] = t.transformPath(split[0]) - split[1] = t.transformPath(split[1]) - args = append(args, strings.Join(split, "::")) - } - hook.Args = args - edits.Hooks[i] = hook - } - - for i, mount := range edits.Mounts { - mount.HostPath = t.transformPath(mount.HostPath) - edits.Mounts[i] = mount - } - - return nil -} - -func (t rootTransformer) transformPath(path string) string { - if !strings.HasPrefix(path, t.root) { - return path - } - - return filepath.Join(t.targetRoot, strings.TrimPrefix(path, t.root)) -} diff --git a/pkg/nvcdi/transform/root/builder.go b/pkg/nvcdi/transform/root/builder.go new file mode 100644 index 00000000..7ac7b7b8 --- /dev/null +++ b/pkg/nvcdi/transform/root/builder.go @@ -0,0 +1,38 @@ +/** +# Copyright 2023 NVIDIA CORPORATION +# +# 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 root + +import ( + "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" + "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform/noop" +) + +type builder struct { + transformer + relativeTo string +} + +func (b *builder) build() transform.Transformer { + if b.root == b.targetRoot { + return noop.New() + } + + if b.relativeTo == "container" { + return containerRootTransformer(b.transformer) + } + return hostRootTransformer(b.transformer) +} diff --git a/pkg/nvcdi/transform/root/container-root.go b/pkg/nvcdi/transform/root/container-root.go new file mode 100644 index 00000000..e57583c2 --- /dev/null +++ b/pkg/nvcdi/transform/root/container-root.go @@ -0,0 +1,112 @@ +/** +# 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 root + +import ( + "fmt" + "strings" + + "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" + "tags.cncf.io/container-device-interface/specs-go" +) + +// containerRootTransformer transforms the roots of container paths in a CDI spec. +type containerRootTransformer transformer + +var _ transform.Transformer = (*containerRootTransformer)(nil) + +// Transform replaces the root in a spec with a new root. +// It walks the spec and replaces all container paths that start with root with the target root. +func (t containerRootTransformer) Transform(spec *specs.Spec) error { + if spec == nil { + return nil + } + + for _, d := range spec.Devices { + d := d + if err := t.applyToEdits(&d.ContainerEdits); err != nil { + return fmt.Errorf("failed to apply root transform to device %s: %w", d.Name, err) + } + } + + if err := t.applyToEdits(&spec.ContainerEdits); err != nil { + return fmt.Errorf("failed to apply root transform to spec: %w", err) + } + return nil +} + +func (t containerRootTransformer) applyToEdits(edits *specs.ContainerEdits) error { + for i, dn := range edits.DeviceNodes { + edits.DeviceNodes[i] = t.transformDeviceNode(dn) + } + + for i, hook := range edits.Hooks { + edits.Hooks[i] = t.transformHook(hook) + } + + for i, mount := range edits.Mounts { + edits.Mounts[i] = t.transformMount(mount) + } + + return nil +} + +func (t containerRootTransformer) transformDeviceNode(dn *specs.DeviceNode) *specs.DeviceNode { + dn.Path = t.transformPath(dn.Path) + + return dn +} + +func (t containerRootTransformer) transformHook(hook *specs.Hook) *specs.Hook { + // The Path in the startContainer hook MUST resolve in the container namespace. + if hook.HookName == "startContainer" { + hook.Path = t.transformPath(hook.Path) + } + + // The createContainer and startContainer hooks MUST execute in the container namespace. + if hook.HookName != "createContainer" && hook.HookName != "startContainer" { + return hook + } + + var args []string + for _, arg := range hook.Args { + if !strings.Contains(arg, "::") { + args = append(args, t.transformPath(arg)) + continue + } + + // For the 'create-symlinks' hook, special care is taken for the + // '--link' flag argument which takes the form ::. + // Both paths, the target and link paths, are transformed. + split := strings.SplitN(arg, "::", 2) + split[0] = t.transformPath(split[0]) + split[1] = t.transformPath(split[1]) + args = append(args, strings.Join(split, "::")) + } + hook.Args = args + + return hook +} + +func (t containerRootTransformer) transformMount(mount *specs.Mount) *specs.Mount { + mount.ContainerPath = t.transformPath(mount.ContainerPath) + return mount +} + +func (t containerRootTransformer) transformPath(path string) string { + return (transformer)(t).transformPath(path) +} diff --git a/pkg/nvcdi/transform/root/container-root_test.go b/pkg/nvcdi/transform/root/container-root_test.go new file mode 100644 index 00000000..fd098cbc --- /dev/null +++ b/pkg/nvcdi/transform/root/container-root_test.go @@ -0,0 +1,342 @@ +/** +# 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 root + +import ( + "testing" + + "github.com/stretchr/testify/require" + "tags.cncf.io/container-device-interface/specs-go" +) + +func TestContainerRootTransformer(t *testing.T) { + testCases := []struct { + description string + root string + targetRoot string + spec *specs.Spec + expectedSpec *specs.Spec + }{ + { + description: "nil spec", + root: "/root", + targetRoot: "/target-root", + spec: nil, + expectedSpec: nil, + }, + { + description: "empty spec", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{}, + expectedSpec: &specs.Spec{}, + }, + { + description: "device nodes", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + {HostPath: "/root/dev/nvidia0", Path: "/root/dev/nvidia0"}, + {HostPath: "/target-root/dev/nvidia1", Path: "/target-root/dev/nvidia1"}, + {HostPath: "/different-root/dev/nvidia2", Path: "/different-root/dev/nvidia2"}, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + {HostPath: "/root/dev/nvidia0", Path: "/target-root/dev/nvidia0"}, + {HostPath: "/target-root/dev/nvidia1", Path: "/target-root/dev/nvidia1"}, + {HostPath: "/different-root/dev/nvidia2", Path: "/different-root/dev/nvidia2"}, + }, + }, + }, + }, + { + description: "mounts", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Mounts: []*specs.Mount{ + {HostPath: "/root/lib/lib1.so", ContainerPath: "/root/lib/lib1.so"}, + {HostPath: "/target-root/lib/lib2.so", ContainerPath: "/target-root/lib/lib2.so"}, + {HostPath: "/different-root/lib/lib3.so", ContainerPath: "/different-root/lib/lib3.so"}, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Mounts: []*specs.Mount{ + {HostPath: "/root/lib/lib1.so", ContainerPath: "/target-root/lib/lib1.so"}, + {HostPath: "/target-root/lib/lib2.so", ContainerPath: "/target-root/lib/lib2.so"}, + {HostPath: "/different-root/lib/lib3.so", ContainerPath: "/different-root/lib/lib3.so"}, + }, + }, + }, + }, + { + description: "hooks", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + { + HookName: "createContainer", + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + { + HookName: "createContainer", + Path: "/different-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/different-root/path/to/target::/different-root/path/to/link", + }, + }, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + { + HookName: "createContainer", + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + { + HookName: "createContainer", + Path: "/different-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/different-root/path/to/target::/different-root/path/to/link", + }, + }, + }, + }, + }, + }, + { + description: "update ldcache hooks", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "nvidia-ctk", "hook", "update-ldcache", + "--folder", + "/root/path/to/target", + }, + }, + { + HookName: "createContainer", + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "nvidia-ctk", "hook", "update-ldcache", + "--folder", + "/target-root/path/to/target", + }, + }, + { + HookName: "createContainer", + Path: "/different-root/usr/bin/nvidia-ctk", + Args: []string{ + "nvidia-ctk", "hook", "update-ldcache", + "--folder", + "/different-root/path/to/target", + }, + }, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "nvidia-ctk", "hook", "update-ldcache", + "--folder", + "/target-root/path/to/target", + }, + }, + { + HookName: "createContainer", + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "nvidia-ctk", "hook", "update-ldcache", + "--folder", + "/target-root/path/to/target", + }, + }, + { + HookName: "createContainer", + Path: "/different-root/usr/bin/nvidia-ctk", + Args: []string{ + "nvidia-ctk", "hook", "update-ldcache", + "--folder", + "/different-root/path/to/target", + }, + }, + }, + }, + }, + }, + { + description: "startContainer hook updates path and arguments", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "startContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "startContainer", + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + }, + }, + }, + }, + { + description: "createContainer hook skips path but updates arguments", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + }, + }, + }, + }, + { + description: "createRuntime hook skips path and arguments", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createRuntime", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createRuntime", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + err := New( + WithRoot(tc.root), + WithTargetRoot(tc.targetRoot), + WithRelativeTo("container"), + ).Transform(tc.spec) + require.NoError(t, err) + require.Equal(t, tc.spec, tc.expectedSpec) + }) + } +} diff --git a/pkg/nvcdi/transform/root/host-root.go b/pkg/nvcdi/transform/root/host-root.go new file mode 100644 index 00000000..ca5f7883 --- /dev/null +++ b/pkg/nvcdi/transform/root/host-root.go @@ -0,0 +1,115 @@ +/** +# 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 root + +import ( + "fmt" + "strings" + + "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" + "tags.cncf.io/container-device-interface/specs-go" +) + +// hostRootTransformer transforms the roots of host paths in a CDI spec. +type hostRootTransformer transformer + +var _ transform.Transformer = (*hostRootTransformer)(nil) + +// Transform replaces the root in a spec with a new root. +// It walks the spec and replaces all host paths that start with root with the target root. +func (t hostRootTransformer) Transform(spec *specs.Spec) error { + if spec == nil { + return nil + } + + for _, d := range spec.Devices { + d := d + if err := t.applyToEdits(&d.ContainerEdits); err != nil { + return fmt.Errorf("failed to apply root transform to device %s: %w", d.Name, err) + } + } + + if err := t.applyToEdits(&spec.ContainerEdits); err != nil { + return fmt.Errorf("failed to apply root transform to spec: %w", err) + } + return nil +} + +func (t hostRootTransformer) applyToEdits(edits *specs.ContainerEdits) error { + for i, dn := range edits.DeviceNodes { + edits.DeviceNodes[i] = t.transformDeviceNode(dn) + } + + for i, hook := range edits.Hooks { + edits.Hooks[i] = t.transformHook(hook) + } + + for i, mount := range edits.Mounts { + edits.Mounts[i] = t.transformMount(mount) + } + + return nil +} + +func (t hostRootTransformer) transformDeviceNode(dn *specs.DeviceNode) *specs.DeviceNode { + if dn.HostPath == "" { + dn.HostPath = dn.Path + } + dn.HostPath = t.transformPath(dn.HostPath) + + return dn +} + +func (t hostRootTransformer) transformHook(hook *specs.Hook) *specs.Hook { + // The Path in the startContainer hook MUST resolve in the container namespace. + if hook.HookName != "startContainer" { + hook.Path = t.transformPath(hook.Path) + } + + // The createContainer and startContainer hooks MUST execute in the container namespace. + if hook.HookName == "createContainer" || hook.HookName == "startContainer" { + return hook + } + + var args []string + for _, arg := range hook.Args { + if !strings.Contains(arg, "::") { + args = append(args, t.transformPath(arg)) + continue + } + + // For the 'create-symlinks' hook, special care is taken for the + // '--link' flag argument which takes the form ::. + // Both paths, the target and link paths, are transformed. + split := strings.SplitN(arg, "::", 2) + split[0] = t.transformPath(split[0]) + split[1] = t.transformPath(split[1]) + args = append(args, strings.Join(split, "::")) + } + hook.Args = args + + return hook +} + +func (t hostRootTransformer) transformMount(mount *specs.Mount) *specs.Mount { + mount.HostPath = t.transformPath(mount.HostPath) + return mount +} + +func (t hostRootTransformer) transformPath(path string) string { + return (transformer)(t).transformPath(path) +} diff --git a/pkg/nvcdi/transform/root_test.go b/pkg/nvcdi/transform/root/host-root_test.go similarity index 64% rename from pkg/nvcdi/transform/root_test.go rename to pkg/nvcdi/transform/root/host-root_test.go index a16d0950..6fe2f30f 100644 --- a/pkg/nvcdi/transform/root_test.go +++ b/pkg/nvcdi/transform/root/host-root_test.go @@ -14,7 +14,7 @@ # limitations under the License. **/ -package transform +package root import ( "testing" @@ -23,7 +23,7 @@ import ( "tags.cncf.io/container-device-interface/specs-go" ) -func TestRootTransformer(t *testing.T) { +func TestHostRootTransformer(t *testing.T) { testCases := []struct { description string root string @@ -150,11 +150,113 @@ func TestRootTransformer(t *testing.T) { }, }, }, + { + description: "createContainer hook skips arguments", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + }, + { + description: "startContainer hook skips path and arguments", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "startContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "startContainer", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + }, + { + description: "createRuntime hook updates path and arguments", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createRuntime", + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createRuntime", + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + }, + }, + }, + }, } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - err := NewRootTransformer(tc.root, tc.targetRoot).Transform(tc.spec) + err := New( + WithRoot(tc.root), + WithTargetRoot(tc.targetRoot), + ).Transform(tc.spec) require.NoError(t, err) require.Equal(t, tc.spec, tc.expectedSpec) }) diff --git a/pkg/nvcdi/transform/root/options.go b/pkg/nvcdi/transform/root/options.go new file mode 100644 index 00000000..1b9d5aba --- /dev/null +++ b/pkg/nvcdi/transform/root/options.go @@ -0,0 +1,41 @@ +/** +# Copyright 2023 NVIDIA CORPORATION +# +# 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 root + +// Option defines a functional option for configuring a transormer. +type Option func(*builder) + +// WithRoot sets the (from) root for the root transformer. +func WithRoot(root string) Option { + return func(b *builder) { + b.root = root + } +} + +// WithTargetRoot sets the (to) target root for the root transformer. +func WithTargetRoot(root string) Option { + return func(b *builder) { + b.targetRoot = root + } +} + +// WithRelativeTo sets whether the specified root is relative to the host or container. +func WithRelativeTo(relativeTo string) Option { + return func(b *builder) { + b.relativeTo = relativeTo + } +} diff --git a/pkg/nvcdi/transform/root/root.go b/pkg/nvcdi/transform/root/root.go new file mode 100644 index 00000000..36203fba --- /dev/null +++ b/pkg/nvcdi/transform/root/root.go @@ -0,0 +1,47 @@ +/** +# Copyright 2023 NVIDIA CORPORATION +# +# 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 root + +import ( + "path/filepath" + "strings" + + "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" +) + +// transformer transforms roots of paths. +type transformer struct { + root string + targetRoot string +} + +// New creates a root transformer using the specified options. +func New(opts ...Option) transform.Transformer { + b := &builder{} + for _, opt := range opts { + opt(b) + } + return b.build() +} + +func (t transformer) transformPath(path string) string { + if !strings.HasPrefix(path, t.root) { + return path + } + + return filepath.Join(t.targetRoot, strings.TrimPrefix(path, t.root)) +} diff --git a/tools/container/toolkit/toolkit.go b/tools/container/toolkit/toolkit.go index d57ba9c1..473d7ca7 100644 --- a/tools/container/toolkit/toolkit.go +++ b/tools/container/toolkit/toolkit.go @@ -26,7 +26,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/config" "github.com/NVIDIA/nvidia-container-toolkit/internal/system/nvdevices" "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi" - "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" + transformroot "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform/root" toml "github.com/pelletier/go-toml" log "github.com/sirupsen/logrus" "github.com/urfave/cli/v2" @@ -712,9 +712,9 @@ func generateCDISpec(opts *options, nvidiaCTKPath string) error { if err != nil { return fmt.Errorf("failed to genereate CDI spec for management containers: %v", err) } - err = transform.NewRootTransformer( - opts.DriverRootCtrPath, - opts.DriverRoot, + err = transformroot.New( + transformroot.WithRoot(opts.DriverRootCtrPath), + transformroot.WithTargetRoot(opts.DriverRoot), ).Transform(spec.Raw()) if err != nil { return fmt.Errorf("failed to transform driver root in CDI spec: %v", err) From bc4e19aa48309acdc77e70bcff3afc6751fd9954 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 27 Nov 2023 11:17:55 +0100 Subject: [PATCH 2/2] Add --relative-to option to nvidia-ctk transform root This change adds a --relative-to option to the nvidia-ctk transform root command. This defaults to "host" maintaining the existing behaviour. If --relative-to=container is specified, the root transform is applied to container paths in the CDI specification instead of host paths. Signed-off-by: Evan Lezar --- CHANGELOG.md | 1 + cmd/nvidia-ctk/cdi/transform/root/root.go | 32 ++++++++++++++++------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f7389e0..c4812fde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Add support for injecting /dev/nvidia-nvswitch* devices if the NVIDIA_NVSWITCH=enabled envvar is specified. * Added support for `nvidia-ctk runtime configure --enable-cdi` for the `docker` runtime. Note that this requires Docker >= 25. * Fixed bug in `nvidia-ctk config` command when using `--set`. The types of applied config options are now applied correctly. +* Add `--relative-to` option to `nvidia-ctk transform root` command. This controls whether the root transformation is applied to host or container paths. * [libnvidia-container] Fix device permission check when using cgroupv2 (fixes #227) diff --git a/cmd/nvidia-ctk/cdi/transform/root/root.go b/cmd/nvidia-ctk/cdi/transform/root/root.go index 8933a815..4de8d9c6 100644 --- a/cmd/nvidia-ctk/cdi/transform/root/root.go +++ b/cmd/nvidia-ctk/cdi/transform/root/root.go @@ -23,7 +23,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/spec" - "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" + transformroot "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform/root" "github.com/urfave/cli/v2" "tags.cncf.io/container-device-interface/pkg/cdi" ) @@ -39,8 +39,9 @@ type transformOptions struct { type options struct { transformOptions - from string - to string + from string + to string + relativeTo string } // NewCommand constructs a generate-cdi command with the specified logger @@ -67,6 +68,11 @@ func (m command) build() *cli.Command { } c.Flags = []cli.Flag{ + &cli.StringFlag{ + Name: "from", + Usage: "specify the root to be transformed", + Destination: &opts.from, + }, &cli.StringFlag{ Name: "input", Usage: "Specify the file to read the CDI specification from. If this is '-' the specification is read from STDIN", @@ -79,9 +85,10 @@ func (m command) build() *cli.Command { Destination: &opts.output, }, &cli.StringFlag{ - Name: "from", - Usage: "specify the root to be transformed", - Destination: &opts.from, + Name: "relative-to", + Usage: "specify whether the transform is relative to the host or to the container. One of [ host | container ]", + Value: "host", + Destination: &opts.relativeTo, }, &cli.StringFlag{ Name: "to", @@ -95,6 +102,12 @@ func (m command) build() *cli.Command { } func (m command) validateFlags(c *cli.Context, opts *options) error { + switch opts.relativeTo { + case "host": + case "container": + default: + return fmt.Errorf("invalid --relative-to value: %v", opts.relativeTo) + } return nil } @@ -104,9 +117,10 @@ func (m command) run(c *cli.Context, opts *options) error { return fmt.Errorf("failed to load CDI specification: %w", err) } - err = transform.NewHostRootTransformer( - opts.from, - opts.to, + err = transformroot.New( + transformroot.WithRoot(opts.from), + transformroot.WithTargetRoot(opts.to), + transformroot.WithRelativeTo(opts.relativeTo), ).Transform(spec.Raw()) if err != nil { return fmt.Errorf("failed to transform CDI specification: %w", err)