avoid setting NOTIFY_SOCKET from calling process #54
Conversation
|
Can you rebase on master to pick up the fix for travis plz? |
tonistiigi
force-pushed
the
notify-socket
branch
from
c8b0b68 to
1bf23a3
Compare
Codecov Report
@@ Coverage Diff @@
## master #54 +/- ##
=========================================
- Coverage 6.12% 6.03% -0.09%
=========================================
Files 7 7
Lines 637 646 +9
=========================================
Hits 39 39
- Misses 591 600 +9
Partials 7 7
Continue to review full report at Codecov.
|
|
LGTM |
command_linux.go
Outdated
| continue loop0 | ||
| } | ||
| } | ||
| out = append(in, v) |
There was a problem hiding this comment.
I guess this should be out = append(out, v) :)
Signed-off-by: Tonis Tiigi <[email protected]>
tonistiigi
force-pushed
the
notify-socket
branch
from
1bf23a3 to
c0c55f3
Compare
|
Can we rather fix the issue on runc side? |
| @@ -32,10 +33,24 @@ func (r *Runc) command(context context.Context, args ...string) *exec.Cmd { | |||
| cmd.SysProcAttr = &syscall.SysProcAttr{ | |||
| Setpgid: r.Setpgid, | |||
| } | |||
| cmd.Env = os.Environ() | |||
| cmd.Env = filterEnv(os.Environ(), "NOTIFY_SOCKET") // NOTIFY_SOCKET introduces a special behavior in runc but should only be set if invoked from systemd | |||
There was a problem hiding this comment.
I think NOTIFY_SOCKET env is from systemd and I meet the error before. 😂
There was a problem hiding this comment.
I think the NOTIFY_SOCKET is used for daemon running in the background. go-runc is just like a tool and I think it is ok to remove it.
There was a problem hiding this comment.
I'm fine with runc having a special mode for mounting extra things into container but it needs to be more explicit. Then if this feature is found to be useful for go-runc, it can have a special option for it.
I would be for it but we do need something for a patch release as well and as this affects tools like buildkit and containerd it isn't always very easy to control what runc binary they use. |
- github.com/containerd/go-runc containerd/go-runc@7d11b49...e029b79 - containerd/go-runc#52 Fix Method of judging command execution failure - fixes "init.pid: no such file or directory: unknown" errors - containerd/go-runc#54 avoid setting NOTIFY_SOCKET from calling process Signed-off-by: Sebastiaan van Stijn <[email protected]> Signed-off-by: Derek McGowan <[email protected]>
- github.com/containerd/go-runc containerd/go-runc@7d11b49...e029b79 - containerd/go-runc#52 Fix Method of judging command execution failure - fixes "init.pid: no such file or directory: unknown" errors - containerd/go-runc#54 avoid setting NOTIFY_SOCKET from calling process Signed-off-by: Sebastiaan van Stijn <[email protected]> Signed-off-by: Derek McGowan <[email protected]> Upstream-commit: 1617be92d301de1386adabad5f241d3653b6c8ff Component: engine
- github.com/containerd/go-runc containerd/go-runc@7d11b49...e029b79 - containerd/go-runc#52 Fix Method of judging command execution failure - fixes "init.pid: no such file or directory: unknown" errors - containerd/go-runc#54 avoid setting NOTIFY_SOCKET from calling process Signed-off-by: Sebastiaan van Stijn <[email protected]> Signed-off-by: Derek McGowan <[email protected]> Signed-off-by: zach <[email protected]>
Having
NOTIFY_SOCKETset mutates the spec in runc in unexpected way. As this feature is for running runc with systemd it doesn't have use forgo-runc.related to moby/moby#39866
@crosbymichael
Signed-off-by: Tonis Tiigi [email protected]