From 0516fc96ca46dbf9794fd096cd888452e317ff5f Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Wed, 22 Feb 2023 11:42:38 -0800 Subject: [PATCH 1/3] Add Transform interface and initial implemention for a root transform Signed-off-by: Christopher Desiniotis --- pkg/nvcdi/transform/api.go | 24 ++++++ pkg/nvcdi/transform/root.go | 95 +++++++++++++++++++++++ pkg/nvcdi/transform/root_test.go | 126 +++++++++++++++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 pkg/nvcdi/transform/api.go create mode 100644 pkg/nvcdi/transform/root.go create mode 100644 pkg/nvcdi/transform/root_test.go diff --git a/pkg/nvcdi/transform/api.go b/pkg/nvcdi/transform/api.go new file mode 100644 index 00000000..9e3e5bd6 --- /dev/null +++ b/pkg/nvcdi/transform/api.go @@ -0,0 +1,24 @@ +/** +# 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 "github.com/container-orchestrated-devices/container-device-interface/specs-go" + +// Transformer defines the API for applying arbitrary transforms to a spec in-place +type Transformer interface { + Transform(*specs.Spec) error +} diff --git a/pkg/nvcdi/transform/root.go b/pkg/nvcdi/transform/root.go new file mode 100644 index 00000000..6910ad2e --- /dev/null +++ b/pkg/nvcdi/transform/root.go @@ -0,0 +1,95 @@ +/** +# 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" + + "github.com/container-orchestrated-devices/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. +func NewRootTransformer(root string, targetRoot string) Transformer { + 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 { + 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) + + // TODO: The args need more attention. For the hooks that create symlinks we would have to transform these too. + var args []string + for _, arg := range hook.Args { + args = append(args, t.transformPath(arg)) + } + 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_test.go b/pkg/nvcdi/transform/root_test.go new file mode 100644 index 00000000..1689d5b1 --- /dev/null +++ b/pkg/nvcdi/transform/root_test.go @@ -0,0 +1,126 @@ +/** +# 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 ( + "testing" + + "github.com/container-orchestrated-devices/container-device-interface/specs-go" + "github.com/stretchr/testify/require" +) + +func TestRootTransformer(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"}, + {HostPath: "/target-root/dev/nvidia1"}, + {HostPath: "/different-root/dev/nvidia2"}, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + {HostPath: "/target-root/dev/nvidia0"}, + {HostPath: "/target-root/dev/nvidia1"}, + {HostPath: "/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"}, + {HostPath: "/target-root/lib/lib2.so"}, + {HostPath: "/different-root/lib/lib3.so"}, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Mounts: []*specs.Mount{ + {HostPath: "/target-root/lib/lib1.so"}, + {HostPath: "/target-root/lib/lib2.so"}, + {HostPath: "/different-root/lib/lib3.so"}, + }, + }, + }, + }, + { + description: "hooks", + root: "/root", + targetRoot: "/target-root", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + {Path: "/root/usr/bin/nvidia-ctk"}, + {Path: "/target-root/usr/bin/nvidia-ctk"}, + {Path: "/different-root/usr/bin/nvidia-ctk"}, + }, + }, + }, + expectedSpec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + {Path: "/target-root/usr/bin/nvidia-ctk"}, + {Path: "/target-root/usr/bin/nvidia-ctk"}, + {Path: "/different-root/usr/bin/nvidia-ctk"}, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + err := NewRootTransformer(tc.root, tc.targetRoot).Transform(tc.spec) + require.NoError(t, err) + require.Equal(t, tc.spec, tc.expectedSpec) + }) + } +} From 45ed3b0412bad9baa9c2cddbbf4c84e7a7179ef5 Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Wed, 22 Feb 2023 13:11:10 -0800 Subject: [PATCH 2/3] Handle hook arguments for creation of symlinks Signed-off-by: Christopher Desiniotis --- pkg/nvcdi/transform/no-op.go | 35 +++++++++++++++++++++++ pkg/nvcdi/transform/root.go | 24 ++++++++++++++-- pkg/nvcdi/transform/root_test.go | 48 ++++++++++++++++++++++++++++---- 3 files changed, 98 insertions(+), 9 deletions(-) create mode 100644 pkg/nvcdi/transform/no-op.go diff --git a/pkg/nvcdi/transform/no-op.go b/pkg/nvcdi/transform/no-op.go new file mode 100644 index 00000000..7985f195 --- /dev/null +++ b/pkg/nvcdi/transform/no-op.go @@ -0,0 +1,35 @@ +/** +# 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 ( + "github.com/container-orchestrated-devices/container-device-interface/specs-go" +) + +type noop struct{} + +var _ Transformer = (*noop)(nil) + +// NewNoopTransformer returns a no-op transformer +func NewNoopTransformer() Transformer { + return noop{} +} + +// Transform is a no-op +func (n noop) Transform(spec *specs.Spec) error { + return nil +} diff --git a/pkg/nvcdi/transform/root.go b/pkg/nvcdi/transform/root.go index 6910ad2e..53bcf2a4 100644 --- a/pkg/nvcdi/transform/root.go +++ b/pkg/nvcdi/transform/root.go @@ -32,8 +32,13 @@ type rootTransformer struct { var _ Transformer = (*rootTransformer)(nil) // NewRootTransformer creates a new transformer for modifying -// the root for paths in a CDI spec. +// 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, @@ -69,10 +74,23 @@ func (t rootTransformer) applyToEdits(edits *specs.ContainerEdits) error { for i, hook := range edits.Hooks { hook.Path = t.transformPath(hook.Path) - // TODO: The args need more attention. For the hooks that create symlinks we would have to transform these too. var args []string for _, arg := range hook.Args { - args = append(args, t.transformPath(arg)) + 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 diff --git a/pkg/nvcdi/transform/root_test.go b/pkg/nvcdi/transform/root_test.go index 1689d5b1..519c9eae 100644 --- a/pkg/nvcdi/transform/root_test.go +++ b/pkg/nvcdi/transform/root_test.go @@ -98,18 +98,54 @@ func TestRootTransformer(t *testing.T) { spec: &specs.Spec{ ContainerEdits: specs.ContainerEdits{ Hooks: []*specs.Hook{ - {Path: "/root/usr/bin/nvidia-ctk"}, - {Path: "/target-root/usr/bin/nvidia-ctk"}, - {Path: "/different-root/usr/bin/nvidia-ctk"}, + { + Path: "/root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/root/path/to/target::/root/path/to/link", + }, + }, + { + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + { + 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{ - {Path: "/target-root/usr/bin/nvidia-ctk"}, - {Path: "/target-root/usr/bin/nvidia-ctk"}, - {Path: "/different-root/usr/bin/nvidia-ctk"}, + { + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + { + Path: "/target-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/target-root/path/to/target::/target-root/path/to/link", + }, + }, + { + Path: "/different-root/usr/bin/nvidia-ctk", + Args: []string{ + "--link", + "/different-root/path/to/target::/different-root/path/to/link", + }, + }, }, }, }, From 87e406eee62bb47fdfd98f18da3919b3edb2e4c5 Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Thu, 23 Feb 2023 11:08:04 -0800 Subject: [PATCH 3/3] Update root transformer tests to ensure container path is not modified Signed-off-by: Christopher Desiniotis --- pkg/nvcdi/transform/root_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/nvcdi/transform/root_test.go b/pkg/nvcdi/transform/root_test.go index 519c9eae..0d036f90 100644 --- a/pkg/nvcdi/transform/root_test.go +++ b/pkg/nvcdi/transform/root_test.go @@ -52,18 +52,18 @@ func TestRootTransformer(t *testing.T) { spec: &specs.Spec{ ContainerEdits: specs.ContainerEdits{ DeviceNodes: []*specs.DeviceNode{ - {HostPath: "/root/dev/nvidia0"}, - {HostPath: "/target-root/dev/nvidia1"}, - {HostPath: "/different-root/dev/nvidia2"}, + {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: "/target-root/dev/nvidia0"}, - {HostPath: "/target-root/dev/nvidia1"}, - {HostPath: "/different-root/dev/nvidia2"}, + {HostPath: "/target-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"}, }, }, }, @@ -75,18 +75,18 @@ func TestRootTransformer(t *testing.T) { spec: &specs.Spec{ ContainerEdits: specs.ContainerEdits{ Mounts: []*specs.Mount{ - {HostPath: "/root/lib/lib1.so"}, - {HostPath: "/target-root/lib/lib2.so"}, - {HostPath: "/different-root/lib/lib3.so"}, + {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: "/target-root/lib/lib1.so"}, - {HostPath: "/target-root/lib/lib2.so"}, - {HostPath: "/different-root/lib/lib3.so"}, + {HostPath: "/target-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"}, }, }, },