Merge pull request #776 from elezar/fix-libcuda-symlink

Force symlink creation in create-symlink hook
This commit is contained in:
Evan Lezar 2024-11-07 18:28:01 +01:00 committed by GitHub
commit 3cb613a12b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 87 additions and 52 deletions

View File

@ -63,7 +63,7 @@ func (m command) build() *cli.Command {
c.Flags = []cli.Flag{
&cli.StringSliceFlag{
Name: "link",
Usage: "Specify a specific link to create. The link is specified as target::link",
Usage: "Specify a specific link to create. The link is specified as target::link. If the link exists in the container root, it is removed.",
Destination: &cfg.links,
},
// The following flags are testing-only flags.
@ -112,18 +112,19 @@ func (m command) run(c *cli.Context, cfg *config) error {
// createLink creates a symbolic link in the specified container root.
// This is equivalent to:
//
// chroot {{ .containerRoot }} ln -s {{ .target }} {{ .link }}
// chroot {{ .containerRoot }} ln -f -s {{ .target }} {{ .link }}
//
// If the specified link already exists and points to the same target, this
// operation is a no-op. If the link points to a different target, an error is
// returned.
// operation is a no-op.
// If a file exists at the link path or the link points to a different target
// this file is removed before creating the link.
//
// Note that if the link path resolves to an absolute path oudside of the
// specified root, this is treated as an absolute path in this root.
func (m command) createLink(containerRoot string, targetPath string, link string) error {
linkPath := filepath.Join(containerRoot, link)
exists, err := doesLinkExist(targetPath, linkPath)
exists, err := linkExists(targetPath, linkPath)
if err != nil {
return fmt.Errorf("failed to check if link exists: %w", err)
}
@ -132,17 +133,21 @@ func (m command) createLink(containerRoot string, targetPath string, link string
return nil
}
resolvedLinkPath, err := symlink.FollowSymlinkInScope(linkPath, containerRoot)
// We resolve the parent of the symlink that we're creating in the container root.
// If we resolve the full link path, an existing link at the location itself
// is also resolved here and we are unable to force create the link.
resolvedLinkParent, err := symlink.FollowSymlinkInScope(filepath.Dir(linkPath), containerRoot)
if err != nil {
return fmt.Errorf("failed to follow path for link %v relative to %v: %w", link, containerRoot, err)
}
resolvedLinkPath := filepath.Join(resolvedLinkParent, filepath.Base(linkPath))
m.logger.Infof("Symlinking %v to %v", resolvedLinkPath, targetPath)
err = os.MkdirAll(filepath.Dir(resolvedLinkPath), 0755)
if err != nil {
return fmt.Errorf("failed to create directory: %v", err)
}
err = os.Symlink(targetPath, resolvedLinkPath)
err = symlinks.ForceCreate(targetPath, resolvedLinkPath)
if err != nil {
return fmt.Errorf("failed to create symlink: %v", err)
}
@ -150,9 +155,9 @@ func (m command) createLink(containerRoot string, targetPath string, link string
return nil
}
// doesLinkExist returns true if link exists and points to target.
// An error is returned if link exists but points to a different target.
func doesLinkExist(target string, link string) (bool, error) {
// linkExists checks whether the specified link exists.
// A link exists if the path exists, is a symlink, and points to the specified target.
func linkExists(target string, link string) (bool, error) {
currentTarget, err := symlinks.Resolve(link)
if errors.Is(err, os.ErrNotExist) {
return false, nil
@ -163,5 +168,5 @@ func doesLinkExist(target string, link string) (bool, error) {
if currentTarget == target {
return true, nil
}
return true, fmt.Errorf("unexpected link target: %s", currentTarget)
return false, nil
}

View File

@ -12,7 +12,7 @@ import (
"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup/symlinks"
)
func TestDoesLinkExist(t *testing.T) {
func TestLinkExist(t *testing.T) {
tmpDir := t.TempDir()
require.NoError(
t,
@ -22,21 +22,23 @@ func TestDoesLinkExist(t *testing.T) {
),
)
exists, err := doesLinkExist("d", filepath.Join(tmpDir, "/a/b/c"))
exists, err := linkExists("d", filepath.Join(tmpDir, "/a/b/c"))
require.NoError(t, err)
require.True(t, exists)
exists, err = doesLinkExist("/a/b/f", filepath.Join(tmpDir, "/a/b/e"))
exists, err = linkExists("/a/b/f", filepath.Join(tmpDir, "/a/b/e"))
require.NoError(t, err)
require.True(t, exists)
_, err = doesLinkExist("different-target", filepath.Join(tmpDir, "/a/b/c"))
require.Error(t, err)
exists, err = linkExists("different-target", filepath.Join(tmpDir, "/a/b/c"))
require.NoError(t, err)
require.False(t, exists)
_, err = doesLinkExist("/a/b/d", filepath.Join(tmpDir, "/a/b/c"))
require.Error(t, err)
exists, err = linkExists("/a/b/d", filepath.Join(tmpDir, "/a/b/c"))
require.NoError(t, err)
require.False(t, exists)
exists, err = doesLinkExist("foo", filepath.Join(tmpDir, "/a/b/does-not-exist"))
exists, err = linkExists("foo", filepath.Join(tmpDir, "/a/b/does-not-exist"))
require.NoError(t, err)
require.False(t, exists)
}
@ -190,43 +192,55 @@ func TestCreateLinkAbsolutePath(t *testing.T) {
}
func TestCreateLinkAlreadyExists(t *testing.T) {
tmpDir := t.TempDir()
hostRoot := filepath.Join(tmpDir, "/host-root/")
containerRoot := filepath.Join(tmpDir, "/container-root")
testCases := []struct {
description string
containerContents []dirOrLink
shouldExist []string
}{
{
description: "link already exists with correct target",
containerContents: []dirOrLink{{path: "/lib/libfoo.so", target: "libfoo.so.1"}},
shouldExist: []string{},
},
{
description: "link already exists with different target",
containerContents: []dirOrLink{{path: "/lib/libfoo.so", target: "different-target"}, {path: "different-target"}},
shouldExist: []string{"{{ .containerRoot }}/different-target"},
},
}
require.NoError(t, makeFs(hostRoot))
require.NoError(t, makeFs(containerRoot, dirOrLink{path: "/lib/libfoo.so", target: "libfoo.so.1"}))
for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
tmpDir := t.TempDir()
hostRoot := filepath.Join(tmpDir, "/host-root/")
containerRoot := filepath.Join(tmpDir, "/container-root")
require.NoError(t, makeFs(hostRoot))
require.NoError(t, makeFs(containerRoot, tc.containerContents...))
// nvidia-cdi-hook create-symlinks --link libfoo.so.1::/lib/libfoo.so
err := getTestCommand().createLink(containerRoot, "libfoo.so.1", "/lib/libfoo.so")
require.NoError(t, err)
target, err := symlinks.Resolve(filepath.Join(containerRoot, "lib/libfoo.so"))
require.NoError(t, err)
require.Equal(t, "libfoo.so.1", target)
}
// nvidia-cdi-hook create-symlinks --link libfoo.so.1::/lib/libfoo.so
err := getTestCommand().createLink(containerRoot, "libfoo.so.1", "/lib/libfoo.so")
require.NoError(t, err)
target, err := symlinks.Resolve(filepath.Join(containerRoot, "lib/libfoo.so"))
require.NoError(t, err)
require.Equal(t, "libfoo.so.1", target)
func TestCreateLinkAlreadyExistsDifferentTarget(t *testing.T) {
tmpDir := t.TempDir()
hostRoot := filepath.Join(tmpDir, "/host-root/")
containerRoot := filepath.Join(tmpDir, "/container-root")
require.NoError(t, makeFs(hostRoot))
require.NoError(t, makeFs(containerRoot, dirOrLink{path: "/lib/libfoo.so", target: "different-target"}))
// nvidia-cdi-hook create-symlinks --link libfoo.so.1::/lib/libfoo.so
err := getTestCommand().createLink(containerRoot, "libfoo.so.1", "/lib/libfoo.so")
require.Error(t, err)
target, err := symlinks.Resolve(filepath.Join(containerRoot, "lib/libfoo.so"))
require.NoError(t, err)
require.Equal(t, "different-target", target)
for _, p := range tc.shouldExist {
require.DirExists(t, strings.ReplaceAll(p, "{{ .containerRoot }}", containerRoot))
}
})
}
}
func TestCreateLinkOutOfBounds(t *testing.T) {
tmpDir := t.TempDir()
hostRoot := filepath.Join(tmpDir, "/host-root/")
hostRoot := filepath.Join(tmpDir, "/host-root")
containerRoot := filepath.Join(tmpDir, "/container-root")
require.NoError(t, makeFs(hostRoot))
require.NoError(t,
makeFs(hostRoot,
dirOrLink{path: "libfoo.so"},
),
)
require.NoError(t,
makeFs(containerRoot,
dirOrLink{path: "/lib"},
@ -240,12 +254,13 @@ func TestCreateLinkOutOfBounds(t *testing.T) {
// nvidia-cdi-hook create-symlinks --link ../libfoo.so.1::/lib/foo/libfoo.so
_ = getTestCommand().createLink(containerRoot, "../libfoo.so.1", "/lib/foo/libfoo.so")
// TODO: We need to enabled this check once we have updated the implementation.
// require.Error(t, err)
_, err = os.Lstat(filepath.Join(hostRoot, "libfoo.so"))
require.ErrorIs(t, err, os.ErrNotExist)
_, err = os.Lstat(filepath.Join(containerRoot, hostRoot, "libfoo.so"))
require.NoError(t, err)
target, err := symlinks.Resolve(filepath.Join(containerRoot, hostRoot, "libfoo.so"))
require.NoError(t, err)
require.Equal(t, "../libfoo.so.1", target)
require.DirExists(t, filepath.Join(hostRoot, "libfoo.so"))
}
type dirOrLink struct {

View File

@ -33,3 +33,18 @@ func Resolve(filename string) (string, error) {
return os.Readlink(filename)
}
// ForceCreate creates a specified symlink.
// If a file (or empty directory) exists at the path it is removed.
func ForceCreate(target string, link string) error {
_, err := os.Lstat(link)
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to get file info: %w", err)
}
if !os.IsNotExist(err) {
if err := os.Remove(link); err != nil {
return fmt.Errorf("failed to remove existing file: %w", err)
}
}
return os.Symlink(target, link)
}