Wait for reexec to finish when fileOutput is nil #7292
Conversation
| if ret < 0 { | ||
| return false, -1, errors.New("error waiting for the re-exec process") | ||
| } | ||
|
|
There was a problem hiding this comment.
IDK, but will ask. We have a couple of other returns before the end of this function at lines 311 and 315. Do we need to call there too?
There was a problem hiding this comment.
I think we probably should. Can we defer a function that runs reexec_in_user_namespace_wait? Would there be a problem if we ran it against the same PID in rapid succession?
There was a problem hiding this comment.
@jdieter if we did the defer, we'd need to capture the status of the call and return appropriately based on that. I'm not sure that's doable with defer, @mheon or @vrothberg might have some smarter thoughts on that or my question in general.
There was a problem hiding this comment.
we could probably record the exit error and run it only when retErr != nil. What do you think?
There was a problem hiding this comment.
Ok, I'm not very proficient in Go, but tell me if my lastest push has what you're looking for. I've setup a deferred function that waits for the child if retErr != nil. I don't bother with the return value from reexec_in_user_namespace_wait because we presumably are more interested in passing whatever error is causing us to bail.
|
@giuseppe PTAL |
Currently, we're not cleanup up after ourselves when fileOutput is nil. This patch fixes that. Signed-off-by: Jonathan Dieter <[email protected]>
|
@giuseppe PTAL |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, jdieter 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 |
openshift-ci-robot
added
the
approved
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Currently, we're not cleaning up up after ourselves when
fileOutputis nil. This patch fixes that.