From 5638f47cb0e8f9d96d20b3e53efdc9901297ffaf Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 8 Aug 2023 14:21:55 +0200 Subject: [PATCH 1/3] Add sort CDI spec transoformer Signed-off-by: Evan Lezar --- pkg/nvcdi/transform/sorter.go | 95 ++++++++ pkg/nvcdi/transform/sorter_test.go | 355 +++++++++++++++++++++++++++++ 2 files changed, 450 insertions(+) create mode 100644 pkg/nvcdi/transform/sorter.go create mode 100644 pkg/nvcdi/transform/sorter_test.go diff --git a/pkg/nvcdi/transform/sorter.go b/pkg/nvcdi/transform/sorter.go new file mode 100644 index 00000000..e64de305 --- /dev/null +++ b/pkg/nvcdi/transform/sorter.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 ( + "os" + "path/filepath" + "sort" + "strings" + + "github.com/container-orchestrated-devices/container-device-interface/specs-go" +) + +type sorter struct{} + +var _ Transformer = (*sorter)(nil) + +// NewSorter creates a transformer that sorts container edits. +func NewSorter() Transformer { + return nil +} + +// Transform sorts the entities in the specified CDI specification. +func (d sorter) Transform(spec *specs.Spec) error { + if spec == nil { + return nil + } + if err := d.transformEdits(&spec.ContainerEdits); err != nil { + return err + } + var updatedDevices []specs.Device + for _, device := range spec.Devices { + if err := d.transformEdits(&device.ContainerEdits); err != nil { + return err + } + updatedDevices = append(updatedDevices, device) + } + spec.Devices = d.sortDevices(updatedDevices) + return nil +} + +func (d sorter) transformEdits(edits *specs.ContainerEdits) error { + edits.DeviceNodes = d.sortDeviceNodes(edits.DeviceNodes) + edits.Mounts = d.sortMounts(edits.Mounts) + return nil +} + +func (d sorter) sortDevices(devices []specs.Device) []specs.Device { + sort.Slice(devices, func(i, j int) bool { + return devices[i].Name < devices[j].Name + }) + return devices +} + +// sortDeviceNodes sorts the specified device nodes by container path. +// If two device nodes have the same container path, the host path is used to break ties. +func (d sorter) sortDeviceNodes(entities []*specs.DeviceNode) []*specs.DeviceNode { + sort.Slice(entities, func(i, j int) bool { + ip := strings.Count(filepath.Clean(entities[i].Path), string(os.PathSeparator)) + jp := strings.Count(filepath.Clean(entities[j].Path), string(os.PathSeparator)) + if ip == jp { + return entities[i].Path < entities[j].Path + } + return ip < jp + }) + return entities +} + +// sortMounts sorts the specified mounts by container path. +// If two mounts have the same mount path, the host path is used to break ties. +func (d sorter) sortMounts(entities []*specs.Mount) []*specs.Mount { + sort.Slice(entities, func(i, j int) bool { + ip := strings.Count(filepath.Clean(entities[i].ContainerPath), string(os.PathSeparator)) + jp := strings.Count(filepath.Clean(entities[j].ContainerPath), string(os.PathSeparator)) + if ip == jp { + return entities[i].ContainerPath < entities[j].ContainerPath + } + return ip < jp + }) + return entities +} diff --git a/pkg/nvcdi/transform/sorter_test.go b/pkg/nvcdi/transform/sorter_test.go new file mode 100644 index 00000000..94643ca9 --- /dev/null +++ b/pkg/nvcdi/transform/sorter_test.go @@ -0,0 +1,355 @@ +/** +# 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 TestSortSpec(t *testing.T) { + testCases := []struct { + description string + spec *specs.Spec + expected *specs.Spec + }{ + { + description: "sort sorts devices by name and device edits", + spec: &specs.Spec{ + Devices: []specs.Device{ + { + Name: "device2", + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/device2/dev0", + }, + { + Path: "/dev/device2/dev1", + }, + }, + Mounts: []*specs.Mount{ + { + ContainerPath: "/lib/device2/mount1", + }, + { + ContainerPath: "/lib/device2/mount0", + }, + }, + }, + }, + { + Name: "device1", + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/device1/dev1", + }, + { + Path: "/dev/device1/dev0", + }, + }, + Mounts: []*specs.Mount{ + { + ContainerPath: "/lib/device1/mount0", + }, + { + ContainerPath: "/lib/device1/mount1", + }, + }, + }, + }, + }, + }, + expected: &specs.Spec{ + Devices: []specs.Device{ + { + Name: "device1", + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/device1/dev0", + }, + { + Path: "/dev/device1/dev1", + }, + }, + Mounts: []*specs.Mount{ + { + ContainerPath: "/lib/device1/mount0", + }, + { + ContainerPath: "/lib/device1/mount1", + }, + }, + }, + }, + { + Name: "device2", + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/device2/dev0", + }, + { + Path: "/dev/device2/dev1", + }, + }, + Mounts: []*specs.Mount{ + { + ContainerPath: "/lib/device2/mount0", + }, + { + ContainerPath: "/lib/device2/mount1", + }, + }, + }, + }, + }, + }, + }, + { + description: "sort sorts common edits", + spec: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/device2/dev0", + }, + { + Path: "/dev/device2/dev1", + }, + { + Path: "/dev/device1/dev1", + }, + { + Path: "/dev/device1/dev0", + }, + }, + Mounts: []*specs.Mount{ + { + ContainerPath: "/lib/device2/mount1", + }, + { + ContainerPath: "/lib/device2/mount0", + }, + { + ContainerPath: "/lib/device1/mount0", + }, + { + ContainerPath: "/lib/device1/mount1", + }, + }, + }, + }, + expected: &specs.Spec{ + ContainerEdits: specs.ContainerEdits{ + DeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/device1/dev0", + }, + { + Path: "/dev/device1/dev1", + }, + { + Path: "/dev/device2/dev0", + }, + { + Path: "/dev/device2/dev1", + }, + }, + Mounts: []*specs.Mount{ + { + ContainerPath: "/lib/device1/mount0", + }, + { + ContainerPath: "/lib/device1/mount1", + }, + { + ContainerPath: "/lib/device2/mount0", + }, + { + ContainerPath: "/lib/device2/mount1", + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + s := sorter{} + err := s.Transform(tc.spec) + require.NoError(t, err) + + require.EqualValues(t, tc.expected, tc.spec) + }) + } +} + +func TestSortDeviceNodes(t *testing.T) { + testCases := []struct { + description string + deviceNodes []*specs.DeviceNode + expectedDeviceNodes []*specs.DeviceNode + }{ + { + description: "sorted remains sorted", + deviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/nvidia0", + }, + { + Path: "/dev/nvidia1", + }, + }, + expectedDeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/nvidia0", + }, + { + Path: "/dev/nvidia1", + }, + }, + }, + { + description: "unsorted gets sorted", + deviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/nvidia1", + }, + { + Path: "/dev/nvidia0", + }, + }, + expectedDeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/nvidia0", + }, + { + Path: "/dev/nvidia1", + }, + }, + }, + { + description: "shorter paths are first", + deviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/nvidia0/another", + }, + { + Path: "/dev/nvidia0", + }, + }, + expectedDeviceNodes: []*specs.DeviceNode{ + { + Path: "/dev/nvidia0", + }, + { + Path: "/dev/nvidia0/another", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + s := sorter{} + sorted := s.sortDeviceNodes(tc.deviceNodes) + + require.EqualValues(t, tc.expectedDeviceNodes, sorted) + }) + } +} + +func TestStortMounts(t *testing.T) { + testCases := []struct { + description string + mounts []*specs.Mount + expectedMounts []*specs.Mount + }{ + { + description: "sorted remains sorted", + mounts: []*specs.Mount{ + { + ContainerPath: "/lib/nvidia0", + }, + { + ContainerPath: "/lib/nvidia1", + }, + }, + expectedMounts: []*specs.Mount{ + { + ContainerPath: "/lib/nvidia0", + }, + { + ContainerPath: "/lib/nvidia1", + }, + }, + }, + { + description: "unsorted gets sorted", + mounts: []*specs.Mount{ + { + ContainerPath: "/lib/nvidia1", + }, + { + ContainerPath: "/lib/nvidia0", + }, + }, + expectedMounts: []*specs.Mount{ + { + ContainerPath: "/lib/nvidia0", + }, + { + ContainerPath: "/lib/nvidia1", + }, + }, + }, + { + description: "shorter paths are first", + mounts: []*specs.Mount{ + { + ContainerPath: "/lib/nvidia0/another", + }, + { + ContainerPath: "/lib/nvidia0", + }, + }, + expectedMounts: []*specs.Mount{ + { + ContainerPath: "/lib/nvidia0", + }, + { + ContainerPath: "/lib/nvidia0/another", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + s := sorter{} + sorted := s.sortMounts(tc.mounts) + + require.EqualValues(t, tc.expectedMounts, sorted) + }) + } +} From 7a4d2cff67a976a9ea0d56f146f3803dd0439c26 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 8 Aug 2023 14:30:59 +0200 Subject: [PATCH 2/3] Add merged CDI spec transformer Signed-off-by: Evan Lezar --- pkg/nvcdi/transform/merge.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 pkg/nvcdi/transform/merge.go diff --git a/pkg/nvcdi/transform/merge.go b/pkg/nvcdi/transform/merge.go new file mode 100644 index 00000000..2fd9a6a9 --- /dev/null +++ b/pkg/nvcdi/transform/merge.go @@ -0,0 +1,36 @@ +/** +# 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 merged []Transformer + +// Merge creates a merged transofrmer from the specified transformers. +func Merge(transformers ...Transformer) Transformer { + return merged(transformers) +} + +// Transform applies all the transformers in the merged set. +func (t merged) Transform(spec *specs.Spec) error { + for _, transformer := range t { + if err := transformer.Transform(spec); err != nil { + return err + } + } + return nil +} From cbdbcd87ffef9971053ad59e76578a9565e33e0a Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 8 Aug 2023 14:31:39 +0200 Subject: [PATCH 3/3] Add sorter to simplifying transformer Signed-off-by: Evan Lezar --- pkg/nvcdi/transform/merged-device_test.go | 24 +++++++++++------------ pkg/nvcdi/transform/simplify.go | 6 +++++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/pkg/nvcdi/transform/merged-device_test.go b/pkg/nvcdi/transform/merged-device_test.go index d8ef7244..10328530 100644 --- a/pkg/nvcdi/transform/merged-device_test.go +++ b/pkg/nvcdi/transform/merged-device_test.go @@ -163,6 +163,18 @@ func TestMergedDevice(t *testing.T) { }, expectedSpec: &specs.Spec{ Devices: []specs.Device{ + { + Name: "all", + ContainerEdits: specs.ContainerEdits{ + Hooks: []*specs.Hook{ + { + HookName: "createContainer", + Path: "/usr/bin/nvidia-ctk", + Args: []string{"nvidia-ctk", "hook", "chmod", "--mode", "755", "--path", "/dev/dri"}, + }, + }, + }, + }, { Name: "gpu0", ContainerEdits: specs.ContainerEdits{ @@ -187,18 +199,6 @@ func TestMergedDevice(t *testing.T) { }, }, }, - { - Name: "all", - ContainerEdits: specs.ContainerEdits{ - Hooks: []*specs.Hook{ - { - HookName: "createContainer", - Path: "/usr/bin/nvidia-ctk", - Args: []string{"nvidia-ctk", "hook", "chmod", "--mode", "755", "--path", "/dev/dri"}, - }, - }, - }, - }, }, }, }, diff --git a/pkg/nvcdi/transform/simplify.go b/pkg/nvcdi/transform/simplify.go index a2162ad6..ddc6f597 100644 --- a/pkg/nvcdi/transform/simplify.go +++ b/pkg/nvcdi/transform/simplify.go @@ -29,7 +29,11 @@ var _ Transformer = (*simplify)(nil) // NewSimplifier creates a simplifier transformer. // This transoformer ensures that entities in the spec are deduplicated and that common edits are removed from device-specific edits. func NewSimplifier() Transformer { - return &simplify{} + return Merge( + dedupe{}, + simplify{}, + sorter{}, + ) } // Transform simplifies the supplied spec.