Bug fix with runc container lifetime management#1272
Bug fix with runc container lifetime management#1272helsaawy merged 3 commits intomicrosoft:masterfrom
Conversation
| entity := log.G(ctx).WithField(logfields.ContainerID, c.id) | ||
| entity.Debug("opengcs::Container::Delete") |
There was a problem hiding this comment.
Should the logs here be something other than debug?
There was a problem hiding this comment.
Ill change them to Info
| Err error `json:"error,omitempty"` | ||
| } | ||
|
|
||
| func (l *standardLogEntry) asError() (err error) { |
There was a problem hiding this comment.
I'm not the biggest fan of this function, trying to see if I can think of another way of doing things
There was a problem hiding this comment.
I also am not a fan, but I couldn't find an example elsewhere of something similar, and simply wrapping the string an errors.New meant the string operations would be pushed elsewhere in the code.
There was a problem hiding this comment.
Why is wrapping the string an issue?
There was a problem hiding this comment.
Can we not directly reference the errors in https://github.com/opencontainers/runc/blob/master/libcontainer/error.go instead of re-hardcoding the strings here?
There was a problem hiding this comment.
I think it may be because of version issues (I think I was usuing v0.0.115), but the runc errors I was getting were container with id exists, and not what runc exports of container with given ID already exists. Similarly, container does not exist has the container id in the middle of it
I do want to revisit this in the future if we're confident those runc errors will hold steady and use those errors
There was a problem hiding this comment.
For the string wrapping, that means elsewhere in the code, if we want to match against the error returned, we'd have to use string search, and that would leak into the runc handling code
Fixed bug where container is cast as a process, which then causes the container to be deleted prematurely before when the container finishes executing. Added conversion of runc log file error strings into `error` types that wrap HResult error types. Wrapped runc errors from log file, which is more informative that error returned from cmd execution. Added traces to guest container operations, to trace low level container operations. Signed-off-by: Hamza El-Saawy <[email protected]>
helsaawy
force-pushed
the
he/runcstate
branch
from
b15c2ff to
b2e849f
Compare
Signed-off-by: Hamza El-Saawy <[email protected]>
Signed-off-by: Hamza El-Saawy <[email protected]>
Fixed bug where container is cast as a process, which then causes the container to be deleted prematurely before when the container finishes executing. Currently, whenever an LCOW container is stopped, the logs show multiple errors being raised that runc cannot find the container, which cause the Kill command issued by containerd to exit unsuccessfully. Added conversion of runc log file error strings into error types that wrap HResult error types. Wrapped runc errors from log file, which is more informative that error returned from cmd execution. Added traces to guest container operations, to trace low level container operations. Signed-off-by: Hamza El-Saawy [email protected]
6 participants
Fixed bug where container is cast as a process, which then causes the
container to be deleted prematurely before when the container finishes
executing.
Currently, whenever an LCOW container is stopped, the logs show multiple
errors being raised that runc cannot find the container, which cause the
Killcommand issued by containerd to exit unsuccessfully.Added conversion of runc log file error strings into
errortypes thatwrap HResult error types.
Wrapped runc errors from log file, which is more informative that error
returned from cmd execution.
Added traces to guest container operations, to trace low level container
operations.
Signed-off-by: Hamza El-Saawy [email protected]