rootless: fix a regression when using -d #5245
Conversation
openshift-ci-robot
added
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AkihiroSuda, giuseppe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
libpod/oci_conmon_linux.go
Outdated
| return errors.Wrapf(err, "failed to create rootless port sync pipe") | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
should you do the defer errorhandling for rootlessPortSyncR/W here as you do below for rootlessSlipSyncR/W around line 1177?
There was a problem hiding this comment.
no, we cannot close rootlessPortSyncR here, it must be left open as it used later
| if ctr.rootlessPortSyncR != nil { | ||
| defer errorhandling.CloseQuiet(ctr.rootlessPortSyncR) | ||
| } | ||
|
|
There was a problem hiding this comment.
Do you need a similar for rootlessSlirpSyncR?
There was a problem hiding this comment.
do you mean rootlessSlirpSyncW? No, it is already closed in libpod/oci_conmon_linux.go
|
LGTM |
when using -d and port mapping, make sure the correct fd is injected into conmon. Move the pipe creation earlier as the fd must be known at the time we create the container through conmon. Closes: containers#5167 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe
force-pushed
the
rootless-portmapping-detach
branch
from
bcb82fe
to
170fd7b
Compare
|
tests are passing now |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
when using -d and port mapping, make sure the correct fd is injected
into conmon.
Move the pipe creation earlier as the fd must be known at the time we
create the container through conmon.
Closes: #5167
Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com