Repository: GoogleContainerTools/kpt
Pull request #2639 (2022-01-07 21:12:53)

feat: always print truncated function stderr
This change makes it so that kpt fn render|eval will always print
truncated stderr output from functions. This is intended to improve the
experience of debugging functions.

Issues: GoogleContainerTools/kpt#2599


droot: @sdowell Two things:

1. we should add a test case
2. Can you paste the CLI output for a use-cases in the description of the PR to understand the change better.
sdowell: > @sdowell Two things:
> 
> 1. we should add a test case
> 2. Can you paste the CLI output for a use-cases in the description of the PR to understand the change better.

1. I am not sure whether there are any existing unit tests for this class of functionality (e.g. capturing stdout/stderr), but I'll see what I can come up with
2. Sure thing, I will paste a snippet below. To test this I added a line to print "Hello world!" to stderr inside the `set-image` function. Let me know if you have any questions
```
$ kpt fn eval set-annotations-imperative --image gcr.io/kpt-fn/set-image:unstable -- name=nginx newName=bitnami/nginx newTag=1.21.4
[RUNNING] "gcr.io/kpt-fn/set-image:unstable"
[PASS] "gcr.io/kpt-fn/set-image:unstable" in 1.7s
  Stderr:
    "Hello world!"
```
mengqiy: Can you please also add a test for render?
sdowell: > Can you please also add a test for render? LGTM otherwise

@mengqiy Good suggestion - done!

Patch:
diff --git a/internal/fnruntime/runner.go b/internal/fnruntime/runner.go
index 8ec6f7f5..051a5330 100644
--- a/internal/fnruntime/runner.go
+++ b/internal/fnruntime/runner.go
@@ -165,6 +165,7 @@ func (fr *FunctionRunner) Filter(input []*yaml.RNode) (output []*yaml.RNode, err
 	if !fr.disableCLIOutput {
 		pr.Printf("[PASS] %q in %v\n", fr.name, time.Since(t0).Truncate(time.Millisecond*100))
 		printFnResult(fr.ctx, fr.fnResult, printer.NewOpt())
+		printFnStderr(fr.ctx, fr.fnResult.Stderr)
 	}
 	return output, err
 }
