ctr_logs: use container name or ID as SYSLOG_IDENTIFIER for journald #294
Conversation
|
fixes #293 |
|
LGTM |
| @@ -257,6 +274,9 @@ int write_journald(int pipe, char *buf, ssize_t buflen) | |||
| if (name && writev_buffer_append_segment(dev_null, &bufv, container_name, name_len + NAME_EQ_LEN) < 0) | |||
| return -1; | |||
|
|
|||
| if (writev_buffer_append_segment(dev_null, &bufv, syslog_identifier, syslog_identifier_len) < 0) | |||
There was a problem hiding this comment.
Not sure but i think we must check if syslog_identifier is valid before flushing. Maybe
- if (writev_buffer_append_segment(dev_null, &bufv, syslog_identifier, syslog_identifier_len) < 0)
+ if (syslog_identifier && writev_buffer_append_segment(dev_null, &bufv, syslog_identifier, syslog_identifier_len) < 0)There was a problem hiding this comment.
I suppose we could but I intentionally made it so that syslog_identifier was always defined (either container name or ID). if it'd make you more comfortable I can update
There was a problem hiding this comment.
@haircommander oh it is completely fine then :) . Thanks for confirming.
| syslog_identifier_len = name_len + SYSLOG_IDENTIFIER_EQ_LEN; | ||
| } else { | ||
| syslog_identifier = g_strdup_printf("SYSLOG_IDENTIFIER=%s", short_cuuid); | ||
| syslog_identifier_len = TRUNC_ID_LEN + SYSLOG_IDENTIFIER_EQ_LEN; |
There was a problem hiding this comment.
the one thing I am concerned about is the null byte here, but in other uses we use TRUNC_ID_LEN + CID_EQ_LEN which doesn't seem to count null
|
LGTM |
|
@haircommander are these CI failures expected? If so, please merge |
|
I was able to reproduce the test failure, we want cri-o/cri-o#5334 first |
now, when journald is used for the log driver, the logs will appear as: ctrName[PID]:... or partialID[PID]:... (if using a sufficiently old podman) instead of conmon[PID] Signed-off-by: Peter Hunt <pehunt@redhat.com>
haircommander
force-pushed
the
syslog-identifier
branch
from
c8fe7b8
to
e7a5e0c
Compare
|
hold for #297 |
|
Since #297 was merged, I am going to merge this. |
now, when journald is used for the log driver, the logs will appear as:
ctrName[PID]:...
or
partialID[PID]:... (if using a sufficiently old podman)
instead of
conmon[PID]
Signed-off-by: Peter Hunt pehunt@redhat.com