tilt-dev/tilt-extensions

`restart_process` fails with permission denied

Opened this issue · 7 comments

docker_build_with_restart has been failing for me recently, but I've found out what's the issue - even though /tmp/.restart-proc has permissions 666, the /tmp folder has sticky bit enabled and the containers run with root user instead of 1001 that's the owner of /tmp/.restart-proc.

Will copy 1 file(s) to container: [api-fb9658fb7-pg8pd/api]
- '~/code/go/build/api' --> '/bin/api'
[CMD 1/1] sh -c date > /tmp/.restart-proc
sh: can't create /tmp/.restart-proc: Permission denied
  → Failed to update container api-fb9658fb7-pg8pd/api: executing on container 818f0243f0: command "date > /tmp/.restart-proc" failed with exit code: 1
Build Failed: executing on container 818f0243f0: command "date > /tmp/.restart-proc" failed with exit code: 1

I've found two temporary workarounds, but they won't scale:

  • kubectl exec into the pod and toggle /tmp sticky bit
  • kubectl exec into the pod and chown /tmp/.restart-proc

Any other ideas on how to solve this? Could the .restart-proc file be moved to /tmp/.restart/proc and sticky bit toggled on tmp/.restart folder?

The fix was rolled back due to the regression reported in #544, sorry

Hey, I took a look at the issue there, not sure how to proceed.

       → /bin/sh: 1: [mkdir,: not found
...
process "/bin/sh -c [\"mkdir\", \"-p\", \"\\tmp\\.restart\"]"

It seems for some reason docker builder garbles the parameters for this RUN command in exec form. And that path is also not correct. We can work around missing mkdir by using WORKDIR dockerfile instruction.

But the os.path.dirname output will still be a windows-style path that will, in the best case, create a useless directory. I think here we'll always need a linux-style dirname function. Any idea how to get that in Tiltfile? In golang, this would mean using path.Dir instead of filepath.Dir.

oh ya, good call... i would probably use string manipulation, something like:

index = path.rfind('/')
if index != -1:
  dir = path[0:index]

https://github.com/bazelbuild/starlark/blob/master/spec.md#string%C2%B7rfind

Thanks, this works and it also seems like a safe solution, because / character is not allowed in filenames.

unfortunately, this had to be rolled back again.

a container with a sticky /tmp seems pretty rare. is there some reason why you can't solve this problem with the existing restart_file param?

Sad. I think you should consider banning me from any future contributions, given the track record :)

a container with a sticky /tmp seems pretty rare.

[~] docker run -t --rm busybox:latest ls -ld /tmp
drwxrwxrwt    2 root     root          4096 Dec 19 21:10 /tmp
[~] docker run -t --rm alpine:latest ls -ld /tmp
drwxrwxrwt    2 root     root          4096 Dec  7 09:45 /tmp
[~] docker run -t --rm ubuntu:latest ls -ld /tmp
drwxrwxrwt 2 root root 4096 Dec 11 14:34 /tmp

Found the root cause for my issues, though. I use colima instead of docker desktop and in a recent update it switched the base VM to ubuntu. And there is this new kernel parameter fs.protected_regular that's set to 2 by default and inherited by all containers. In my case it prevents the root user from overwriting regular files created by other users when sticky bit is set in the folder. In docker for desktop, this parameter seems to be set to 0 by default.

I've now set the parameter to 0, but still, would you accept a WORKDIR-based solution that I outlined in the second PR? It doesn't require mkdir nor sh in the base container image.

eh, it happens.

how would a WORKDIR-based solution work?