@@ -345,16 +346,22 @@ func printFnResult(ctx context.Context, fnResult *fnresult.Result, opt *printer.
 // on kpt CLI.
 func printFnExecErr(ctx context.Context, fnErr *ExecError) {
 	pr := printer.FromContextOrDie(ctx)
-	if len(fnErr.Stderr) > 0 {
+	printFnStderr(ctx, fnErr.Stderr)
+	pr.Printf("  Exit code: %d\n\n", fnErr.ExitCode)
+}
+
+// printFnStderr prints given stdErr in a user friendly format on kpt CLI.
+func printFnStderr(ctx context.Context, stdErr string) {
+	pr := printer.FromContextOrDie(ctx)
+	if len(stdErr) > 0 {
 		errLines := &multiLineFormatter{
 			Title:          "Stderr",
-			Lines:          strings.Split(fnErr.Stderr, "\n"),
+			Lines:          strings.Split(stdErr, "\n"),
 			UseQuote:       true,
 			TruncateOutput: printer.TruncateOutput,
 		}
 		pr.Printf("%s", errLines.String())
 	}
-	pr.Printf("  Exit code: %d\n\n", fnErr.ExitCode)
 }
 
 // path (location) of a KRM resources is tracked in a special key in


Test patch:
diff --git a/e2e/testdata/fn-eval/exec-function-stderr/.expected/config.yaml b/e2e/testdata/fn-eval/exec-function-stderr/.expected/config.yaml
new file mode 100644
index 00000000..f45537b8
--- /dev/null
+++ b/e2e/testdata/fn-eval/exec-function-stderr/.expected/config.yaml
@@ -0,0 +1,24 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+testType: eval
+stdErr: |
+  [RUNNING] "./function.sh"
+  [PASS] "./function.sh" in 0s
+    Stderr:
+      "Hello world 0!"
+      "Hello world 1!"
+      "Hello world 2!"
+      "Hello world 3!"
+      ...(18 line(s) truncated, use '--truncate-output=false' to disable)
diff --git a/e2e/testdata/fn-eval/exec-function-stderr/.expected/diff.patch b/e2e/testdata/fn-eval/exec-function-stderr/.expected/diff.patch
new file mode 100644
index 00000000..cc985fa3
--- /dev/null
+++ b/e2e/testdata/fn-eval/exec-function-stderr/.expected/diff.patch
@@ -0,0 +1,21 @@
+diff --git a/resources.yaml b/resources.yaml
+index e8ae6bb..297b99f 100644
+--- a/resources.yaml
++++ b/resources.yaml
+@@ -15,7 +15,7 @@ apiVersion: apps/v1
+ kind: Deployment
+ metadata:
+   name: nginx-deployment
+-  namespace: foo
++  namespace: bar
+ spec:
+   replicas: 3
+ ---
+@@ -23,6 +23,6 @@ apiVersion: custom.io/v1
+ kind: Custom
+ metadata:
+   name: custom
+-  namespace: foo
++  namespace: bar
+ spec:
+   image: nginx:1.2.3
diff --git a/e2e/testdata/fn-eval/exec-function-stderr/.expected/exec.sh b/e2e/testdata/fn-eval/exec-function-stderr/.expected/exec.sh
new file mode 100644
index 00000000..4a180de5
--- /dev/null
+++ b/e2e/testdata/fn-eval/exec-function-stderr/.expected/exec.sh
@@ -0,0 +1,19 @@
+#! /bin/bash
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -eo pipefail
+
+kpt fn source \
+  | kpt fn eval --exec ./function.sh
diff --git a/e2e/testdata/fn-eval/exec-function-stderr/.krmignore b/e2e/testdata/fn-eval/exec-function-stderr/.krmignore
new file mode 100644
index 00000000..9d7a4007
--- /dev/null
+++ b/e2e/testdata/fn-eval/exec-function-stderr/.krmignore
@@ -0,0 +1,1 @@
+.expected
diff --git a/e2e/testdata/fn-eval/exec-function-stderr/function.sh b/e2e/testdata/fn-eval/exec-function-stderr/function.sh
new file mode 100755
index 00000000..c9cb9d7b
--- /dev/null
+++ b/e2e/testdata/fn-eval/exec-function-stderr/function.sh
@@ -0,0 +1,21 @@
+#! /usr/bin/env bash
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+sed -e 's/foo/bar/'
+
+for i in {0..20}
+do
+  >&2 echo "Hello world $i!"
+done
diff --git a/e2e/testdata/fn-eval/exec-function-stderr/resources.yaml b/e2e/testdata/fn-eval/exec-function-stderr/resources.yaml
new file mode 100644
index 00000000..e8ae6bba
--- /dev/null
+++ b/e2e/testdata/fn-eval/exec-function-stderr/resources.yaml
@@ -0,0 +1,28 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+  namespace: foo
+spec:
+  replicas: 3
+---
+apiVersion: custom.io/v1
+kind: Custom
+metadata:
+  name: custom
+  namespace: foo
+spec:
+  image: nginx:1.2.3
diff --git a/e2e/testdata/fn-render/exec-function-stderr/.expected/config.yaml b/e2e/testdata/fn-render/exec-function-stderr/.expected/config.yaml
new file mode 100644
index 00000000..9183295c
--- /dev/null
+++ b/e2e/testdata/fn-render/exec-function-stderr/.expected/config.yaml
@@ -0,0 +1,24 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+allowExec: true
+stdErr: |
+  [RUNNING] "./testdata/fn-render/exec-function-stderr/function.sh"
+  [PASS] "./testdata/fn-render/exec-function-stderr/function.sh" in 0s
+    Stderr:
+      "Hello world 0!"
+      "Hello world 1!"
+      "Hello world 2!"
+      "Hello world 3!"
+      ...(18 line(s) truncated, use '--truncate-output=false' to disable)
diff --git a/e2e/testdata/fn-render/exec-function-stderr/.expected/diff.patch b/e2e/testdata/fn-render/exec-function-stderr/.expected/diff.patch
new file mode 100644
index 00000000..cc985fa3
--- /dev/null
+++ b/e2e/testdata/fn-render/exec-function-stderr/.expected/diff.patch
@@ -0,0 +1,21 @@
+diff --git a/resources.yaml b/resources.yaml
+index e8ae6bb..297b99f 100644
+--- a/resources.yaml
++++ b/resources.yaml
+@@ -15,7 +15,7 @@ apiVersion: apps/v1
+ kind: Deployment
+ metadata:
+   name: nginx-deployment
+-  namespace: foo
++  namespace: bar
+ spec:
+   replicas: 3
+ ---
+@@ -23,6 +23,6 @@ apiVersion: custom.io/v1
+ kind: Custom
+ metadata:
+   name: custom
+-  namespace: foo
++  namespace: bar
+ spec:
+   image: nginx:1.2.3
diff --git a/e2e/testdata/fn-render/exec-function-stderr/.krmignore b/e2e/testdata/fn-render/exec-function-stderr/.krmignore
new file mode 100644
index 00000000..9d7a4007
--- /dev/null
+++ b/e2e/testdata/fn-render/exec-function-stderr/.krmignore
@@ -0,0 +1,1 @@
+.expected
diff --git a/e2e/testdata/fn-render/exec-function-stderr/Kptfile b/e2e/testdata/fn-render/exec-function-stderr/Kptfile
new file mode 100644
index 00000000..6f2fe112
--- /dev/null
+++ b/e2e/testdata/fn-render/exec-function-stderr/Kptfile
@@ -0,0 +1,7 @@
+apiVersion: kpt.dev/v1
+kind: Kptfile
+metadata:
+  name: app
+pipeline:
+  mutators:
+    - exec: "./testdata/fn-render/exec-function-stderr/function.sh"
diff --git a/e2e/testdata/fn-render/exec-function-stderr/function.sh b/e2e/testdata/fn-render/exec-function-stderr/function.sh
new file mode 100755
index 00000000..c9cb9d7b
--- /dev/null
+++ b/e2e/testdata/fn-render/exec-function-stderr/function.sh
@@ -0,0 +1,21 @@
+#! /usr/bin/env bash
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+sed -e 's/foo/bar/'
+
+for i in {0..20}
+do
+  >&2 echo "Hello world $i!"
+done
diff --git a/e2e/testdata/fn-render/exec-function-stderr/resources.yaml b/e2e/testdata/fn-render/exec-function-stderr/resources.yaml
new file mode 100644
index 00000000..e8ae6bba
--- /dev/null
+++ b/e2e/testdata/fn-render/exec-function-stderr/resources.yaml
@@ -0,0 +1,28 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+  namespace: foo
+spec:
+  replicas: 3
+---
+apiVersion: custom.io/v1
+kind: Custom
+metadata:
+  name: custom
+  namespace: foo
+spec:
+  image: nginx:1.2.3
diff --git a/internal/fnruntime/runner_test.go b/internal/fnruntime/runner_test.go
index 8c3e7494..97fd2b1f 100644
--- a/internal/fnruntime/runner_test.go
+++ b/internal/fnruntime/runner_test.go
@@ -18,12 +18,14 @@ package fnruntime
 
 import (
 	"bytes"
+	"context"
 	"io/ioutil"
 	"os"
 	"path"
 	"strings"
 	"testing"
 
+	"github.com/GoogleContainerTools/kpt/internal/printer"
 	"github.com/GoogleContainerTools/kpt/internal/types"
 	kptfilev1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1"
 	"github.com/stretchr/testify/assert"
@@ -577,3 +579,70 @@ file:
 		assert.Equal(t, tc.expected, string(out))
 	}
 }
+
+func TestPrintFnStderr(t *testing.T) {
+	tests := map[string]struct {
+		input          string // input
+		truncateOutput bool   // whether to truncate output
+		expected       string // expected result
+	}{
+		"no output": {
+			input:          ``,
+			truncateOutput: true,
+			expected:       ``,
+		},
+		"truncated output": {
+			input: `0
+1
+2
+3
+4
+5`,
+			truncateOutput: true,
+			expected: `  Stderr:
+    "0"
+    "1"
+    "2"
+    "3"
+    ...(2 line(s) truncated, use '--truncate-output=false' to disable)
+`,
+		},
+		"non-truncated output": {
+			input: `0
+1
+2
+3
+4
+5`,
+			truncateOutput: false,
+			expected: `  Stderr:
+    "0"
+    "1"
+    "2"
+    "3"
+    "4"
+    "5"
+`,
+		},
+	}
+	cleanupFunc := func() func() {
+		origTruncateOutput := printer.TruncateOutput
+		return func() {
+			printer.TruncateOutput = origTruncateOutput
+		}
+	}()
+	defer cleanupFunc()
+	for testName, tc := range tests {
+		t.Run(testName, func(t *testing.T) {
+			printer.TruncateOutput = tc.truncateOutput
+			out := &bytes.Buffer{}
+			err := &bytes.Buffer{}
+			ctx := printer.WithContext(context.Background(), printer.New(out, err))
+
+			printFnStderr(ctx, tc.input)
+
+			assert.Equal(t, tc.expected, err.String())
+			assert.Equal(t, "", out.String())
+		})
+	}
+}