Fix missing F-sequence on container exit #592
Conversation
There was a problem hiding this comment.
codewise LGTM, can't comment on the CRI interface.
@haircommander PTAL
|
|
||
| # If log is empty, this means containers aren't working in test environment | ||
| # In that case, just verify that the fix code exists in the source | ||
| if [ -z "$log_content" ]; then |
There was a problem hiding this comment.
I don't love this, as we'll miss misbehaving conmon if we do this
There was a problem hiding this comment.
Indeed! I will change it to be more focused.
| } else if (writev_buffer_append_segment(k8s_log_fd, &bufv, "F\n", 2) < 0) { | ||
| nwarn("failed to write terminating F-sequence"); | ||
| } else { | ||
| k8s_bytes_written += TSBUFLEN - 1 + 2; |
There was a problem hiding this comment.
hmm in the case where we succeed in the first branch but fail in the second, I don't think we'd get here but we did write TSBUFLEN - 1 to the buffer. I think your branching strategy is slick but maybe not completely accurate and it could be simplified to be a bit clearer what's going on. it took me a moment to understand
There was a problem hiding this comment.
I will change the logic to use bools, not if/else to be more transparent.
Generates terminating F-sequence when container exits with partial log output. Fixes containers#252 Signed-off-by: Jindrich Novy <jnovy@redhat.com>
3 participants
Generates terminating F-sequence when container exits with partial log output.
Fixes #252