Remove log file from runc commands #1436
Conversation
| type standardLogEntry struct { | ||
| Level logrus.Level `json:"level"` | ||
| Message string `json:"msg"` | ||
| Err error `json:"error,omitempty"` | ||
| } | ||
|
|
||
| func (l *standardLogEntry) asError() (err error) { | ||
| err = parseRuncError(l.Message) | ||
| if l.Err != nil { | ||
| err = errors.Wrapf(err, l.Err.Error()) |
There was a problem hiding this comment.
Given we swapped to fmt.Errorf in most other places in this PR, could we continue the tradition here
There was a problem hiding this comment.
Although I remember the GCS being one of the only places we made use of the stack embedding pkg/errors does, but I'll need to find this..
There was a problem hiding this comment.
Lets see if this breaks nightly (more) ...
There was a problem hiding this comment.
Okay here's what I was talking about
hcsshim/internal/guest/bridge/bridge.go
Line 449 in 06ce0c3
There was a problem hiding this comment.
Oh, I forgot about that
do we use the line number or stack frame at all in debugging?
Since "github.com/pkg/errors" is on its way out, do we want to switch over to "errors", and hope we add enough context via fmt.Errorf?
Or keep it?
There was a problem hiding this comment.
Mmm... I'd say lets leave it for now and make shifting away a separate change (if we want).
There was a problem hiding this comment.
Undid all the errors stuff
Certain runc commands (eg, Delete, State) do not need to use a log file since they do not pass stdio to the container. This PR switches the commands to consume the error directly from stderr, without needing to parse from a file. This fixes a bug where the directory containing the runc log file for a container is deleted before the container itself can be deleted via the runc command. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Certain runc commands (eg, Delete, State) do not need to use a log file since they do not pass stdio to the container. This PR switches the commands to consume the error directly from stderr, without needing to parse from a file. This fixes a bug where the directory containing the runc log file for a container is deleted before the container itself can be deleted via the runc command. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Certain runc commands (eg, Delete, State) do not need to use a log file since they do not pass stdio to the container. This PR switches the commands to consume the error directly from stderr, without needing to parse from a file. This fixes a bug where the directory containing the runc log file for a container is deleted before the container itself can be deleted via the runc command. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Certain runc commands (eg, Delete, State) do not need to use a log file
since runc does not consume the stdio and pass it to the container.
This PR switches the commands to consume the error directly from stderr,
without needing to parse from a file.
This fixes a bug where the directory containing the runc log file for a
container is deleted before the container itself can be deleted via the
runc command.
Runc errors that the directory containing the logfile does not exist, but since the returned error is
errors.Wrapf(runcErr,...), whereruncErris parsed from the logfile, it ends up being nil.The fix is to read directly from stderr, skipping the log file.
Additionally, even if the runc delete command errors, the container is still deleted from the
gcs, since if runc was able to delete successfully delete the container but still errored, future restarts will be successful.Signed-off-by: Hamza El-Saawy hamzaelsaawy@microsoft.com