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.

Sign up for GitHub

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

Jump to bottom

Support ls #120

Open
9 of 15 tasks
eyalb181 opened this issue Jun 7, 2022 · 9 comments
Open
9 of 15 tasks

Support ls #120

eyalb181 opened this issue Jun 7, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@eyalb181
Copy link
Member

eyalb181 commented Jun 7, 2022 •
edited by infiniteregrets
Loading

Our files feature currently support most of the relevant syscalls, but we still need to add support for ls.

After #904, this is what is left to be done (prioritized)

  • Go Linux x86-64 (same flow/integration test should work~)
  • Python list directory
  • Node list directory
  • ls binary - macOS
  • ls binary - Linux
  • All of the libc functions:
    • opendir
    • fdopendir
    • readdir
    • readdir_r
    • telldir
    • seekdir
    • rewinddir
    • closedir
    • dirfd

I think the best way would be to drop each as separate PRs, first few tasks will resolve most of the libc functions :)
@eyalb181 eyalb181 added the enhancement New feature or request label Jun 7, 2022
@eyalb181 eyalb181 assigned meowjesty and unassigned meowjesty Jun 7, 2022
@infiniteregrets infiniteregrets self-assigned this Jun 8, 2022
@infiniteregrets infiniteregrets mentioned this issue Jun 10, 2022
Closed
@infiniteregrets
Copy link
Contributor

openat was fixed in #151, detours for ls still pending but low priority
@eyalb181 eyalb181 changed the title Support ls, openat Aug 28, 2022
@t4lz t4lz self-assigned this Sep 5, 2022
@aviramha aviramha mentioned this issue Jan 4, 2023
Closed
@t4lz t4lz mentioned this issue Jan 17, 2023
Closed
bors bot pushed a commit that referenced this issue Jan 23, 2023
@t4lz
getdents64 hook (#958)
7b4de94 
Add a detour for the `getdents64` Linux syscall to support `os.ReadDir` on Go.
Part of #120.

This PR adds a detour for `getdents64`, the necessary protocol messages for that (request + response), the handling on the agent side, and the following tests:
1. Integration test for Go's `os.ReadDir` on linux, using the same go program as the existing integration test for mac. The test has to be different because on Linux Go calls the `getdents64` syscall so it's a different flow with different messages, so the linux test simulates that case.
2. Integration test for bypassing `os.ReadDir`. This test checks that if we did hook the syscall but then determined this directory should be read locally, that when we then "bypass" and call the syscall locally - it works as expected.
3. E2E test - uses a similar Go app, and verifies that `getdents64` detour logic also works on the agent's side.
@t4lz t4lz removed their assignment Jan 23, 2023
@eyalb181 eyalb181 mentioned this issue Feb 28, 2023
Open
@aviramha aviramha mentioned this issue Feb 28, 2023
Closed
@t4lz
Copy link
Member

t4lz commented Oct 11, 2023

Maybe we should recheck some of the use-cases after #2003. Maybe more stuff works now?
@Razz4780
Copy link
Contributor

Ah yes, I checked ls on linux after #2003
It worked :)
@MartinEmrich
Copy link

MartinEmrich commented Oct 18, 2023 •
edited
Loading

Should this also include newfstatat()? I run a java application via mirrord, and it cannot find a file that's present remotely (via a configmap mount).

Running within strace, I see this attempt to access that path:

[pid 18182] newfstatat(AT_FDCWD, "/etc/xxx/yyy", 0x7f985ce7fd20, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)

In Java, I use java.nio.file.Files walk() method at that point.

#2001 also mentions that Java class, but I have no segfault, and run 3.72.0 which is said to containt that fix.
@t4lz
Copy link
Member

t4lz commented Oct 18, 2023

@MartinEmrich files under /etc/ are read locally by default. In order to make mirrord read that file remotely even though it is in /etc/, you can add the file's path to the feature.fs.read_only configuration value, or to feature.fs.read_write, if you also want to write to that file. Feel free to come to our discord for live support.
@MartinEmrich
Copy link

@t4lz Thanks, that did the trick, it works!
@eyalb181
Copy link
Member Author

@infiniteregrets Could you please retest the first four use cases (Python list - ls on Linux)?
@infiniteregrets
Copy link
Contributor

infiniteregrets commented Dec 29, 2023 •
edited
Loading

macos:
does not work on
@infiniteregrets
Copy link
Contributor

support for ls in python and node was added by #2145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
6 participants
@eyalb181 @MartinEmrich @t4lz @Razz4780 @meowjesty @infiniteregrets