Forward Hook output logs to helm controller - #1604
lubronzhan wants to merge 2 commits into
Conversation
Initialize the hook output callback to prevent a nil-function panic. Forward bounded container output through the existing action logger and failure-event buffer, with tests for the Helm log-streaming path. Signed-off-by: lubronzhan <lubronzhan@gmail.com> Assisted-by: Codex/gpt-6
Forward each Helm write without introducing truncation or chunk limits. Leave output policy changes for a separate maintainer discussion. Signed-off-by: lubronzhan <lubronzhan@gmail.com> Assisted-by: Codex/gpt-6
4330205 to
53ad674
Compare
|
|
||
| func (w *hookLogWriter) Write(p []byte) (int, error) { | ||
| if len(p) > 0 { | ||
| w.log.Debug("Helm hook output", "output", string(p)) |
There was a problem hiding this comment.
Is this guaranteed to be an individual log line from the hook container? How will Helm call Write()? Will it send the entire log stream at once or can it buffer the logs? I'm concerned of this approach breaking up the log stream in a weird way. Need to understand how Helm calls this function to know what to do. Do we have to keep a buffer to detect lines?
There was a problem hiding this comment.
You're right—there is no guarantee that each Write() contains a complete line. Helm uses io.Copy from the Pod log response into this writer, so a write can contain multiple lines or only part of one.
I propose buffering partial lines per container and emitting each complete line through the action logger. This would preserve line boundaries without introducing truncation.
One detail to resolve is the final line without a trailing newline: Helm doesn't close the supplied writer, so Write() alone cannot detect the end of the stream. We would need an explicit flush after log collection, including on errors.
Does that approach make sense?
There was a problem hiding this comment.
I'd also go further and try to parse each detected line as JSON. If the parsing succeeds, then we are better off adding the parsed map[string]any to some key in our own logger e.g. logFields. This way we capture structured logging much better (and structured logging is what every container in a k8s cluster should do!). If the parsing fails, then we can just trim leading and trailing whitespaces and add the line as-is to e.g. logLine. These two keys logFields and logLine are more descripted of what we just parsed.
2 participants
Fixes #1562.
Initialize the hook output callback to prevent a nil-function panic when a chart uses
helm.sh/hook-output-log-policy. Forward container output through the existing action debug logger and failure-event buffer.Tests cover the Helm log-streaming path, the no-logger fallback, and forwarding output without truncation.
This PR was written in part with the assistance of generative AI.