[docker log] Adjust matcher for corner cases + UT#6296
Conversation
prognant
added
changelog/no-changelog
truthbk
left a comment
truthbk
left a comment
There was a problem hiding this comment.
Nice stuff, tricky little bug. Just a couple questions, nits.
| } | ||
| return bs[i-len(exists)] | ||
| } | ||
| return 0xFF |
There was a problem hiding this comment.
So the only thing I don't like about this - and I totally understand why we're doing this because the function interface is so much more usable than a (byte, error) return type - and it might honestly never be a problem given how this function is used in matchHeader(), is that 0xFF could be considered a valid byte value. I will leave this to your best judgement, but in any case we should add a comment to reflect 0xFF is the return code when overflowing exists+bs.
There was a problem hiding this comment.
I switched getByte to checkByte and wrapped the equality assertion inside the function. IMHO it keeps the code simple while avoiding the case where it could return 0xFF for a non-existing byte.
| d.Stop() | ||
| } | ||
|
|
||
| func TestDecoderDetectMultipleDockerHeader(t *testing.T) { |
3 participants
What does this PR do?
Rewrite the tailing-from-docker-socket-end-of-line-matcher
Motivation
The old implementation was missing some cases leading to split lines.
It was also doing
append(...)on potentially large chunk of data.Additional Notes
Not 100% sure, but when tailing from the docker socket, instead of
splitting message based on
\nit may sound better to split messageevery docker header and do the line re-assembling later (as it may
be required in other cases, as per #6265 & #6266)
Describe your test plan
Added various UT, attempted to cover most of realistic use cases.
See AC-184 for a Dockerfile that was exhibiting the issue and gets fixed by this PR.