Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NVSHAS-8532 - Failed to find container's symbolic link at the same folder #1189

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pohanhuangtw
Copy link
Contributor

@pohanhuangtw pohanhuangtw commented Jan 25, 2024

Summary

  1. Fix the symlink issues in a folder, since the folder itself can be another symlink
  2. Add unit-test cases on both relative symlink and absolute symlink
  3. retryResolveSymlink is a function to iterative resolve the symlink by checking if the prefix of the not exist files can be potential symlink folder, then resolve it (only call at fail of os.Lstat)

How to verify

pohanhuangtw/allinone:8532

@@ -139,6 +207,17 @@ func GetContainerRealFilePath(pid int, symlinkPath string, inTest bool) (string,

visitedSymlink[currentPath] = struct{}{}
if underProcRoot {
if _, err := os.Lstat(resolvedPath); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the error first with https://pkg.go.dev/os#IsNotExist ?

@@ -81,6 +81,74 @@ func extractProcRootPath(pid int, input string, inTest bool) (string, error) {
}
}

// The retryResolveSymlink splits the path into parts and checks each segment for symlinks, updating the path as it resolves them.
func retryResolveSymlink(procRoot, prefix, remaining string) (bool, string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this logic support this case?

  • Resolving /a/b/c in a container.
  • /a is a symlink pointing to /d/e/f
  • /d/e is another symlink pointing to /g/h/i

The expected result should be /g/h/i/f/b/c. The flow is:

  1. /a/b/c => /d/e/f + /b/c
  2. /d/e/f/b/c => /g/h/i + /f/b/c => /g/h/i/f/b/c

But from your code, it might become /d/e/f/b/c?

  1. /a => /d/e/f
  2. /d/e/f + /b => /d/e/f/b
  3. /d/e/f/b + /c => /d/e/f/b/c
# ls -al a 
lrwxrwxrwx 1 root root 6 Jan 25 16:22 a -> /d/e/f
# ls -al /d/e
lrwxrwxrwx 1 root root 6 Jan 25 16:22 /d/e -> /g/h/i
# stat /a/b/c
Device: 20002eh/2097198d	Inode: 58720946    Links: 1
# stat /g/h/i/f/b/c
Device: 20002eh/2097198d	Inode: 58720946    Links: 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @holyspectral ,
Thanks for the review!
You're correct, the correct way is to recursive back to re-run it from beginning.
In new commit, I add a traverse count to limit the number of traverse, and guarantee retry from the beginning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants