Improves and fixes jailer #96
Conversation
|
Some tests are failing, any pointers would help. I assume it's all tests that were calling setupLogging and expecting it to create fifos. |
I reran the CI job and there are a few things that are showing as failing: This looks like it's caused by your change here; the These two look like failures to find the log fifo, possibly because the On our CI host, I don't know offhand what's taking too long to run. |
|
Can you also add an In addition, an update to the |
|
Might be a little while until I look at this again. It works for us right now so I'm just using it as-is. |
|
Anything missing here, since @xibz's pushes? |
There was a problem hiding this comment.
Hi @jeromegn, my apologies for the delay! I have a few small questions about some of the changes.
machine.go
Outdated
| @@ -152,6 +152,14 @@ func (m *Machine) Logger() *log.Entry { | |||
| return m.logger.WithField("subsystem", userAgent) | |||
| } | |||
|
|
|||
| // PID returns the machine's process ID | |||
| func (m *Machine) PID() (int, error) { | |||
There was a problem hiding this comment.
I could be missing something, but I don't see this function getting called anywhere.
There was a problem hiding this comment.
Yea, I wasn't sure about this function, but I assumed @jeromegn was using it for something, so I didn't touch it.
There was a problem hiding this comment.
I am using it, but it could be part of a separate PR. I need to get at the PID for the process for the scheduling software I'm using.
There was a problem hiding this comment.
@jeromegn Thanks! Can we move this to a separate PR?
|
@samuelkarp - Did you mean wrapped body at 72 instead of 50? |
|
@xibz oops, yes 😅 |
This fix includes proper handling of fifos as before the SDK was not adding the fifo files to the correct root path during jailing. We now have a new handler, CreateLogFilesHandler, that will now create the fifos in the appropriate location and ensure that the fifos have correct permissions. Signed-off-by: xibz <impactbchang@gmail.com>
Prior to this fix the SDK would use os.Stdin if no stdin was provided in the jailer config. This would cause shell sessions to break and should only include stdin if it was provided in the jailer config. Signed-off-by: xibz <impactbchang@gmail.com>
The machine configuration fifo paths originally pointed to somewhere on the host. However, the fifo paths should be pointing to the chroot of the jailer. This fix addresses that and renames the error that is returned from the jailer handler as it was way too specific. Signed-off-by: xibz <impactbchang@gmail.com>
Description of changes:
machine.PID()to get the process ID and possibly gather metrics on the process