Skip to content

Commit

Permalink
Change format.Writer to not format phase output log lines (#1431)
Browse files Browse the repository at this point in the history
When kube.ExecOutput() writes the remote stdout/stderr outputs to the
user-provided writers, it calls format.Writer() to format the outputs
to the Kanister-log format. This causes the "phase output" to be
prepended with log metadata such as log timestamp, caller function etc.

A previous attempt to expand the output.Parse() function regex to handle
these log metadata prefixes has caused some downstream failures.

This fix reverts to the previous implementation of output.Parse(). It
also updates the format.Writer() to not format any "phase output".

Signed-off-by: Ivan Sim <ivan.sim@kasten.io>

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
ihcsim and mergify[bot] authored May 6, 2022
1 parent 3f1f8a3 commit a1d4240
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 19 deletions.
5 changes: 5 additions & 0 deletions pkg/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/kanisterio/kanister/pkg/field"
"github.com/kanisterio/kanister/pkg/log"
pkgout "github.com/kanisterio/kanister/pkg/output"
)

var regex = regexp.MustCompile("[\r\n]")
Expand All @@ -36,6 +37,10 @@ func Log(podName string, containerName string, output string) {
func LogTo(w io.Writer, pod string, container string, output string) {
if output != "" {
for _, line := range regex.Split(output, -1) {
if strings.Contains(line, pkgout.PhaseOpString) {
continue
}

if line != "" {
fields := field.M{
"Pod": pod,
Expand Down
6 changes: 2 additions & 4 deletions pkg/function/kube_exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,9 @@ func (s *KubeExecTest) TestParseLogAndCreateOutput(c *C) {
outChecker Checker
}{
{"###Phase-output###: {\"key\":\"version\",\"value\":\"0.78.0\"}", map[string]interface{}{"version": "0.78.0"}, IsNil, NotNil},
{"###Phase-output###: {\"key\":\"version\",\"value\":\"0.78.0\"}\n###Phase-output###: {\"key\":\"path\",\"value\":\"/backup/path\"}",
map[string]interface{}{"version": "0.78.0", "path": "/backup/path"}, IsNil, NotNil},
{"Random message ###Phase-output###: {\"key\":\"version\",\"value\":\"0.78.0\"}", map[string]interface{}{"version": "0.78.0"}, IsNil, NotNil},
{"###Phase-output###: {\"key\":\"version\",\"value\":\"0.78.0\"},Random message", map[string]interface{}{"version": "0.78.0"}, IsNil, NotNil},
{"Random message ###Phase-output###: {\"key\":\"version\",\"value\":\"0.78.0\"},Random message", map[string]interface{}{"version": "0.78.0"}, IsNil, NotNil},
{"{\"Out\":\"###Phase-output###: {\"key\":\"version\",\"value\":\"0.78.0\"}\"}", map[string]interface{}{"version": "0.78.0"}, IsNil, NotNil},
{"{\"Out\":\"###Phase-output###: {\"key\":\"version\",\"value\":\"0.78.0\"}\", \"Pod\":\"my-pod\"}", map[string]interface{}{"version": "0.78.0"}, IsNil, NotNil},
{"Random message with newline \n###Phase-output###: {\"key\":\"version\",\"value\":\"0.78.0\"}", map[string]interface{}{"version": "0.78.0"}, IsNil, NotNil},
{"###Phase-output###: Invalid message", nil, NotNil, IsNil},
{"Random message", nil, IsNil, IsNil},
Expand Down
17 changes: 2 additions & 15 deletions pkg/output/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,7 @@ func fPrintOutput(w io.Writer, key, value string) error {
return nil
}

// reStr looks for phase output patterns from log lines.
// it matches these patterns:
// case1: some-prefix###Phase-output###: {"foo":"bar"}
// case2: ###Phase-output###: {"foo":"bar","baz":"bazz"}some-suffix
// case3: ###Phase-output###: {"foo":"bar","baz":"bazz"}
// case4: ###Phase-output###: foo
// See unit tests.
// {.*?} ensures non-greedy evaluation so that the matching will stop at the
// first curly bracket.
// The Parse function return case4 as errors to the caller.
const reStr = `(.*)` + PhaseOpString + `( *)({.*?}|.*)`
const reStr = PhaseOpString + `(.*)$`

var logRE = regexp.MustCompile(reStr)

Expand All @@ -101,12 +91,9 @@ func Parse(l string) (*Output, error) {
if len(match) == 0 {
return nil, nil
}

stripped := strings.Replace(match[0][3], "\\", "", -1)
op, err := UnmarshalOutput(stripped)
op, err := UnmarshalOutput(match[0][1])
if err != nil {
return nil, err
}

return op, nil
}

0 comments on commit a1d4240

Please sign in to comment.